Update ReportError to use module specific ids of the insufficient permissions error when we build the get help link

[eugene-manuilov]

Feature Description

The insufficient permissions error has no error code, so when we build a get help link only error message is used. We need to generate a fake error code for the insufficient permissions error in the ReportError component to allow the support team to redirect users to module-specific articles.

https://github.com/google/site-kit-wp/blob/0d5b670504745849367c6274776ba9adcc824358/assets/js/components/ReportError.js#L77-L81

So, instead of passing an error to the getErrorTroubleshootingLinkURL selector directly, we need to check if this error is the insufficient permissions error and add ${ moduleSlug }_insufficient_permissions error code.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• The Get Help link in the ReportError component for the insufficient permissions error should have module specific error code: ${ moduleSlug }_insufficient_permissions.

Implementation Brief

In assets/js/components/ReportError.js, add the following changes:

• Map the errors array and check if the error object has an insufficient permissions error using the isInsufficientPermissionsError helper.
• If it is, return the existing error object and add the module slug to the error code - code: ${ moduleSlug }_insufficient_permissions.
• If it is not, then return the existing error object.

Test Coverage

In assets/js/components/ReportError.test.js, add test coverage to check the getErrorTroubleshootingLinkURL selector returns the correct URL for the insufficient permissions error with the error code for the Get help link.

QA Brief

Changelog entry

[eugene-manuilov]

IB ✔️

[mohitwp]

QA Update ❌

@asvinb

• Verified on both latest and dev environment.
• Verified for analytics, search console and AdSense module on main, entity and WP dashboard.
• On dev 'Get help' link slug changed successfully to - ${ moduleSlug }_insufficient_permissions.

Issue > On clicking 'Get help' link user is not navigating to correct destination. All 'Get help' links navigating user to https://sitekit.withgoogle.com/documentation/troubleshooting/using-troubleshooting-mode/ . If you compare it with latest environment then you will notice that links navigating users to destination as per the error. For example - https://sitekit.withgoogle.com/documentation/troubleshooting/analytics/#sufficient-permissions

[asvinb]

@eugene-manuilov Can you advise here? Shall the AC be updated?

[eugene-manuilov]

Issue > On clicking 'Get help' link user is not navigating to correct destination. All 'Get help' links navigating user to https://sitekit.withgoogle.com/documentation/troubleshooting/using-troubleshooting-mode/ . If you compare it with latest environment then you will notice that links navigating users to destination as per the error. For example - https://sitekit.withgoogle.com/documentation/troubleshooting/analytics/#sufficient-permissions

@mohitwp this will be fixed once we upload an updated version of the redirects mapping document to production. @jamesozzie is the document ready and contains redirects for new module aware permission redirects?

[jamesozzie]

@eugene-manuilov I'm reviewing the mapping sheet now. In order for me to add the correct "Get Help" link can you confirm the error codes per module are as below:

analytics_insufficient_permissions - which will direct to this guide adsense_insufficient_permissions - which will direct to this guide searchconsole_insufficient_permissions - which will direct to this guide

If so I'll go ahead and add these to the sheet. They don't exist on the sheet at present.

[eugene-manuilov]

searchconsole_insufficient_permissions - which will direct to this guide

@jamesozzie this one should be search-console_insufficient_permissions, other look good.

[jamesozzie]

These are all updated on the sheet now. Thanks @eugene-manuilov

[eugene-manuilov]

Thanks, @jamesozzie.

@mohitwp, I am assigning this ticket back to you. Redirects will point to correct documentation pages once we upload the new version of the redirects mapping document. If you have no other concerns, then I think we can consider this ticket QAed.

[mohitwp]

QA Update ✅

• Verified on both latest and dev environment.
• Verified for analytics, search console and AdSense module on main, entity and WP dashboard.
• On dev 'Get help' link slug changed successfully to - ${ moduleSlug }_insufficient_permissions.

Note : Redirection is not currently correct, but Eugene has suggested that this will be fixed when we upload the new version of the redirects mapping document.

[felixarntz]

Approval ❓

@eugene-manuilov @asvinb Looking at https://github.com/google/site-kit-wp/pull/6068/files#diff-b95a49b3fe9c5fd7c5b5f782eef76c5c71fd74299f4bdce4a7156144db48d621R74, doesn't this possibly cause a slight performance leak (since we're now always passing a new (non-identical) object to the getErrorTroubleshootingLinkURL selector? We should probably memoize the creation of the object there to avoid that. LMKWYT.

Another question unrelated to the code here, has the service / the CSV already been updated to account for the new error codes? Or do we need to create a task to create matching documentation?

[eugene-manuilov]

@felixarntz

doesn't this possibly cause a slight performance leak (since we're now always passing a new (non-identical) object to the getErrorTroubleshootingLinkURL selector?

Technically saying only memory can leak, performance can degrade. I don't think there is a potential memory leak here because the new object exists only in the useSelect callback scope. Then that object is destroyed by the garbage collector when we exit that scope.

There is also no performance degradation issues from my point of view. We just create a new object and pass it by reference into the selector function. It should be fine.

Another question unrelated to the code here, has the service / the CSV already been updated to account for the new error codes? Or do we need to create a task to create matching documentation?

James has already added new redirects for module specific errors to the redirects mapping document. I have created a new ticket in Asana and asked Adam to help with uploading it to production.

[felixarntz]

@eugene-manuilov

There is also no performance degradation issues from my point of view. We just create a new object and pass it by reference into the selector function. It should be fine.

Maybe I'm slightly confused here, I just remember there can be issues where creating new objects in selectors etc can cause unnecessary re-rendering cycles with React components, we had problems like this before. Have we ruled out that can occur here?

[eugene-manuilov]

there can be issues where creating new objects in selectors etc can cause unnecessary re-rendering cycles with React components, we had problems like this before.

I am not sure that anything like this can happen in this component. @asvinb or @tofumatt are you aware of anything like this?

[eugene-manuilov]

Another question unrelated to the code here, has the service / the CSV already been updated to account for the new error codes? Or do we need to create a task to create matching documentation?

James has already added new redirects for module specific errors to the redirects mapping document. I have created a new ticket in Asana and asked Adam to help with uploading it to production.

@felixarntz @jamesozzie the CSV with the latest error redirects mapping has been uploaded to production and new redirects work now: https://sitekit.withgoogle.com/support/?error_id=search-console_insufficient_permissions

[felixarntz]

I've opened #6114 here. When in doubt, it's better to include this just to be safe :)

[aaemnnosttv]

@felixarntz please see my comments on your PR, but TL;DR this shouldn't be necessary 👍