#2711: Domain manager view - remove manager when only one manager [BOB] #3388
Conversation
|
🥳 Successfully deployed to developer sandbox bob. |
|
🥳 Successfully deployed to developer sandbox bob. |
|
🥳 Successfully deployed to developer sandbox bob. |
|
🥳 Successfully deployed to developer sandbox bob. |
| @@ -1317,6 +1318,57 @@ def test_domain_invitation_flow(self): | |||
| home_page = self.app.get(reverse("home")) | |||
| self.assertContains(home_page, self.domain.name) | |||
|
|
|||
| @less_console_noise_decorator | |||
| def test_domain_user_role_delete(self): | |||
There was a problem hiding this comment.
No previous tests existed for this view
| # Assert that the Cancel link (form) is not displayed | ||
| self.assertNotContains(response, f"/invitation/{invitation.id}/cancel") |
There was a problem hiding this comment.
Prior test was not specific enough
| # Check if more than one manager exists on the domain. | ||
| # If only one exists, prevent this from happening | ||
| has_multiple_managers = len(UserDomainRole.objects.filter(domain=domain_pk)) > 1 | ||
| if not has_multiple_managers: | ||
| return False | ||
|
|
There was a problem hiding this comment.
This condition no longer returns a 403 permissions problem, and instead the view handles this and returns a specific error message.
| # Add conditionals to the context (such as "can_delete_users") | ||
| context = self._add_booleans_to_context(context) | ||
|
|
There was a problem hiding this comment.
no longer needed in context since template is no longer disabling button based on context values this method was adding
|
🥳 Successfully deployed to developer sandbox bob. |
| @@ -69,7 +69,6 @@ <h2 class> Domain managers </h2> | |||
| </th> | |||
| {% if not portfolio %}<td data-label="Role">{{ item.permission.role|title }}</td>{% endif %} | |||
| <td> | |||
| {% if can_delete_users %} | |||
There was a problem hiding this comment.
conditional now obsolete
| {% else %} | ||
| <input | ||
| type="submit" | ||
| class="usa-button--unstyled disabled-button usa-tooltip usa-tooltip--registrar" | ||
| value="Remove" | ||
| data-position="bottom" | ||
| title="Domains must have at least one domain manager" | ||
| data-tooltip="true" | ||
| aria-disabled="true" | ||
| role="button" | ||
| > | ||
| {% endif %} |
There was a problem hiding this comment.
no longer need to present disabled button under any conditions
dave-kennedy-ecs
changed the title
dave-kennedy-ecs
requested review from
lizpearl,
therealslimhsiehdy,
Matt-Spence,
CocoByte,
rachidatecs,
Katherine-Osos,
erinysong,
asaki222 and
zandercymatics
|
Hi Dave! Can you change the screen reader output on the Remove button to include the email of the domain manager being removed like "Remove " |
|
🥳 Successfully deployed to developer sandbox bob. |
Yes. I set the aria-label to "Remove {{email}}" on the Remove link. |
|
I'm just realizing I didn't quite update the alert message and it said the same thing as org model, which doesn't make sense. Can you update the message to say:
Also, can you match the usual margin we have when alerts stack between the error alert that appears and the info alert below it |
|
🥳 Successfully deployed to developer sandbox bob. |
Ticket
Resolves #2711
Changes
Context for reviewers
For developers reviewing, I added some notes in the PR where I removed or changed code and why.
Setup
Code Review Verification Steps
As the original developer, I have
Satisfied acceptance criteria and met development standards
Ensured code standards are met (Original Developer)
Validated user-facing changes (if applicable)
As a code reviewer, I have
Reviewed, tested, and left feedback about the changes
Validated user-facing changes as a developer
Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.
As a designer reviewer, I have
Verified that the changes match the design intention
Validated user-facing changes as a designer
References
Screenshots