Return full stops back to the insufficient permissions error description

[eugene-manuilov]

Bug Description

Steps to reproduce

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots

Additional Context

• PHP Version:
• OS: [e.g. iOS]
• Browser [e.g. chrome, safari]
• Plugin Version [e.g. 22]
• Device: [e.g. iPhone6]

---

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

Acceptance criteria

• The error message from the getInsufficientPermissionsErrorDescription function should always have individual sentences separated from each other with a full stop.
• At the same time, it should still ensure that the last sentence does not end in a full stop, per our UX guidelines (text with 1 or 2 sentences should not end in a full stop).

Implementation Brief

• Using assets/js/util/insufficient-permissions-error-description.js,
  • Add a full stop, . at the end of every message.
  • Before returning the complete string from the function, check:
    • if userInfo is empty and remove the last . from message.

Test Coverage

• Add tests for getInsufficientPermissionsErrorDescription.

QA Brief

Changelog entry

• Add full stops to the insufficient permissions error description.

[aaemnnosttv]

@eugene-manuilov did we remove these recently?

[eugene-manuilov]

@aaemnnosttv it was removed as part of the work for #2597 (see here: https://github.com/google/site-kit-wp/pull/3316/files#diff-8e4815a7647006d9aea19dc2447a736e048c3ec903b9babb2af7934d3a06d451).

[eugene-manuilov]

@asvinb full stops should be added back to each version of the message copy and then trimmed from the end of the combined string. This is important to add full stops to all messages because it needs to be properly localized.

[eugene-manuilov]

IB ✔️

[eugene-manuilov]

@mitogh could you please add QAB?

[mitogh]

Thanks, @eugene-manuilov however while I was trying to find a scenario to replicate this issue I found 2 existing bugs in production that prevent us from actually testing this behavior as it should be.

• Some components remain in loading state on error #4542
• forbidden reason of error is not handled as insufficientPermissions error. #4543

Let me know what the best next steps should be here instead.

cc @wpdarren

[wpdarren]

@eugene-manuilov any thoughts on how we can test this based on @mitogh comment above?

[eugene-manuilov]

@wpdarren try to edit the Analytics settings using a secondary user that doesn't have access to the selected Analytics account.

[wpdarren]

QA Update: ✅

Verified:

• The error message has individual sentences separated from each other with a full stop.
• The last sentence does not end in a full stop.

Thanks @eugene-manuilov I got confused if this could be tested or not.

[felixarntz]

@eugene-manuilov @wpdarren The ACs here were defined a while ago and are actually not entirely correct. I don't remember exactly in which issue we had that conversation a few weeks back, but I remember for sure that we clarified that actually only single sentences should ever not end in a full stop.

This is not a big deal, but it should also be really quick to fix - can we add the full stop also to the second sentence above?

[mitogh]

A PR has been created with the requested change:

• Add a full stop for the second sentence. #4572

cc @felixarntz @eugene-manuilov @wpdarren

[felixarntz]

@mitogh The changes in the PR looks good, but it needs to be based on and target main - I left a PR comment with that as well. Can you please update it?

[mitogh]

Thanks, @felixarntz as discussed in previous PR #4572 was closed due it was based on develop instead I've cherry-pick the commit into a new PR #4574 and opened it against main.

[felixarntz]

Thanks @mitogh, merged!

Note for the future: Please keep in mind to also move and assign the issue accordingly in the project board, e.g. move from Execution to Code Review and unassign yourself if you're no longer working on it / waiting for someone else to review 👍

[felixarntz]

Approval (based on #4160 (comment)) ✅

[mitogh]

Thanks for the flag 💯 @felixarntz