Change responses export columns #28678

CGastrell · 2023-01-31T22:25:50Z

Second iteration on responses export functionality: reorder and prioritize columns.

Fixes #28677

Proposed changes:

Used backend column naming to group and translate. The column order should now be as described here: pcsBup-K4-p2

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

pcsBup-K4-p2

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Use a site with some responses or create one, publish a post with a form and fill it a couple of times. Use multiple/different forms for fully testing the behavior.
Go to "Feedback" menu on wp-admin and use the "Export" button. Both CSV or exporting to Google Sheets should show columns:

Title
Source
Date
[ all columns/values filled on submissions ]
Consent
IP Address

github-actions · 2023-01-31T22:26:50Z

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run

bin/jetpack-downloader test jetpack change/responses-export-columns

to get started. More details: p9dueE-5Nn-p2

github-actions · 2023-01-31T22:27:04Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
⚠️ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: February 7, 2023.
Scheduled code freeze: January 31, 2023.

digitalwaveride · 2023-02-01T14:06:05Z

Nice Christian!
The general "three categories" structure of the columns worked for me as advertised. 👍

Two things I noticed, and we might want to address this in a different PR if useful:

Date columns vs. Date Picker

If you look at the screenshots below, I am using two different forms on two different posts.
The post /number 1 includes a date picker block (using the default label of the block). On the WP-Admin view the data for this date block shows up as expected.
On the export though, the date picker block for one of the responses seems to overwrite the "date column", which is supposed to be the general metadata date column.
Weirdly enough the other "/number 1" form response is not overwriting this column. 🤷
An expected "date column" for the date picker block is missing for both.

The order of the columns of the "middle category" `[ all columns/values filled on submissions ]` seems to not take the structure of the form into account.

It is different from the structure in the WP-Admin view.
If you compare the /second form in WP-Admin with the Export, the "Website" column comes before the "Please rate our website" ... and there is the known issue of a "Comment" column is containing the content of "How could we improve" ... which should be it's own column.

EXPORT:

WPADMIN VIEW:

CGastrell · 2023-02-01T14:27:28Z

the "Website" column comes before the "Please rate our website" ... and there is the known issue of a "Comment" column is containing the content of "How could we improve" ... which should be it's own column.

I think this is a known issue and, sadly, not the goal of this change to fix it. The are some resolutions on backend to "guess" what is a comment and in what order the fields are turned into columns. There's another "guess" as to how to make some of those fields, through different forms, match on the same column. These all deserves a separate PR to try and fix it.

…extra data. Add dictionary with translations for them

…ps and order achieved

CGastrell · 2023-02-01T19:48:56Z

Any hints on how to fix this

projects/packages/forms/src/contact-form/class-contact-form-plugin.php

jeherve · 2023-02-03T11:39:11Z

projects/packages/forms/src/contact-form/class-contact-form-plugin.php

+					$md['-6_source'] .= $parsed['path'];
+				}
+				if ( $parsed && ! empty( $parsed['query'] ) ) {
+					$md['-6_source'] .= '?' . $parsed['query'];


Would we need to sanitize this to avoid issues?

Mmm... might be, it's something we set internally with esc_url( get_permalink( get_the_ID() ) ). I'd say it's safe enough, but maybe it's worth cleansing the parsed parts?

jeherve · 2023-02-03T11:40:51Z

projects/packages/forms/src/contact-form/class-contact-form-plugin.php

 				 */
-				$renamed_field = preg_replace( '/^(\d{1,2}_)/', '', $single_field_name );
+				$renamed_field = preg_replace( '/^(-?\d{1,2}_)/', '', $renamed_field );


I wonder if we should expand the comment a bit, for our future selves? to explain why we need to care about a possible - here?

Added some more explanation on how/why prefixes could be negative values and a reference to the sorting function. Hope that's enough

Co-authored-by: Jeremy Herve <[email protected]>

jeherve

Looking good. I ran into the same ordering issues with the csv, but since this is already already tracked it should be good to merge.

…9448) Co-authored-by: oskosk <[email protected]>

CGastrell self-assigned this Jan 31, 2023

CGastrell requested a review from ice9js January 31, 2023 22:26

CGastrell added [Feature] Contact Form [Status] Needs Team Review Obsolete. Use Needs Review instead. [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ labels Jan 31, 2023

CGastrell marked this pull request as ready for review January 31, 2023 22:26

github-actions bot added the [Status] In Progress label Jan 31, 2023

CGastrell removed the [Status] In Progress label Jan 31, 2023

github-actions bot added the [Status] In Progress label Jan 31, 2023

github-actions bot added [Status] Needs Test Review Unit Tests labels Feb 1, 2023

CGastrell removed [Status] In Progress [Status] Needs Test Review labels Feb 1, 2023

CGastrell added 5 commits February 1, 2023 15:26

[not verified] use hardcoded and prefixed column names for responses …

daa944c

…extra data. Add dictionary with translations for them

[not verified] use new well known column names on export: column grou…

5effd7e

…ps and order achieved

[not verified] changelog

ccf3dc9

[not verified] update tests to expect new extra columns on data

b32294c

[not verified] replace str_starts_with with strpos (php8 compat)

72bd228

CGastrell force-pushed the change/responses-export-columns branch from e30d82b to 72bd228 Compare February 1, 2023 18:26

github-actions bot added the [Status] Needs Test Review label Feb 1, 2023

port same changes to new package setup

cc51872

github-actions bot added the [Package] Forms label Feb 1, 2023

CGastrell removed the [Status] Needs Test Review label Feb 1, 2023

CGastrell requested a review from digitalwaveride February 2, 2023 13:57

CGastrell removed the Unit Tests label Feb 2, 2023

CGastrell removed the [Status] Needs Team Review Obsolete. Use Needs Review instead. label Feb 2, 2023

Merge branch 'trunk' into change/responses-export-columns

6c2e506

CGastrell removed the [Status] Needs Test Review label Feb 2, 2023

github-actions bot added the [Status] Needs Test Review label Feb 2, 2023

CGastrell removed the [Status] Needs Test Review label Feb 2, 2023

jeherve requested changes Feb 3, 2023

View reviewed changes

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Feb 3, 2023

github-actions bot added the [Status] Needs Test Review label Feb 3, 2023

Drop redundant isset/empty combo

2b8c60e

Co-authored-by: Jeremy Herve <[email protected]>

CGastrell dismissed stale reviews from ice9js and digitalwaveride via 2b8c60e February 3, 2023 14:22

reminder for future selves on the numeral prefix regex/use/condition

f8860a7

CGastrell requested a review from jeherve February 3, 2023 15:09

CGastrell added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] Needs Test Review labels Feb 3, 2023

github-actions bot added the [Status] Needs Test Review label Feb 3, 2023

jeherve approved these changes Feb 3, 2023

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 3, 2023

CGastrell merged commit 3e554bd into trunk Feb 3, 2023

CGastrell deleted the change/responses-export-columns branch February 3, 2023 16:29

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 3, 2023

github-actions bot added this to the jetpack/11.8 milestone Feb 3, 2023

iandunn mentioned this pull request Mar 10, 2023

Export assigns data to wrong row when fields reordered #29429

Closed

CGastrell mentioned this pull request Mar 13, 2023

Fix export bug: missing data/columns (A sort of Revert of #28678) #29448

Merged

3 tasks

sdixon194 pushed a commit that referenced this pull request Mar 14, 2023

Fix export bug: missing data/columns (A sort of Revert of #28678) (#2…

3f18cbb

…9448) Co-authored-by: oskosk <[email protected]>

sdixon194 pushed a commit that referenced this pull request Mar 14, 2023

Fix export bug: missing data/columns (A sort of Revert of #28678) (#2…

223fe51

…9448) Co-authored-by: oskosk <[email protected]>

jeherve added the [Tests] Includes Tests label Apr 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Change responses export columns #28678

Change responses export columns #28678

Uh oh!

CGastrell commented Jan 31, 2023

Uh oh!

github-actions bot commented Jan 31, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Jan 31, 2023 •

edited

Loading

Uh oh!

digitalwaveride commented Feb 1, 2023

Uh oh!

CGastrell commented Feb 1, 2023

Uh oh!

CGastrell commented Feb 1, 2023

Uh oh!

Uh oh!

jeherve Feb 3, 2023

Uh oh!

CGastrell Feb 3, 2023

Uh oh!

jeherve Feb 3, 2023

Uh oh!

CGastrell Feb 3, 2023

Uh oh!

jeherve left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Change responses export columns #28678

Change responses export columns #28678

Uh oh!

Conversation

CGastrell commented Jan 31, 2023

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Uh oh!

github-actions bot commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

digitalwaveride commented Feb 1, 2023

Date columns vs. Date Picker

The order of the columns of the "middle category" [ all columns/values filled on submissions ] seems to not take the structure of the form into account.

Uh oh!

CGastrell commented Feb 1, 2023

Uh oh!

CGastrell commented Feb 1, 2023

Uh oh!

Uh oh!

jeherve Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

CGastrell Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

jeherve Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

CGastrell Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Jan 31, 2023 •

edited

Loading

github-actions bot commented Jan 31, 2023 •

edited

Loading

The order of the columns of the "middle category" `[ all columns/values filled on submissions ]` seems to not take the structure of the form into account.