Skip to content

Comments

[4.0] Option to use a menu item for privacy policy.#29024

Merged
rdeutz merged 10 commits intojoomla:4.0-devfrom
Harmageddon:privacy_policy_menu_item
Oct 16, 2020
Merged

[4.0] Option to use a menu item for privacy policy.#29024
rdeutz merged 10 commits intojoomla:4.0-devfrom
Harmageddon:privacy_policy_menu_item

Conversation

@Harmageddon
Copy link
Contributor

@Harmageddon Harmageddon commented May 10, 2020

This PR is based on a feature request brought up by user fire67 on the German forum: https://forum.joomla.de/thread/11738-plugin-inhalt-datenschutzerkl%C3%A4rung/

Apparently, some page builders like SP PageBuilder don't register their "articles" as real Joomla! articles. If you create your privacy policy using such a page builder (might apply to some CCK extensions, too), you can't choose it in the privacy consent plugin, because there, only articles are available.

Summary of Changes

This PR adds a menu item parameter to plg_system_privacyconsent and plg_content_confirmconsent that can be used instead of the article parameter, and a parameter to choose between the two.

Testing Instructions

At least one test on a multilingual site would be very welcome. I replicated the mechanism that chooses the associated article in the respective language, for menu items. It works on my small local setup. But as I'm no expert in multilingual sites, I'd be happy if someone with more expertise could check if it works as expected.

  1. Enable the Plugin "System - Privacy Consent".
  2. Select an article for your privacy policy.
  3. In frontend, navigate to the registration form and make sure that the modal box for the privacy policy works as expected (in all languages).
  4. Apply this PR.
  5. Make sure everything still works like before.
  6. In the plugin config, switch "Privacy Type" to "Menu Item" and select a menu item. To make sure that there is a difference, maybe select a category blog or something else, but not the menu item for the privacy article you used before.
  7. Repeat step 3.
  8. Switch the type back to "Article" and make sure it works as before.
  9. Repeat everything from 1 to 8 with the plugin "Content - Confirm Consent" and the contact form.

You might want to check if the privacy links work for all settings with SEO URLs enabled and disabled.

Documentation Changes Required

Should be documented at https://docs.joomla.org/J3.x:Privacy (or the J4 version of that?).

Additional Notes

I noticed that the full $article object including link, alias, id, catid and language, is passed to the layout, but only the link is used there (at least in core). Should I do the same for the menu item, so overrides could access further information or leave it as it is?

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels May 10, 2020
@Harmageddon
Copy link
Contributor Author

Apparently, this is not the only privacy consent plugin. I'm going to provide an analogous implementation for the content plugin for contact forms tomorrow.
Thanks to @ChristineWk for pointing me there. ;-)

@Harmageddon
Copy link
Contributor Author

Harmageddon commented May 11, 2020

Updated with the according changes to Content - Confirm Consent. Testing instructions are now updated, too. It's ready to test now! ;-)

Stupid question here: Why do we have two plugins for almost the same thing, including two similar, but still different implementations for the form field that displays the modal?

Co-authored-by: Quy <quy@fluxbb.org>
Co-authored-by: SharkyKZ <sharkykz@gmail.com>
Co-authored-by: Quy <quy@fluxbb.org>
@ChristineWk
Copy link

@Harmageddon
Some test-results:

a) System - Privacy Consent (registration form) > there is no switch to "Menu Item".

b) Content - Confirm Consent (contact form) > successful. See screenshots

content_confirm_consent

content_confirm_consent_menue

@ChristineWk
Copy link

Now I got a) also. It wasn't there before.

system_privacy_consent_frontend

@ChristineWk
Copy link

I have tested this item ✅ successfully on ea4abe9

but not tested multilingual site


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@carcam
Copy link

carcam commented May 21, 2020

I have tested this item ✅ successfully on ea4abe9

I have tetsed this successfully:

  1. Added German language to the site
  2. Added German menu item for blog: "Das Blog" associated with "Blog" menu item
  3. Enable plugin "privacy - consent" and changed to "Privacy Type -> menu item"
  4. Set Privacy Menu Item to "Blog" (English menu item)
  5. When checked the privacy consent link I see the menu item for English blog when in English language and the link for Das Blog when in German language
    This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 21, 2020
Co-authored-by: Tobias Zulauf <zero-24@users.noreply.github.com>
@Harmageddon Harmageddon requested a review from zero-24 May 24, 2020 13:19
@richard67
Copy link
Member

Previous test results are still valid, because the change after the test was only in doc blocks. I've restored the test results in the tracker so the count there and here is correct again.

@richard67
Copy link
Member

RTC status is also still valid, see my previous comment.

@Quy Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jun 6, 2020
@Harmageddon Harmageddon requested a review from Quy June 7, 2020 11:06
@Quy Quy removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jun 7, 2020
@ChristineWk
Copy link

I have tested this item ✅ successfully on ae56966

Code Review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@ceford
Copy link
Contributor

ceford commented Jun 30, 2020

I did have the menu item SomeOtherArticleMenuItem set to All. Setting that to English I get the English Privacy message in all languages as you described. And the direct menu link only appears in the English page. Back to All languages and the direct menu link leads to the Not Found page in French and German. The Article itself is set to English.

So this is really a problem of making sure the Article and Menu Item have compatible language settings and is nothing to do with Privacy Policy.

@Harmageddon
Copy link
Contributor Author

@ceford Thank you for the clarification! So is this a successful test from your side or do you see any remaining issues with this PR?

@ceford
Copy link
Contributor

ceford commented Jul 1, 2020

Yes - Pass.

@richard67
Copy link
Member

@ceford Does your previous mean you have tested this PR completely with success? If so, please mark the test result again at the issue tracker because your previous result has been reset with the last commit.

@richard67
Copy link
Member

@zero-24 GH still shows changes requested by your review, but the review comments have been solved. Maybe you can just pass again a neutral review to reset this?

@zero-24 zero-24 removed their request for review July 1, 2020 11:47
@zero-24
Copy link
Contributor

zero-24 commented Jul 1, 2020

Should be gone now

@richard67
Copy link
Member

@zero-24 No, is still there in the box at the bottom.

Copy link
Contributor

@zero-24 zero-24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be gone now

@ceford
Copy link
Contributor

ceford commented Jul 1, 2020

I have tested this item 🔴 unsuccessfully on e56f475

I selected a menu item for the Privacy Policy and that worked fine. I went back and selected Privacy Type: Article; Privacy Article: Select brought up a list to select from. But the links are dead and I see javascript:void() at the bottom left of the screen. I am now on Beta-3 Dev. Is there a Javascript issue?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@ChristineWk
Copy link

#29897


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@Harmageddon
Copy link
Contributor Author

The bug you mentioned has been fixed in #29860, merged a few minutes ago. As mentioned there, it was introduced in #29464, which has been merged into 4.0-dev just before the release of beta2. So the bug came with beta2 and was fixed after the release of beta2. I'd therefore ask you to test with beta1 or a current version (nightly starting from tomorrow, Fr 03 July or git from right now).

Thank you @ChristineWk for pointing this out!

@ceford
Copy link
Contributor

ceford commented Jul 3, 2020

If I pick an article in English that has no associations in French and German then I get that Article in English in the Privacy Policy dialog in all three languages. Good!

If I pick the menu item that points to that article, also without associations in French and German, then I only get that Article in English in the Privacy Policy dialog. In French and German I get the dialog with a 404 not found page. And it is a big page with all of the modules in left and right columns. I accept that that could be left as a User education problem but it would be better if there were a technical solution.

Otherwise the normal behaviour works fine.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@Harmageddon
Copy link
Contributor Author

Like I mentioned above, the menu item should be set to English in your second case as well. Is this the case?

@ceford
Copy link
Contributor

ceford commented Jul 3, 2020

Yes, it works properly with the menu item set to English.

One more question: why is this plugin in the System group instead of the Privacy group?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@ceford
Copy link
Contributor

ceford commented Jul 3, 2020

I have tested this item ✅ successfully on e56f475

With the small proviso I mentioned previously I think this is OK. I have documented the changes in the Help screen:

https://docs.joomla.org/Chunk4x:Extensions_Plugin_Manager_Edit_System_Group#System_-_Privacy_Consent

And there drawn attention to multilingual associations.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@Harmageddon
Copy link
Contributor Author

Thank you for your tests and the effort on the documentation! :-)

@ChristineWk
Copy link

I have tested this item ✅ successfully on dc39f39

Version 4.0.0-beta3-dev Jul6th2020 new Installation


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@jwaisner
Copy link
Member

jwaisner commented Jul 6, 2020

I have tested this item ✅ successfully on dc39f39


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@jwaisner
Copy link
Member

jwaisner commented Jul 6, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29024.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 6, 2020
@richard67
Copy link
Member

@wilsonge Anything which keeps this RTC PR from being merged? Does it count as "new feauture" and so has to go into 4.1?

@rdeutz rdeutz added this to the Joomla 4.0 milestone Oct 16, 2020
@rdeutz rdeutz merged commit 5813af5 into joomla:4.0-dev Oct 16, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators

Projects

None yet

Development

Successfully merging this pull request may close these issues.