Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Entrepreneur trial: Fix incorrectly used plan name in the trial "Welcome" note #1489

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

ivan-ottinger
Copy link
Contributor

@ivan-ottinger ivan-ottinger commented Jul 11, 2024

Changes proposed in this Pull Request:

The proposed change addresses issue discovered in https://github.com/Automattic/dotcom-forge/issues/7220#issuecomment-2186585648 where incorrectly used plan name is causing confusing "Welcome" note message.

Before After
342354629-7e4dafae-0a28-4f25-be6d-bb08ee43ae4b Markup on 2024-07-11 at 15:18:17

Originally, I aimed to avoid hardcoding the plan/product name, but it is not easily accessible in wc-calypso-bridge, plus we would need to load the plan/product name based on its slug - as the trial site won't have the full "Entrepreneur" plan/product name assigned to it anyways.

The proposed solution has advantage compared to the old Woo Express plan code (https://github.com/Automattic/wc-calypso-bridge/pull/1471/files#diff-26640952f8341c7dd518fee8f2bccdc3bcefefe943c53b992ff0bb4cf1da0876L38) where the plan name was hardcoded as well, but also was a part of the translated string. With the currently proposed PR, if the plan ever changes in the future, there won't be any need for new translations.

Related discussion: p1720687546715789-slack-C02TCEHP3HA

  • 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:

The changes proposed in this PR are targeting Entrepreneur Trial plan, however, due to technical limitations, we will only perform a partial test on a "full" Entrepreneur plan site.

If you don't have your WoA test site yet, please follow the steps outlined in pdDOJh-3ob-p2 and create a test site with Entrepreneur plan (the plan can be added through the SA).

  1. Check out the PR and sync it with your WoA test site on an Entrepreneur plan.
  2. Make the following temporary change:
diff --git a/includes/class-wc-calypso-bridge-setup.php b/includes/class-wc-calypso-bridge-setup.php
index 5429c57..16d1866 100644
--- a/includes/class-wc-calypso-bridge-setup.php
+++ b/includes/class-wc-calypso-bridge-setup.php
@@ -127,6 +127,11 @@ class WC_Calypso_Bridge_Setup {
 
 				$wp_admin_bar->remove_menu( 'ab-new-post' );
 			}, PHP_INT_MAX );
+
+			add_action( 'wp_before_admin_bar_render', function () {
+				include_once WC_CALYPSO_BRIDGE_PLUGIN_PATH . '/includes/notes/class-wc-calypso-bridge-free-trial-welcome.php';
+				WC_Calypso_Bridge_Free_Trial_Welcome_Note::possibly_add_note();
+			}, PHP_INT_MAX );
 		}
 	}
 
diff --git a/includes/notes/class-wc-calypso-bridge-free-trial-welcome.php b/includes/notes/class-wc-calypso-bridge-free-trial-welcome.php
index 826c430..ff49fb5 100644
--- a/includes/notes/class-wc-calypso-bridge-free-trial-welcome.php
+++ b/includes/notes/class-wc-calypso-bridge-free-trial-welcome.php
@@ -67,6 +67,7 @@ class WC_Calypso_Bridge_Free_Trial_Welcome_Note {
 	 * @return bool
 	 */
 	public static function can_be_added() {
+		return true;
 		$note = self::get_note();
 
 		if ( ! $note instanceof Note && ! $note instanceof WC_Admin_Note ) {
  1. Navigate to /wp-admin/admin.php?page=wc-admin and review the Welcome note. It should include correct copy - as can be seen in the After screenshot above.

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?

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.

@ivan-ottinger ivan-ottinger self-assigned this Jul 11, 2024
Copy link

Size Change: 0 B

Total Size: 201 kB

ℹ️ View Unchanged
Filename Size
./build/53.js 1.08 kB
./build/index.css 878 B
./build/index.js 126 kB
./build/marketing.js 58.3 kB
./build/payment-gateway-suggestions.css 1.24 kB
./build/payment-gateway-suggestions.js 6.57 kB
./build/plugins.js 3.93 kB
./build/style-index.css 2.15 kB
./build/style-marketing.css 800 B

compressed-size-action

@ivan-ottinger ivan-ottinger requested a review from a team July 11, 2024 13:37
Copy link

@phcp phcp left a comment

Choose a reason for hiding this comment

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

The PR looks good. Thanks for the fix!

@ivan-ottinger ivan-ottinger merged commit 18d469c into master Jul 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants