Add survey when disabling new experience #36544

octaedro · 2023-01-22T18:24:07Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR adds a survey when the user disables the new product management experience. Is based on this PR.

It will be visible every time the user disables the new experience.

This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

Go to a new product using the new product management experience and disable it. Use the bottom bar button or select the "use the classic editor" option at the top bar.

Make sure you have tracks enabled and also outputted in the console: localStorage.setItem( 'debug', 'wc-admin:*' );
A survey will be shown, and the Send feedback button should be disabled until an option is selected or a text is added to the text area.
Send feedback and verify that the checked options and comment are being tracked.
A notice should be shown in the old experience saying:

6. Verify that the `Skip` button and the `X` to close the modal work as expected.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

github-actions · 2023-01-22T18:48:00Z

Test Results Summary

Commit SHA: d34541e

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 53s
E2E Tests	189	0	0	6	0	195	15m 46s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

joshuatf

Very nicely done! Added a few minor comments and question around how we show notices via the data store.

joshuatf · 2023-01-23T18:48:47Z

...ocommerce-admin/client/customer-effort-score-tracks/product-mvp-feedback-modal-container.tsx

+		};
+	} );
+
+	const classEditorUrl = values.id


Suggested change

const classEditorUrl = values.id

const classicEditorUrl = values.id

joshuatf · 2023-01-23T18:56:00Z

plugins/woocommerce-admin/client/customer-effort-score-tracks/data/reducer.js

@@ -62,6 +63,16 @@ const reducer = ( state = DEFAULT_STATE, action ) => {
 				...state,
 				queue: [ ...state.queue, newTrack ],
 			};
+		case TYPES.SHOW_PRODUCT_MVP_FEEDBACK_MODAL:


I won't let it block this PR if this is the easiest way to get this working, but I'm not so sure about using data stores to show/hide modals. I wonder what the original reasons for doing this were.

If these are not needed outside of the app, I feel context might have been a better fit.

If data stores are necessary, I wonder if we could have more generic getters/setters. E.g., showModal( modalName ), hideModal( modalName ), isModalVisible( modalName ), etc.

I concur with you. I followed the pattern we were using to not add more complexity to this part, but I think that it would be a really nice refactor to handle the visibility of the modals differently.

joshuatf · 2023-01-23T19:18:51Z

...ocommerce-admin/client/customer-effort-score-tracks/product-mvp-feedback-modal-container.tsx

+	const recordScore = ( checked: string[], comments: string ) => {
+		recordEvent( 'product_mvp_feedback', {
+			action: 'disable',
+			checked: checked.join( ',' ),


I was under the impression that tracks handled converting arrays automatically. Is this required?

Nice catch! I fixed it in the commit a01ffe6

joshuatf · 2023-01-23T19:19:47Z

plugins/woocommerce/src/Admin/Features/NewProductManagementExperience.php

@@ -28,6 +29,16 @@ public function __construct() {

 		add_action( 'admin_enqueue_scripts', array( $this, 'enqueue_styles' ) );
 		add_action( 'get_edit_post_link', array( $this, 'update_edit_product_link' ), 10, 2 );
+		if ( isset( $_GET['new-product-experience-disabled'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
+			TransientNotices::add(


Should we redirect removing new-product-experience-disabled from the URL so that this message does not appear if the user refreshes?

Good idea! I add it in the commit 7e43808

octaedro · 2023-01-23T22:47:02Z

Hey @joshuatf, I just addressed the suggestions you made.
Could you take another look at this PR?

joshuatf

Added a couple more comments around the fixes, but hopefully pretty small fixes. Let me know if you have any questions 😄

joshuatf · 2023-01-23T23:22:18Z

plugins/woocommerce/src/Admin/Features/NewProductManagementExperience.php

@@ -22,6 +23,24 @@ class NewProductManagementExperience {
 	 * Constructor
 	 */
 	public function __construct() {
+		$new_product_experience_param = 'new-product-experience-disabled';
+		if ( isset( $_GET[ $new_product_experience_param ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
+			$url  = isset( $_SERVER['HTTPS'] ) && 'on' === $_SERVER['HTTPS'] ? 'https://' : 'http://';


I think we can just use remove_query_arg( 'new-product-experience-disabled', $url ) to remove the arg here.

https://developer.wordpress.org/reference/functions/remove_query_arg/

joshuatf · 2023-01-23T23:23:22Z

plugins/woocommerce/src/Admin/Features/NewProductManagementExperience.php

+			$url  = preg_replace( '/(&|\?)' . preg_quote( $new_product_experience_param ) . '=[^&]*$/', '', $url ); // phpcs:ignore WordPress.PHP.PregQuoteDelimiter.Missing
+			$url  = preg_replace( '/(&|\?)' . preg_quote( $new_product_experience_param ) . '=[^&]*&/', '$1', $url ); // phpcs:ignore WordPress.PHP.PregQuoteDelimiter.Missing
+
+			wp_safe_redirect( $url );


Might be a tiny bit safer to move this after the TransientNotices below and exit;.

joshuatf · 2023-01-23T23:25:19Z

plugins/woocommerce/src/Admin/Features/NewProductManagementExperience.php

@@ -22,6 +23,24 @@ class NewProductManagementExperience {
 	 * Constructor
 	 */
 	public function __construct() {
+		$new_product_experience_param = 'new-product-experience-disabled';


Might be nice to move this logic out of the constructor and into a method. Something like $this->maybe_show_disabled_notice().

# Conflicts: # plugins/woocommerce/src/Admin/Features/NewProductManagementExperience.php

joshuatf

Testing well and code looks great! Thanks, Fernando! 🚢

* Add customer-score-tracks data * Add callback after disabling new exp * Add TransientNotice after filling out the survey # Conflicts: # plugins/woocommerce/src/Admin/Features/NewProductManagementExperience.php * Remove comments * Remove NEW_PRODUCT_MANAGEMENT_FEEDBACK * Add changelog * Rename const * Remove queryParam after showing notice * Fix lint * Fix lint 2.0 * Remove empty line * Refactor `maybe_show_disabled_notice` * Fix lint 3.0 Co-authored-by: Fernando Marichal <contacto@fernandomarichal.com>

github-actions bot added focus: react admin package: @woocommerce/customer-effort-score issues related to @woocommerce/customer-effort-score plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Jan 22, 2023

octaedro changed the title ~~Add/survey after disabling new exp~~ Add survey when disabling new experience Jan 22, 2023

octaedro changed the base branch from trunk to add/feedback_modal_and_product_mvp_feedback_modal_components January 22, 2023 19:20

octaedro mentioned this pull request Jan 22, 2023

Add feedback modal and product mvp feedback modal components #36532

Merged

8 tasks

octaedro marked this pull request as ready for review January 22, 2023 21:30

octaedro requested a review from joshuatf January 23, 2023 16:52

octaedro self-assigned this Jan 23, 2023

octaedro mentioned this pull request Jan 23, 2023

Add survey after disabling new experience #36518

Closed

8 tasks

joshuatf reviewed Jan 23, 2023

View reviewed changes

octaedro force-pushed the add/survey_after_disabling_new_exp branch from 44b7e27 to 7e43808 Compare January 23, 2023 21:35

github-actions bot removed the package: @woocommerce/customer-effort-score issues related to @woocommerce/customer-effort-score label Jan 23, 2023

Base automatically changed from add/feedback_modal_and_product_mvp_feedback_modal_components to trunk January 23, 2023 23:19

joshuatf reviewed Jan 23, 2023

View reviewed changes

Fernando Marichal added 12 commits January 23, 2023 20:32

Add customer-score-tracks data

14c4c21

Add callback after disabling new exp

244b4e3

Add TransientNotice after filling out the survey

dca1fe6

# Conflicts: # plugins/woocommerce/src/Admin/Features/NewProductManagementExperience.php

Remove comments

654f42e

Remove NEW_PRODUCT_MANAGEMENT_FEEDBACK

3c8769c

Add changelog

ed509d3

Rename const

cd78bed

Remove queryParam after showing notice

1d95288

Fix lint

b545f91

Fix lint 2.0

de2f284

Remove empty line

9a32351

Refactor maybe_show_disabled_notice

914a989

octaedro force-pushed the add/survey_after_disabling_new_exp branch from b9f238e to 914a989 Compare January 23, 2023 23:47

Fix lint 3.0

d34541e

octaedro requested a review from joshuatf January 23, 2023 23:56

joshuatf approved these changes Jan 24, 2023

View reviewed changes

octaedro merged commit c030b7d into trunk Jan 24, 2023

octaedro deleted the add/survey_after_disabling_new_exp branch January 24, 2023 01:43

github-actions bot added this to the 7.5.0 milestone Jan 24, 2023

octaedro modified the milestones: 7.5.0, 7.4.0 Jan 24, 2023

github-actions bot mentioned this pull request Jan 24, 2023

Cherry pick 36544 into release/7.4 #36568

Closed

github-actions bot mentioned this pull request Jan 24, 2023

Delete changelog files based on PR 36544 #36569

Closed

jonathansadowski modified the milestones: 7.4.0, 7.5.0 Jan 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add survey when disabling new experience #36544

Add survey when disabling new experience #36544

octaedro commented Jan 22, 2023 •

edited

github-actions bot commented Jan 22, 2023 •

edited

joshuatf left a comment

joshuatf Jan 23, 2023

joshuatf Jan 23, 2023

octaedro Jan 23, 2023

joshuatf Jan 23, 2023

octaedro Jan 23, 2023

joshuatf Jan 23, 2023

octaedro Jan 23, 2023

octaedro commented Jan 23, 2023

joshuatf left a comment

joshuatf Jan 23, 2023

joshuatf Jan 23, 2023

joshuatf Jan 23, 2023

joshuatf left a comment

	const classEditorUrl = values.id
	const classicEditorUrl = values.id

Add survey when disabling new experience #36544

Add survey when disabling new experience #36544

Conversation

octaedro commented Jan 22, 2023 • edited

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

github-actions bot commented Jan 22, 2023 • edited

Test Results Summary

joshuatf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

octaedro commented Jan 23, 2023

joshuatf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuatf left a comment

Choose a reason for hiding this comment

octaedro commented Jan 22, 2023 •

edited

github-actions bot commented Jan 22, 2023 •

edited