License activation: Add helpful error messages & display help steps on error response. #39333

elliottprogrammer · 2024-09-10T18:27:49Z

This PR updates the various error messages and displays error help steps upon license key activation error.

You can view the Figma designs here: B09ms2D7kgqRmn10LL9lsL-fi-4387_6302
Fixes: https://github.com/Automattic/jetpack-marketing/issues/967

Proposed changes:

Reword (and translate) the various error response messages that come from the server.
Display some specific error help steps below the error message.
See Figma design link above.

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

https://github.com/Automattic/jetpack-marketing/issues/967

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

Adds a new tracking event, tracks when error message is viewed and tracks the type of error thrown.

Testing instructions:

Create a JN site with Jetpack Beta. In the Beta plugin, activate this branch on the Jetpack plugin.
Connect Jetpack to a different WordPress account (Not your A11N wordpress.com account)
- I suggest you log-in to the JN site wp-admin dashboard from a new incognito window, then log into your non-A11N wordpress.com account in another tab (in that same incognito window), and then connect Jetpack to that (non A11N) user account.
  That way you can stay logged in to your A11N WordPress.com account in your other, non-incognito browser window.
On the JN site, open My Jetpack, and click "Activate a license"
Paste in a Jetpack Security or Complete license key that was purchased with your A11N account (with credits). (If you don't have one, you'll need to go ahead and purchase one (with your A11N account).
Click "Activate" button.
Verify the error message and the additional help info are displayed and that it matches the Figma design (link above). See screenshot:
Verify the copy looks good and the links work correctly.
Disconnect the site/user from Jetpack.
Now connect Jetpack to your A11N WordPress.com account.
Go to the "Activate a license" page again and go ahead and activate the Security or Complete license that was purchased with the A11N account. It should activate the product with no errors.
Now go to the "Activate a license" page again, and paste and activate the same license you just activated in the previous steps.
Verify a "This license is already active on your site." message displays (in green with check-mark), and it matches the Figma design. See screenshot:
Now purchase a Jetpack Backup license.
Go to the "Activate a license" page, paste in the Jetpack Backup license, and click "Activate"
Verify you see error, "Your site already has an active Jetpack plan of equal or higher value.", and the additional help info is displayed and that it matches the Figma design. Also see screenshot:
Verify the copy looks good and the links work correctly.
In Store Admin (SA), find the Jetpack Backup license subscription (still attached to a Jetpack Simple Checkout Temporary Site). Copy the license key, but then Remove the subscription.
Go to the "Activate a license" page, paste in the Jetpack Backup license key you copied, and click "Activate".
Verify you see error, "The subscription is no longer active or has expired. Please purchase a new license." and the additional help info is displayed and that it matches the Figma design. Also see screenshot:
Verify the copy looks good and the link works correctly.
Again go to the "Activate a license" page: type in some rubbish (an invalid license key), and click "Activate"
Verify you see error, "Unable to validate this license key." and the additional help info is displayed and that it matches the Figma design. Also see screenshot:

Additional notes:

The [multisite_incompatibility] error cannot be thrown (and tested) at this time, because My Jetpack was removed from multisite sites and therefore cannot activate licenses on multisite sites. There may be some issues concerning this, but it is not in scope of this PR.
The [attached_license] error ("This license is already active on another website.") also is not thrown anymore. The [not_same_owner] error is thrown instead. Still, I'm leaving the front-end code for this error in place because in the near future when we implement the "Error flow for Owned by different user": pbNhbs-aGG-p2, we will need this error message.

github-actions · 2024-09-10T18:28:39Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the update/license-activate-error-messages branch.

To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack update/license-activate-error-messages

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions · 2024-09-10T18:28:46Z

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.
✅ 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.

Follow this PR Review Process:

Ensure all required checks appearing at the bottom of this PR are passing.
Choose a review path based on your changes:
- A. Team Review: add the "[Status] Needs Team Review" label
  - For most changes, including minor cross-team impacts.
  - Example: Updating a team-specific component or a small change to a shared library.
- B. Crew Review: add the "[Status] Needs Review" label
  - For significant changes to core functionality.
  - Example: Major updates to a shared library or complex features.
- C. Both: Start with Team, then request Crew
  - For complex changes or when you need extra confidence.
  - Example: Refactor affecting multiple systems.
Get at least one approval before merging.

Still unsure? Reach out in #jetpack-developers for guidance!

CodeyGuyDylan

The testing plan worked correctly, nice work! I just had some comments on one of the error messages as well as a few things I found in the code

CodeyGuyDylan · 2024-09-11T18:04:51Z

projects/js-packages/licensing/components/activation-screen-error/use-get-error-content.tsx

+		case LICENSE_ERRORS.NOT_SAME_OWNER:
+			return {
+				errorMessage: __(
+					'The account that purchased the plan and the account managing this site are different.',


I think this sounds needlessly complicated personally. On the support page we link to here, the error is This license key is owned by a different user. Perhaps we could go with that instead?

Yeah this ⬆️ is the error message that displayed prior to this PR, and it is specifically the error that is resulting in regular support tickets and churn.
Because ideally what we need to get across to the user is: the (wordpress.com) user account that their site is using for the Jetpack connection is not the same (wordpress.com) user account that was used to purchase the product/license.
You see, there has been various discussion about this: p1HpG7-rfm-p2, and there is an upcoming project: pbNhbs-aGG-p2 to more throughly handle this issue, but for now as quick improvement, this is what design came up with.
Here are my thoughts: TBH, I agree that this error copy could be improved, but at the same time, I'm not necessarily eager to strike up a back and forth feedback loop of copy suggestion, because 1. This is merely a maintenance task, not a project, 2. there has already been quite a bit of discussion about this, 3. this will all be changed or removed in the near future when we are able to work on the upcoming planned project, and 4. Regardless of whether we could improve on the error message or not, I believe all these changes as a whole are quite a good improvement (with limited time investment), especially with the additional help steps/hints that go along with the error.

That makes sense, and especially since we are changing it soon, I think we can go ahead and keep it this way. Thank you for explaining that 😄

projects/js-packages/licensing/components/activation-screen-error/use-get-error-content.tsx

projects/js-packages/licensing/components/activation-screen-error/index.tsx

CodeyGuyDylan · 2024-09-11T18:14:07Z

projects/js-packages/licensing/components/activation-screen-error/index.tsx

+};
+
+const ActivationScreenError: FC< Props > = ( { licenseError, errorType } ) => {
+	const hasLicenseError = licenseError !== null && licenseError !== undefined;


You might think about just checking if ! licenseError here 🤔

If the licenseError was an empty string I think we could assume that there is no error as well. Same with 0 but that's less likely 😆

Hmm, yeah. I just copied this over from the previous existing code.. But now looking at it, I'm not sure it makes sense to have the hasLicenseError variable at all! The prop licenseError will either have an error in it (string) or it won't (null). Therefore I can just check the prop directly with if ( licenseError ) or if ( ! licenseError ), with no need for a different hasLicenseError variable.
Or is that what you are already saying in your feedback?
Anyway, I believe I can remove hasLicenseError and just check the licenseError prop instead. Same goes for in the activation-screen-controls/index.jsx file where it was originally located. And also in the use-get-error-content-.tsx file. 👍

Yes I think we are on the same page, thanks for updating that 😄

projects/js-packages/licensing/components/activation-screen-error/use-get-error-content.tsx

CodeyGuyDylan · 2024-09-11T18:20:23Z

projects/js-packages/licensing/components/activation-screen-error/style.scss

+    }
+
+    span {
+        font-size: 13px;


I see we have a --font-body-extra-small which is 12px and --font-body-small which is 14px. My gut tells me we should use one of those just to avoid having too many different font sizes. I understand if design thinks differently though

Yeah, my only hesitation here is that I believe Design used this specific font size along with the reduced letter spacing so that all errors remain within one line (on desktop anyway). If I change the font size up to 14px, a couple of the errors span over 2 lines (with orphan), and if I drop down the size to 12px, then the error seems a bit too small.
I think i'll stick with the Design specified font size in this instance.. Just because of the reason above, and also because (as mentioned in another comment), this change may not remain in the codebase too long due to the upcoming related project.

In my experience updating font size just to avoid lines overlapping is not best practices, but like you said this will be changing soon so I think it's fine for now 😅

CodeyGuyDylan · 2024-09-11T18:25:58Z

projects/js-packages/licensing/components/activation-screen-error/style.scss

+    color: var(--jp-gray-80);
+    font-size: var(--font-body-small);
+    line-height: calc(var(--font-title-small) - 2px);
+    border: 1px solid #F0F0F0;


We have a lot of grays already in the variables, I wonder if we really need to add a new one for this border specifically 🤔 Another thing that design would have a better feel for

Yeah, I don't want --jp-gray-0 because it's the same color as the background color, and --jp-gray stands out a bit too dark, imo. But actually, --jp-green-0 (#f0f2eb) is very close to #F0F0F0, and with it being only a 1px border, it's almost impossible to tell the difference with the naked eye. It's the closest thing, so I'll use that! 👍😉

Oh nice! I know I always try and suggest using existing variables, hopefully it's not too annoying 😅

Thank you!

projects/js-packages/licensing/components/activation-screen-error/style.scss

projects/js-packages/licensing/components/activation-screen-controls/index.jsx

CodeyGuyDylan

I think this all looks good and the testing plan still works, thanks for those updates!

…n error response. (#39333)

elliottprogrammer added 2 commits September 10, 2024 14:04

Add License: Update error messages & add error help info.

dfe09f7

changelog

70de4dd

elliottprogrammer self-assigned this Sep 10, 2024

github-actions bot added [JS Package] Licensing RNA labels Sep 10, 2024

github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Sep 10, 2024

Update unit test.

d1e2b18

github-actions bot added the [Tests] Includes Tests label Sep 10, 2024

elliottprogrammer added 2 commits September 10, 2024 16:52

Various final touch ups.

6db728b

Fix tiny css style.

3ae949e

elliottprogrammer added [Status] Needs Team Review and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 11, 2024

elliottprogrammer requested a review from a team September 11, 2024 00:28

CodeyGuyDylan requested changes Sep 11, 2024

View reviewed changes

Address all feedback points.

d3d0a2d

elliottprogrammer requested a review from CodeyGuyDylan September 12, 2024 04:33

CodeyGuyDylan approved these changes Sep 12, 2024

View reviewed changes

elliottprogrammer merged commit 2deccb0 into trunk Sep 12, 2024
71 checks passed

elliottprogrammer deleted the update/license-activate-error-messages branch September 12, 2024 18:02

github-actions bot removed the [Status] Needs Team Review label Sep 12, 2024

gogdzl pushed a commit that referenced this pull request Oct 25, 2024

License activation: Add helpful error messages & display help steps o…

7d84795

…n error response. (#39333)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

License activation: Add helpful error messages & display help steps on error response. #39333

License activation: Add helpful error messages & display help steps on error response. #39333

elliottprogrammer commented Sep 10, 2024 •

edited

Loading

github-actions bot commented Sep 10, 2024 •

edited

Loading

github-actions bot commented Sep 10, 2024 •

edited

Loading

CodeyGuyDylan left a comment

CodeyGuyDylan Sep 11, 2024

elliottprogrammer Sep 12, 2024

CodeyGuyDylan Sep 12, 2024

CodeyGuyDylan Sep 11, 2024

elliottprogrammer Sep 12, 2024

CodeyGuyDylan Sep 12, 2024

CodeyGuyDylan Sep 11, 2024

elliottprogrammer Sep 12, 2024

CodeyGuyDylan Sep 12, 2024

CodeyGuyDylan Sep 11, 2024

elliottprogrammer Sep 12, 2024

CodeyGuyDylan Sep 12, 2024

CodeyGuyDylan left a comment

License activation: Add helpful error messages & display help steps on error response. #39333

License activation: Add helpful error messages & display help steps on error response. #39333

Conversation

elliottprogrammer commented Sep 10, 2024 • edited Loading

Proposed changes:

Other information:

Jetpack product discussion

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

Testing instructions:

github-actions bot commented Sep 10, 2024 • edited Loading

github-actions bot commented Sep 10, 2024 • edited Loading

CodeyGuyDylan 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

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

CodeyGuyDylan left a comment

Choose a reason for hiding this comment

elliottprogrammer commented Sep 10, 2024 •

edited

Loading

github-actions bot commented Sep 10, 2024 •

edited

Loading

github-actions bot commented Sep 10, 2024 •

edited

Loading