diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1679eeed2..0cab01d31 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1268,15 +1268,6 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): _meta = Meta() - # Question for reviewers: should this include the invitation field? - # This is the same layout as before. - fieldsets = ( - ( - None, - {"fields": ("user", "portfolio", "invitation", "roles", "additional_permissions")}, - ), - ) - # Columns list_display = [ "user", diff --git a/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py b/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py deleted file mode 100644 index 7a73f7710..000000000 --- a/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py +++ /dev/null @@ -1,34 +0,0 @@ -import logging -from django.core.management import BaseCommand -from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper -from registrar.models import UserPortfolioPermission, PortfolioInvitation -from auditlog.models import LogEntry - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand, PopulateScriptTemplate): - help = "Loops through each UserPortfolioPermission object and populates the invitation field" - - def handle(self, **kwargs): - """Loops through each DomainRequest object and populates - its last_status_update and first_submitted_date values""" - self.existing_invitations = PortfolioInvitation.objects.filter( - portfolio__isnull=False, email__isnull=False - ).select_related("portfolio") - filter_condition = {"invitation__isnull": True, "portfolio__isnull": False, "user__email__isnull": False} - self.mass_update_records(UserPortfolioPermission, filter_condition, fields_to_update=["invitation"]) - - def update_record(self, record: UserPortfolioPermission): - """Associate the invitation to the right object""" - record.invitation = self.existing_invitations.filter( - email=record.user.email, portfolio=record.portfolio - ).first() - TerminalHelper.colorful_logger("INFO", "OKCYAN", f"{TerminalColors.OKCYAN}Adding invitation to {record}") - - def should_skip_record(self, record) -> bool: - """There is nothing to add if no invitation exists""" - return ( - not record - or not self.existing_invitations.filter(email=record.user.email, portfolio=record.portfolio).exists() - ) diff --git a/src/registrar/migrations/0138_userportfoliopermission_invitation.py b/src/registrar/migrations/0138_userportfoliopermission_invitation.py deleted file mode 100644 index abac42ae2..000000000 --- a/src/registrar/migrations/0138_userportfoliopermission_invitation.py +++ /dev/null @@ -1,25 +0,0 @@ -# Generated by Django 4.2.10 on 2024-11-14 21:05 - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0137_suborganization_city_suborganization_state_territory"), - ] - - operations = [ - migrations.AddField( - model_name="userportfoliopermission", - name="invitation", - field=models.ForeignKey( - blank=True, - null=True, - on_delete=django.db.models.deletion.PROTECT, - related_name="created_user_portfolio_permission", - to="registrar.portfolioinvitation", - ), - ), - ] diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 0ea410211..61a6b7397 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -9,8 +9,6 @@ from registrar.models.user_portfolio_permission import UserPortfolioPermission from .utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices # type: ignore from .utility.time_stamped_model import TimeStampedModel from django.contrib.postgres.fields import ArrayField -from django.contrib.admin.models import LogEntry, ADDITION -from django.contrib.contenttypes.models import ContentType logger = logging.getLogger(__name__) @@ -103,7 +101,7 @@ class PortfolioInvitation(TimeStampedModel): # and create a role for that user on this portfolio user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( - portfolio=self.portfolio, user=user, invitation=self + portfolio=self.portfolio, user=user ) if self.roles and len(self.roles) > 0: user_portfolio_permission.roles = self.roles diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 424f09a17..cd48b1b71 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -65,16 +65,6 @@ class UserPortfolioPermission(TimeStampedModel): help_text="Select one or more additional permissions.", ) - # TODO - this needs a small script to update existing values - invitation = models.ForeignKey( - "registrar.PortfolioInvitation", - null=True, - blank=True, - # We don't want to accidentally delete invitations - on_delete=models.PROTECT, - related_name="created_user_portfolio_permission", - ) - def __str__(self): readable_roles = [] if self.roles: diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index 105296855..bd1ad2be0 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -243,6 +243,22 @@ class UserPortfolioPermissionModelAnnotation(BaseModelAnnotation): # Get all members on this portfolio return Q(portfolio=portfolio) + @classmethod + def get_portfolio_invitation_id_query(cls): + """Gets the id of the portfolio invitation that created this UserPortfolioPermission. + This makes the assumption that if an invitation is retrieved, it must have created the given + UserPortfolioPermission object.""" + return Cast( + Subquery( + PortfolioInvitation.objects.filter( + status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED, + email=OuterRef(OuterRef("user__email")), + portfolio=OuterRef(OuterRef("portfolio")) + ).values("id")[:1] + ), + output_field=TextField() + ) + @classmethod def get_annotated_fields(cls, portfolio, csv_report=False): """ @@ -302,13 +318,11 @@ class UserPortfolioPermissionModelAnnotation(BaseModelAnnotation): & Q(user__permissions__domain__domain_info__portfolio=portfolio), ), "source": Value("permission", output_field=CharField()), - "invitation_date": Coalesce( - Func(F("invitation__created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField()), - Value("Invalid date"), - output_field=TextField(), + "invitation_date": PortfolioInvitationModelAnnotation.get_invitation_date_query( + object_id_query=cls.get_portfolio_invitation_id_query() ), - "invited_by": PortfolioInvitationModelAnnotation.get_invited_by_from_audit_log_query( - object_id_query=Cast(OuterRef("invitation__id"), output_field=TextField()) + "invited_by": PortfolioInvitationModelAnnotation.get_invited_by_query( + object_id_query=cls.get_portfolio_invitation_id_query() ), } @@ -350,7 +364,39 @@ class PortfolioInvitationModelAnnotation(BaseModelAnnotation): return Q(portfolio=portfolio) @classmethod - def get_invited_by_from_audit_log_query(cls, object_id_query): + def get_invitation_date_query(cls, object_id_query): + """Returns the date at which the given invitation was created. + Grabs this data from the audit log, given that a portfolio invitation object + is specified via object_id_query.""" + return Coalesce( + Subquery( + LogEntry.objects.filter( + content_type=ContentType.objects.get_for_model(PortfolioInvitation), + object_id=object_id_query, + action_flag=ADDITION, + ) + .annotate( + # Action time will always be equivalent to created_at in this context + display_date=Func( + F("action_time"), + Value("YYYY-MM-DD"), + function="to_char", + output_field=TextField() + ) + ) + .order_by("action_time") + .values("display_date")[:1] + ), + Value("Invalid date"), + output_field=TextField() + ) + + + @classmethod + def get_invited_by_query(cls, object_id_query): + """Returns the user that created the given portfolio invitation. + Grabs this data from the audit log, given that a portfolio invitation object + is specified via object_id_query.""" return Coalesce( Subquery( LogEntry.objects.filter( @@ -374,7 +420,7 @@ class PortfolioInvitationModelAnnotation(BaseModelAnnotation): ) ) .order_by("action_time") - .values("display_email") + .values("display_email")[:1] ), Value("Unknown"), output_field=CharField(), @@ -415,15 +461,13 @@ class PortfolioInvitationModelAnnotation(BaseModelAnnotation): ) ), "source": Value("invitation", output_field=CharField()), - "invitation_date": Coalesce( - Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField()), - Value("Invalid date"), - output_field=TextField(), + "invitation_date": cls.get_invitation_date_query( + object_id_query=Cast(OuterRef("id"), output_field=TextField()) ), # TODO - replace this with a "creator" field on portfolio invitation. This should be another ticket. # Grab the invitation creator from the audit log. This will need to be replaced with a creator field. # When that happens, just replace this with F("invitation__creator") - "invited_by": cls.get_invited_by_from_audit_log_query( + "invited_by": cls.get_invited_by_query( object_id_query=Cast(OuterRef("id"), output_field=TextField()) ), }