Personal Data Export Report: A way to display the group count

[birgire]

It would be informative to know the number of items in a given ("countable") group.

This could be an option for each group, because not all groups are "countable".

The Comments and Media groups could e.g. benefit from this.

Similar was explored in the early stages of the Site Health admin page #39165.

See e.g. the screenshot for that ticket.

[birgire]

group-count.jpg​ is an example showing the number of comments within the core Comments group.

[garrett-eclipse]

Thanks @birgire this would make sense for repeating groups of items such as Comments.

[birgire]

46895.diff​ is a first pass.

It adds a support for the group_display_count boolean key, that determines if the corresponding group count is displayed.

It's displayed for the Comments and Media groups.

[birgire]

ps: It was helpful to have the implementation of the group_description as a comparison :-)

[birgire]

46895-2.diff​ uses instead the group_count group key and introduces two test cases:

• test_group_html_generation_when_group_count_displayed()
• test_group_html_generation_when_group_count_not_displayed()

in the Tests_Privacy_WpPrivacyGeneratePersonalDataExportGroupHtml class.

[birgire]

I think there's no need to display the group count for a single item.

Patch updated in 46895-3.diff​ with a corresponding test case.

[garrett-eclipse]

Thanks @birgire testing this and running the unit tests everything went well, the patch still applies cleanly and all the unit tests run properly.

I also agree with your statement "there's no need to display the group count for a single item" and it begs the question if the group_count (previously group_display_count) boolean is even necessary here. Can we just not display the count if more than one for all groups. I don't believe there's a case where a group has multiple items and the count wouldn't be useful. And by requiring a boolean set would mean a dev-note would be required and the third-party tools that use the exporter will need to be aware of the boolean and update their implementation to utilize it, by simply adding count when >1 all third-party tools automatically are accounted for.

If there are cases where group count isn't desired on multi-item groups another option is a disable boolean which would still require a dev note but then third-party tools will get the count unless they opt-out.

Sidenote - If the boolean makes more sense than just using count>1 I would suggest going back to a more descriptive one like in the initial diff when it was group_display_count as reading $group_count = (bool) $export_datum['group_count']; while I was reviewing had me double-check as on first glance it seemed like the actual group count of items was being cast as a boolean.

Let me know your thoughts on dropping the boolean, I feel it simplifies things and removes the need to educate plugins, etc. via a dev-note.

All the best

[pputzer]

Replying to garrett-eclipse:

I also agree with your statement "there's no need to display the group count for a single item" and it begs the question if the group_count (previously group_display_count) boolean is even necessary here. Can we just not display the count if more than one for all groups. I don't believe there's a case where a group has multiple items and the count wouldn't be useful. And by requiring a boolean set would mean a dev-note would be required and the third-party tools that use the exporter will need to be aware of the boolean and update their implementation to utilize it, by simply adding count when >1 all third-party tools automatically are accounted for.

I think it might be useful to see at a glance which groups could have more items (but don't), so if we decide to go with the explicit enabling property, I think "Comments (1)" should be displayed.

As for the other approach, automatically adding the group count if item count > 1 sounds very elegant architecturally. I was hesitant for a moment for any BC issues, but since this would only affect the HTML output, it should not have an effect on plugins that add to existing Core groups in their data exporters.

[garrett-eclipse]

Updated patch to base count display on count > 1 removing boolean and need for dev-notes

[garrett-eclipse]

Attached example export with multiple comments, one media and multiple custom erasers this one from WP Mail Logging

[garrett-eclipse]

Thanks @pputzer I've refreshed the patch 46895.4.diff to use the count of items to toggle display over-relying on a boolean. This means plugins need no change as illustrated in example export https://core.trac.wordpress.org/attachment/ticket/46895/index%204.html which should multiple Comments, a single Media and multiple mails form WP Mail Logging as an example to illustrate that 3rd parties are supported without requiring a change.

Moving this forward to commit as I've also addressed unit test issues and removed the now unneeded one that did a check on the boolean.

@birgire any final thoughts, would love you to test/review.

[garrett-eclipse]

The export html didn't render in Trac as I wanted so this will better illustrate the export and counts

[birgire]

Thanks for your feedback @garrett-eclipse and @pputzer

I like the simplifications in 46895.4.diff​, the screenshot and HTML file are also helpful. Thanks Garret

It seems to work as expected for the Comments and Media sections.

The Privacy test group run successfully.

So this looks good to me.

[SergeyBiryukov]

In 46209:

Privacy: Display group items count in the personal data export file if there's more than one item in the group.

Props birgire, garrett-eclipse, pputzer.
Fixes #46895.