test: [M3-7508] - Add tests to check Parent and Child Close Account flows #10296
Conversation
cliu-akamai
requested review from
jdamore-linode
and removed request for
a team
|
Coverage Report: ✅ |
jdamore-linode
changed the title
There was a problem hiding this comment.
Looks good @cliu-akamai! I'd like to get either Mariah or Jaalah's approval before merging just to make sure all these scenarios are correct, but it's looking good to me! Thanks!
| /** | ||
| * Confirms that a proxy account cannot close the account | ||
| */ | ||
| it('disables a proxy account to close the account', () => { |
There was a problem hiding this comment.
| it('disables a proxy account to close the account', () => { | |
| it('disables "Close Account" button for proxy users', () => { |
| /** | ||
| * Confirms that a parent account with one or more active child accounts cannot close the account | ||
| */ | ||
| it('disables a parent account with one or more active child accounts to close the account', () => { |
There was a problem hiding this comment.
| it('disables a parent account with one or more active child accounts to close the account', () => { | |
| it('disables "Close Account" button for parent users', () => { |
|
|
||
| // Tooltip message that appears when a parent account with one and more child accounts tries to close the account. | ||
| const removeChildAccountTooltipsMessage = | ||
| 'Remove child accounts before closing the account.'; |
There was a problem hiding this comment.
Nit: We started created constants for things like this which would be nice to use especially when verbiage changes.
There was a problem hiding this comment.
Same for the other two if @jdamore-linode approves, but I understand if in a testing scenario we want to keep things silo'd
There was a problem hiding this comment.
I agree @jaalah-akamai, I think using the constants from src here would make sense.
(The only situations where I might insist that the strings be defined in the tests rather than imported from src is when the language itself is really important, like with notices and warnings involving billing or potential data loss, and that's specifically so the tests break if the language changes: it basically forces us to double check the wording and gives us an extra opportunity to spot typos, etc.)
There was a problem hiding this comment.
Yup I agree with that
| const contactParentUserTooltipsMessage = | ||
| 'Contact your parent user to close your account.'; | ||
|
|
||
| // Tooltip message that appears when a child account tries to close the account. |
There was a problem hiding this comment.
| // Tooltip message that appears when a child account tries to close the account. | |
| // Tooltip message that appears when a proxy account tries to close the account. |
Let's clarify this by saying proxy.
| const mockAccount = accountFactory.build(); | ||
| const mockProfile = profileFactory.build({ | ||
| username: 'default-user', | ||
| restricted: true, |
There was a problem hiding this comment.
This test shouldn't pass in real life because a restricted user should not be able to close an account, as we test in an earlier test case. Depending on what access they have for the account_access grant, they may not be able to see the Settings page at all (they'll get a 403 unauthorized on /account and see an error), or they may be able to see the page and click the Close Account button (with read_only or read_write access), but will see the final Close Account button within the modal greyed out because they are still a restricted user.
So, we should mock an unrestricted default account here:
| restricted: true, | |
| restricted: false, |
If a restricted default user with read_only/read_write account_access:
If a restricted default user with null account_access:
mjac0bs
added
Approved
Description 📝
Add regression tests to check Parent/Child Close Account flows
Major Changes 🔄
How to test 🧪