Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#44005 closed enhancement (fixed)

Introduce Privacy Policy Page helpers; is_privacy_policy() query function, privacy-policy body class, privacy-policy.php template and .menu-item-privacy-policy menu item class

Reported by: clorith's profile Clorith Owned by: garrett-eclipse's profile garrett-eclipse
Milestone: 5.2 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch has-unit-tests has-dev-note
Focuses: template Cc:

Description

With the introduction of a privacy generator (not exactly what it is, but just roll with it), it would be beneficial to include something in the template hierarchy for this as well.

Looking at many large sites and services, their designs are often created to continue drawing focus to new things, leaving the privacy text as "just another page". These same sites have often opted to separate their privacy pages, giving them a simpler and non-intrusive design focusing solely on the text at hand.

This could be achieved with a page template, although creating a page template for a single purpose makes little sense, but given that we're creating tools around this in the first place it is a natural evolution to also provide a separate theme file for this kind of content.

Attachments (6)

44005.diff (2.5 KB) - added by xkon 5 years ago.
Adds is_privacy_policy() and privacy_policy body class.
44005.1.diff (7.1 KB) - added by garrett-eclipse 5 years ago.
Expanded patch to introduce privacy-policy.php template as well as further query coverage for pagename and page_id queries
44005.2.diff (7.1 KB) - added by garrett-eclipse 5 years ago.
Fixed the missed version on get_privacy_policy_template
44005-3.diff (12.1 KB) - added by birgire 5 years ago.
44005.4.diff (12.8 KB) - added by garrett-eclipse 5 years ago.
Minor additions/adjustments to the unit tests started by @birgire
44005.5.diff (15.8 KB) - added by garrett-eclipse 5 years ago.
Added .menu-item-privacy-policy to menu items when they match the wp_page_for_privacy_policy option, also added tests for the new class.

Download all attachments as: .zip

Change History (43)

#1 @danieliser
6 years ago

Maybe make it more generic. Something like a "Policy Page" template or "Text Only" where it is more generic. I imagine the same could be used for Terms & Conditions, Privacy Policy, Disclosures, Licenses etc.

#2 @Clorith
6 years ago

The idea is to hook it up with the content from the privacy setting, adding too many formats here will be confusing and bloat the template hierarchy (especially since only privacy is a format we "officially support" in core, for lack of a better term)

#3 @xkon
6 years ago

If you think that you should add more things than only just a text to explain to the users what's going on in that privacy policy and on their data as it's required to be explained in the simplest form ever ( for example icons with small explanations check how eBay is doing it for reference ) this makes total sense and opens up various possibilities for theme authors & admins to be even more helpful with various ways towards their users. Just think of premade designs from themes that could assist on this. I like it!

This ticket was mentioned in Slack in #gdpr-compliance by allendav. View the logs.


6 years ago

#5 @desrosj
6 years ago

  • Component changed from Themes to Privacy

Moving to the new Privacy component.

#6 @desrosj
6 years ago

  • Focuses privacy added
  • Keywords needs-patch added
  • Version set to 4.9.6

I like this idea. This will also most likely benefit from an accompanying is_privacy_policy() conditional. I started a patch for this but need to follow up and finish it.

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

#8 @desrosj
6 years ago

  • Milestone changed from Awaiting Review to Future Release

#9 @desrosj
6 years ago

  • Keywords gdpr removed

Removing the GDPR keyword. This has been replaced by the new Privacy component and privacy focuses in Trac.

@xkon
5 years ago

Adds is_privacy_policy() and privacy_policy body class.

#10 @xkon
5 years ago

  • Keywords 2nd-opinion needs-testing added

44005.diff handles one part of this ticket by adding the is_privacy_policy() check to be used similarly as is_home() etc. It also adds the privacy-policy body class is the policy page is viewed.

@garrett-eclipse
5 years ago

Expanded patch to introduce privacy-policy.php template as well as further query coverage for pagename and page_id queries

#11 @garrett-eclipse
5 years ago

  • Keywords needs-unit-tests dev-feedback added

Thanks @xkon for getting this started.

I've expanded on your patch mainly to introduce the privacy-policy.php to the template hierarchy but also to cover the pagename/page_id queries.

Full list of changes;

  • Updating comment verbiage to 'Privacy Policy page' to match convention in core.
  • Updated @since to semantic versioning 5.2.0
  • Expanded on docblocks to match level of verbosity as the front_page and home related functions.
  • Moved functions to below is_home/front_page methods to keep all that relate to option settings grouped.
  • Updated is_privacy_policy logic to drop get_queried_object in favour of using is_page with the option value to match is_front_page logic.
  • Added is_privacy_policy declaration to the init_query_flags funciton in class-wp-query.php
  • Added check when queryvar is pagename to set is_privacy_policy to true if matching current option for wp_page_for_privacy_policy.
  • Added check when queryvar is page_id to set is_privacy_policy to true if matching current option for wp_page_for_privacy_policy.
  • Created get_privacy_policy_template to map the privacy-policy.php template.
  • Added privacypolicy to the list of possible values for the $type on the {$type}_template_hierarchy and {$type}_template filters

I've tested this successfully confirming that the privacy-policy body class is applied, the is_privacy_policy() function and the privacy-policy.php template all function properly only on the page set as the Privacy Policy page and no others. Works when in both draft and public states.

Applying needs-unit-tests as next steps and would love additional testing and review.

@garrett-eclipse
5 years ago

Fixed the missed version on get_privacy_policy_template

#12 @garrett-eclipse
5 years ago

  • Owner set to garrett-eclipse
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-privacy by xkon. View the logs.


5 years ago

#14 @xkon
5 years ago

  • Keywords has-patch added; needs-patch 2nd-opinion dev-feedback removed

@garrett-eclipse thanks for the update and getting this further down the road.

Patch applies fine on my end and everything works as expected on both the check + template file.

I think we can get this in line for 5.2 if we manage the unit-tests as well. What do you think? :)

#15 @xkon
5 years ago

  • Milestone changed from Future Release to 5.2

@birgire
5 years ago

#16 follow-up: @birgire
5 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

44005-3.diff adds some unit-tests (first draft) for

  • the is_privacy_policy() template tag.
  • the privacy-policy body class.
  • the support of a privacy policy template in the template hierarchy.

I also adjusted:

Tests_Url_GetPrivacyPolicyUrl::test_get_privacy_policy_url_should_return_valid_url_when_policy_page_set()

to avoid mismatch between the expected http://test.dev/test-dev-privacy-policy/ and actual http://test.dev/?page_id=31 that surfaced when testing the patch.

ps: we could then consider a test with an actual privacy policy template file.

Last edited 5 years ago by birgire (previous) (diff)

@garrett-eclipse
5 years ago

Minor additions/adjustments to the unit tests started by @birgire

#17 in reply to: ↑ 16 @garrett-eclipse
5 years ago

Thanks @birgire for getting the unit tests started here.

In 44005.4.diff I've made some minor modifications;

  • Added a new line before the test_is_privacy_policy function.
  • Added privacy-policy.php to the assertTemplateHierarchy test in test_privacy_template_hierarchy
  • Added 'privacy_policy' => 'is_privacy_policy' to the test_single_template_hierarchy_for_post test
  • Added $this->assertEquals( get_privacy_policy_template(), get_query_template( 'privacy_policy', array( 'privacy-policy.php' ) ) ); to the test_switch_theme

These are all passing in my grunt test and phpunit tests.

Replying to birgire:

ps: we could then consider a test with an actual privacy policy template file.

*This makes sense to me although not sure how to go about do you want to take the lead on that @birgire.

@garrett-eclipse
5 years ago

Added .menu-item-privacy-policy to menu items when they match the wp_page_for_privacy_policy option, also added tests for the new class.

#18 @garrett-eclipse
5 years ago

  • Keywords dev-feedback added
  • Summary changed from Privacy template file to Introduce Privacy Policy Page helpers; is_privacy_policy() query function, privacy-policy body class, privacy-policy.php template and .menu-item-privacy-policy menu item class

@birgire & @xkon while going through this I realized we should add coverage for the menu item when it's the Privacy Policy Page.

I've uploaded 44005.5.diff to add the .menu-item-privacy-policy class to the menu item if it's the current Privacy Policy Page.

I also added two unit tests to follow the front_page versions of the menu item class tests.
test_no_privacy_policy_class_applied - Confirms the class isn't added when not wanted
test_class_applied_to_privacy_policy_page_item - Confirms the class is added when the Privacy Policy Page is the menu item.

With this addition this ticket now introduces;

  • is_privacy_policy() query function
  • privacy-policy body class
  • privacy-policy.php template file
  • .menu-item-privacy-policy menu item class

*Adjusted title accordingly.

The tests are passing successfully for me and all the above is functioning properly in manual testing. Would love some additional testing and feedback and feel we can move this forward to get committer feedback (added dev-feedback for that).

To me this seems to warrant possibly needs-docs, needs-codex and needs-dev-note keywords as it introduces new classes, template hierarchy and query support.

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

This ticket was mentioned in Slack in #core by garrett-eclipse. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

#23 @xkon
5 years ago

This works fine for me @garrett-eclipse , good call on adding the menu class. Manual testing + tests went fine as well.

@desrosj could you take a look whenever possible to see if we're missing anything code-wise, so we can start drafting some docs to help there as well possibly? Thanks! :)

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

#25 @birgire
5 years ago

@garrett-eclipse good catch regarding the menu class.

@garrett-eclipse & @xkon: I did some manual testing on 44005.5.diff and verified few things:

  • The privacy-policy.php template file, in the current theme's directory, is loaded for the Privacy Policy page as expected.
  • The privacy-policy body class shows up on the Privacy Policy page, as expected.
  • The is_privacy_policy(), is_page() and is_singular() are true within the privacy-policy.php template file, as expected.
  • The menu-item-privacy-policy nav-menu class when the Privacy Policy page is added to the nav-menu, as expected.

ps: as a part of the doc part, it would be nice to have

https://developer.wordpress.org/themes/basics/template-hierarchy

updated after this has landed.

This ticket was mentioned in Slack in #core by garrett-eclipse. View the logs.


5 years ago

#27 @garrett-eclipse
5 years ago

  • Keywords commit added; needs-testing dev-feedback removed

This has been tested by several maintainers and so far has positive feedback so am moving it to commit. We'll work on dev-notes and docs during beta.

#28 @garrett-eclipse
5 years ago

  • Status changed from assigned to accepted

#29 @SergeyBiryukov
5 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 44966:

Privacy: Introduce Privacy Policy page helpers:

  • is_privacy_policy() template tag
  • privacy-policy.php template
  • .privacy-policy body class
  • .menu-item-privacy-policy menu item class

Props garrett-eclipse, birgire, xkon, Clorith.
Fixes #44005.

#30 @SergeyBiryukov
5 years ago

In 44968:

PHPCS: Remove extra spaces added in [44966].

See #44005.

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

#32 @garrett-eclipse
5 years ago

  • Keywords needs-dev-note added

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

#35 @SergeyBiryukov
5 years ago

In 45251:

Docs: Correct @return description for get_privacy_policy_template().

Props tmatsuur.
Fixes #46989. See #44005.

#36 @desrosj
5 years ago

  • Keywords has-dev-note added; commit needs-dev-note removed

#37 @garrett-eclipse
4 years ago

  • Focuses privacy removed

Dropping privacy focus as it's already in the Privacy component.

Note: See TracTickets for help on using tickets.