Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#1604 allow analysts to view cancelled invitations - [AD] #3069

Merged
merged 35 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
9777d27
adjusted test
asaki222 Oct 25, 2024
95a7e66
Merge branch 'main' of https://github.com/cisagov/manage.get.gov
asaki222 Oct 25, 2024
5c15d7e
Merge branch 'main' of https://github.com/cisagov/manage.get.gov
asaki222 Oct 30, 2024
963edc1
changes
asaki222 Oct 31, 2024
cb771f6
changes
asaki222 Oct 31, 2024
723bb08
changes
asaki222 Nov 4, 2024
5479823
updates thus far
asaki222 Nov 7, 2024
eaf9e24
Merge branch 'main' of https://github.com/cisagov/manage.get.gov into…
asaki222 Nov 11, 2024
ddb5691
final ish updates
asaki222 Nov 12, 2024
1aa96cb
added test back
asaki222 Nov 12, 2024
edfce15
added more comments
asaki222 Nov 12, 2024
d59c3a9
added more comments
asaki222 Nov 12, 2024
c04b477
updates
asaki222 Nov 12, 2024
4b13169
migration file
asaki222 Nov 13, 2024
9fc1b46
Merge branch 'main' of https://github.com/cisagov/manage.get.gov into…
asaki222 Nov 14, 2024
08b9de2
delete the deleteinvitationdelete view and added print statements to …
asaki222 Nov 14, 2024
199c38c
merge conflict
asaki222 Nov 14, 2024
2d4f092
reformmated
asaki222 Nov 14, 2024
1dfab09
some extra text
asaki222 Nov 18, 2024
e016561
removed update
asaki222 Nov 18, 2024
1b0389b
changes
asaki222 Nov 18, 2024
736faf3
Merge branch 'main' of https://github.com/cisagov/manage.get.gov into…
asaki222 Nov 18, 2024
0d3440e
make migration
asaki222 Nov 18, 2024
fdbcd99
add error
asaki222 Nov 18, 2024
94b3651
Merge branch 'main' of https://github.com/cisagov/manage.get.gov into…
asaki222 Nov 18, 2024
96a129d
Merge branch 'main' into ad/1604-allow-analysts-to-view-cancelled-inv…
asaki222 Nov 18, 2024
fdbac80
Merge branch 'ad/1604-allow-analysts-to-view-cancelled-invitations' o…
asaki222 Nov 18, 2024
d6ee1d4
updates
asaki222 Nov 18, 2024
2a22805
updated for merge conflicts
asaki222 Nov 18, 2024
aa18afc
made changes for the linter
asaki222 Nov 18, 2024
f8e4048
Update src/registrar/models/domain_invitation.py
asaki222 Nov 18, 2024
a43668a
Update src/registrar/views/domain.py
asaki222 Nov 18, 2024
443c200
updated text
asaki222 Nov 18, 2024
629419f
Merge branch 'ad/1604-allow-analysts-to-view-cancelled-invitations' o…
asaki222 Nov 18, 2024
616167a
fixed typo
asaki222 Nov 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/registrar/config/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,9 @@
name="user-profile",
),
path(
"invitation/<int:pk>/delete",
views.DomainInvitationDeleteView.as_view(http_method_names=["post"]),
name="invitation-delete",
"invitation/<int:pk>/cancel",
views.DomainInvitationCancelView.as_view(http_method_names=["post"]),
name="invitation-cancel",
),
path(
"domain-request/<int:pk>/delete",
Expand Down
24 changes: 24 additions & 0 deletions src/registrar/migrations/0138_alter_domaininvitation_status.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 4.2.10 on 2024-11-18 16:47

from django.db import migrations
import django_fsm


class Migration(migrations.Migration):

dependencies = [
("registrar", "0137_suborganization_city_suborganization_state_territory"),
]

operations = [
migrations.AlterField(
model_name="domaininvitation",
name="status",
field=django_fsm.FSMField(
choices=[("invited", "Invited"), ("retrieved", "Retrieved"), ("canceled", "Canceled")],
default="invited",
max_length=50,
protected=True,
),
),
]
11 changes: 11 additions & 0 deletions src/registrar/models/domain_invitation.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class Meta:
class DomainInvitationStatus(models.TextChoices):
INVITED = "invited", "Invited"
RETRIEVED = "retrieved", "Retrieved"
CANCELED = "canceled", "Canceled"

email = models.EmailField(
null=False,
Expand Down Expand Up @@ -73,3 +74,13 @@ def retrieve(self):
# something strange happened and this role already existed when
# the invitation was retrieved. Log that this occurred.
logger.warn("Invitation %s was retrieved for a role that already exists.", self)

@transition(field="status", source=DomainInvitationStatus.INVITED, target=DomainInvitationStatus.CANCELED)
def cancel_invitation(self):
"""When an invitation is cancel, change the status to canceled"""
asaki222 marked this conversation as resolved.
Show resolved Hide resolved
pass

@transition(field="status", source=DomainInvitationStatus.CANCELED, target=DomainInvitationStatus.INVITED)
def update_cancellation_status(self):
"""When an invitation is canceled but reinvited, update the status to invited"""
pass
2 changes: 1 addition & 1 deletion src/registrar/templates/domain_users.html
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ <h2>Invitations</h2>
{% if not portfolio %}<td data-label="Status">{{ invitation.domain_invitation.status|title }}</td>{% endif %}
<td>
{% if invitation.domain_invitation.status == invitation.domain_invitation.DomainInvitationStatus.INVITED %}
<form method="POST" action="{% url "invitation-delete" pk=invitation.domain_invitation.id %}">
<form method="POST" action="{% url "invitation-cancel" pk=invitation.domain_invitation.id %}">
{% csrf_token %}<input type="submit" class="usa-button--unstyled text-no-underline cursor-pointer" value="Cancel">
</form>
{% endif %}
Expand Down
2 changes: 1 addition & 1 deletion src/registrar/templatetags/custom_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def is_domain_subpage(path):
"domain-users-add",
"domain-request-delete",
"domain-user-delete",
"invitation-delete",
"invitation-cancel",
]
return get_url_name(path) in url_names

Expand Down
17 changes: 7 additions & 10 deletions src/registrar/tests/test_views_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -726,21 +726,18 @@ def test_domain_invitation_cancel(self):
"""Posting to the delete view deletes an invitation."""
email_address = "[email protected]"
invitation, _ = DomainInvitation.objects.get_or_create(domain=self.domain, email=email_address)
mock_client = MockSESClient()
with boto3_mocking.clients.handler_for("sesv2", mock_client):
self.client.post(reverse("invitation-delete", kwargs={"pk": invitation.id}))
mock_client.EMAILS_SENT.clear()
with self.assertRaises(DomainInvitation.DoesNotExist):
DomainInvitation.objects.get(id=invitation.id)
self.client.post(reverse("invitation-cancel", kwargs={"pk": invitation.id}))
invitation = DomainInvitation.objects.get(id=invitation.id)
self.assertEqual(invitation.status, DomainInvitation.DomainInvitationStatus.CANCELED)

@less_console_noise_decorator
def test_domain_invitation_cancel_retrieved_invitation(self):
"""Posting to the delete view when invitation retrieved returns an error message"""
"""Posting to the cancel view when invitation retrieved returns an error message"""
email_address = "[email protected]"
invitation, _ = DomainInvitation.objects.get_or_create(
domain=self.domain, email=email_address, status=DomainInvitation.DomainInvitationStatus.RETRIEVED
)
response = self.client.post(reverse("invitation-delete", kwargs={"pk": invitation.id}), follow=True)
response = self.client.post(reverse("invitation-cancel", kwargs={"pk": invitation.id}), follow=True)
# Assert that an error message is displayed to the user
self.assertContains(response, f"Invitation to {email_address} has already been retrieved.")
# Assert that the Cancel link is not displayed
Expand All @@ -751,7 +748,7 @@ def test_domain_invitation_cancel_retrieved_invitation(self):

@less_console_noise_decorator
def test_domain_invitation_cancel_no_permissions(self):
"""Posting to the delete view as a different user should fail."""
"""Posting to the cancel view as a different user should fail."""
email_address = "[email protected]"
invitation, _ = DomainInvitation.objects.get_or_create(domain=self.domain, email=email_address)

Expand All @@ -760,7 +757,7 @@ def test_domain_invitation_cancel_no_permissions(self):
self.client.force_login(other_user)
mock_client = MagicMock()
with boto3_mocking.clients.handler_for("sesv2", mock_client):
result = self.client.post(reverse("invitation-delete", kwargs={"pk": invitation.id}))
result = self.client.post(reverse("invitation-cancel", kwargs={"pk": invitation.id}))

self.assertEqual(result.status_code, 403)

Expand Down
2 changes: 1 addition & 1 deletion src/registrar/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
DomainSecurityEmailView,
DomainUsersView,
DomainAddUserView,
DomainInvitationDeleteView,
DomainInvitationCancelView,
DomainDeleteUserView,
)
from .user_profile import UserProfileView, FinishProfileSetupView
Expand Down
51 changes: 31 additions & 20 deletions src/registrar/views/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Authorization is handled by the `DomainPermissionView`. To ensure that only
authorized users can see information on a domain, every view here should
inherit from `DomainPermissionView` (or DomainInvitationPermissionDeleteView).
inherit from `DomainPermissionView`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a change in our coding practices? What's the justification for no longer inheriting from DomainInvitationPermission{Delete/Cancel}View?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just removed it from this text since DomainInvitationPermissionDeleteView no longer exists. I will add DomainInvitationPermissionCancelView! This is a great catch.

"""

from datetime import date
Expand Down Expand Up @@ -63,7 +63,7 @@
)

from ..utility.email import send_templated_email, EmailSendingError
from .utility import DomainPermissionView, DomainInvitationPermissionDeleteView
from .utility import DomainPermissionView, DomainInvitationPermissionCancelView

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -914,8 +914,10 @@ def _add_invitations_to_context(self, context, portfolio):
has_admin_flag = True
break # Once we find one match, no need to check further

# Add the role along with the computed flag to the list
invitations.append({"domain_invitation": domain_invitation, "has_admin_flag": has_admin_flag})
# Add the role along with the computed flag to the list if the domain invitation
# if the status is not canceled
if domain_invitation.status != "canceled":
invitations.append({"domain_invitation": domain_invitation, "has_admin_flag": has_admin_flag})

# Pass roles_with_flags to the context
context["invitations"] = invitations
Expand Down Expand Up @@ -985,6 +987,23 @@ def _is_member_of_different_org(self, email, requestor, requested_user):
existing_org_invitation and existing_org_invitation.portfolio != requestor_org
)

def _check_invite_status(self, invite, email):
"""Check invitation status if it is canceled or retrieved, and gives the appropiate response"""
asaki222 marked this conversation as resolved.
Show resolved Hide resolved
if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED:
messages.warning(
self.request,
f"{email} is already a manager for this domain.",
)
return False
elif invite.status == DomainInvitation.DomainInvitationStatus.CANCELED:
invite.update_cancellation_status()
invite.save()
return True
else:
# else if it has been sent but not accepted
messages.warning(self.request, f"{email} has already been invited to this domain")
return False

def _send_domain_invitation_email(self, email: str, requestor: User, requested_user=None, add_success=True):
"""Performs the sending of the domain invitation email,
does not make a domain information object
Expand Down Expand Up @@ -1020,17 +1039,8 @@ def _send_domain_invitation_email(self, email: str, requestor: User, requested_u
# Check to see if an invite has already been sent
try:
invite = DomainInvitation.objects.get(email=email, domain=self.object)
# check if the invite has already been accepted
if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED:
add_success = False
messages.warning(
self.request,
f"{email} is already a manager for this domain.",
)
else:
add_success = False
# else if it has been sent but not accepted
messages.warning(self.request, f"{email} has already been invited to this domain")
# check if the invite has already been accepted or has a canceled invite
add_success = self._check_invite_status(invite, email)
except Exception:
logger.error("An error occured")

Expand All @@ -1052,6 +1062,7 @@ def _send_domain_invitation_email(self, email: str, requestor: User, requested_u
self.object,
exc_info=True,
)
logger.info(exc)
raise EmailSendingError("Could not send email invitation.") from exc
else:
if add_success:
Expand Down Expand Up @@ -1127,18 +1138,18 @@ def form_valid(self, form):
return redirect(self.get_success_url())


# The order of the superclasses matters here. BaseDeleteView has a bug where the
# "form_valid" function does not call super, so it cannot use SuccessMessageMixin.
# The workaround is to use SuccessMessageMixin first.
class DomainInvitationDeleteView(SuccessMessageMixin, DomainInvitationPermissionDeleteView):
object: DomainInvitation # workaround for type mismatch in DeleteView
class DomainInvitationCancelView(SuccessMessageMixin, DomainInvitationPermissionCancelView):
object: DomainInvitation
fields = []

def post(self, request, *args, **kwargs):
"""Override post method in order to error in the case when the
domain invitation status is RETRIEVED"""
self.object = self.get_object()
form = self.get_form()
if form.is_valid() and self.object.status == self.object.DomainInvitationStatus.INVITED:
self.object.cancel_invitation()
self.object.save()
return self.form_valid(form)
else:
# Produce an error message if the domain invatation status is RETRIEVED
Expand Down
2 changes: 1 addition & 1 deletion src/registrar/views/utility/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
DomainPermissionView,
DomainRequestPermissionView,
DomainRequestPermissionWithdrawView,
DomainInvitationPermissionDeleteView,
DomainRequestWizardPermissionView,
PortfolioMembersPermission,
DomainRequestPortfolioViewonlyView,
DomainInvitationPermissionCancelView,
)
from .api_views import get_senior_official_from_federal_agency_json
1 change: 0 additions & 1 deletion src/registrar/views/utility/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,6 @@ def has_permission(self):
id=self.kwargs["pk"], domain__permissions__user=self.request.user
).exists():
return False

return True


Expand Down
14 changes: 4 additions & 10 deletions src/registrar/views/utility/permission_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import abc # abstract base class

from django.views.generic import DetailView, DeleteView, TemplateView
from django.views.generic import DetailView, DeleteView, TemplateView, UpdateView
from registrar.models import Domain, DomainRequest, DomainInvitation, Portfolio
from registrar.models.user import User
from registrar.models.user_domain_role import UserDomainRole
Expand Down Expand Up @@ -156,17 +156,11 @@ def template_name(self):
raise NotImplementedError


class DomainInvitationPermissionDeleteView(DomainInvitationPermission, DeleteView, abc.ABC):
"""Abstract view for deleting a domain invitation.

This one is fairly specialized, but this is the only thing that we do
right now with domain invitations. We still have the full
`DomainInvitationPermission` class, but here we just pair it with a
DeleteView.
"""
class DomainInvitationPermissionCancelView(DomainInvitationPermission, UpdateView, abc.ABC):
"""Abstract view for cancelling a DomainInvitation."""

model = DomainInvitation
object: DomainInvitation # workaround for type mismatch in DeleteView
object: DomainInvitation


class DomainRequestPermissionDeleteView(DomainRequestPermission, DeleteView, abc.ABC):
Expand Down
Loading