From b0dc18cc3ff4670e13f5fa18f6fdd3a15b6e1db2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:32:07 -0700 Subject: [PATCH 01/45] base --- src/registrar/utility/csv_export.py | 147 ++++++++++++++++++---------- 1 file changed, 98 insertions(+), 49 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 64d960337..e90b27c29 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -15,6 +15,7 @@ from django.utils import timezone from django.db.models.functions import Concat, Coalesce from django.contrib.postgres.aggregates import StringAgg +from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.templatetags.custom_filters import get_region from registrar.utility.constants import BranchChoices @@ -50,11 +51,7 @@ def format_end_date(end_date): return timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) if end_date else get_default_end_date() -class BaseExport(ABC): - """ - A generic class for exporting data which returns a csv file for the given model. - Base class in an inheritance tree of 3. - """ +class BaseModelDict(ABC): @classmethod @abstractmethod @@ -65,13 +62,6 @@ def model(self): """ pass - @classmethod - def get_columns(cls): - """ - Returns the columns for CSV export. Override in subclasses as needed. - """ - return [] - @classmethod def get_sort_fields(cls): """ @@ -116,7 +106,7 @@ def get_filter_conditions(cls, **export_kwargs): return Q() @classmethod - def get_computed_fields(cls): + def get_annotated_fields(cls): """ Get a dict of computed fields. These are fields that do not exist on the model normally and will be passed to .annotate() when building a queryset. @@ -136,25 +126,10 @@ def get_related_table_fields(cls): Get a list of fields from related tables. """ return [] - - @classmethod - def update_queryset(cls, queryset, **kwargs): - """ - Returns an updated queryset. Override in subclass to update queryset. - """ - return queryset - - @classmethod - def write_csv_before(cls, csv_writer, **export_kwargs): - """ - Write to csv file before the write_csv method. - Override in subclasses where needed. - """ - pass - + @classmethod def annotate_and_retrieve_fields( - cls, initial_queryset, computed_fields, related_table_fields=None, include_many_to_many=False, **kwargs + cls, initial_queryset, annotated_fields, related_table_fields=None, include_many_to_many=False, **kwargs ) -> QuerySet: """ Applies annotations to a queryset and retrieves specified fields, @@ -162,7 +137,7 @@ def annotate_and_retrieve_fields( Parameters: initial_queryset (QuerySet): Initial queryset. - computed_fields (dict, optional): Fields to compute {field_name: expression}. + annotated_fields (dict, optional): Fields to compute {field_name: expression}. related_table_fields (list, optional): Extra fields to retrieve; defaults to annotation keys if None. include_many_to_many (bool, optional): Determines if we should include many to many fields or not **kwargs: Additional keyword arguments for specific parameters (e.g., public_contacts, domain_invitations, @@ -176,8 +151,8 @@ def annotate_and_retrieve_fields( # We can infer that if we're passing in annotations, # we want to grab the result of said annotation. - if computed_fields: - related_table_fields.extend(computed_fields.keys()) + if annotated_fields: + related_table_fields.extend(annotated_fields.keys()) # Get prexisting fields on the model model_fields = set() @@ -187,31 +162,33 @@ def annotate_and_retrieve_fields( if many_to_many or not isinstance(field, ManyToManyField): model_fields.add(field.name) - queryset = initial_queryset.annotate(**computed_fields).values(*model_fields, *related_table_fields) + queryset = initial_queryset.annotate(**annotated_fields).values(*model_fields, *related_table_fields) return cls.update_queryset(queryset, **kwargs) @classmethod - def export_data_to_csv(cls, csv_file, **export_kwargs): + def update_queryset(cls, queryset, **kwargs): """ - All domain metadata: - Exports domains of all statuses plus domain managers. + Returns an updated queryset. Override in subclass to update queryset. """ - writer = csv.writer(csv_file) - columns = cls.get_columns() + return queryset + + @classmethod + def get_model_dict(cls): sort_fields = cls.get_sort_fields() kwargs = cls.get_additional_args() select_related = cls.get_select_related() prefetch_related = cls.get_prefetch_related() exclusions = cls.get_exclusions() annotations_for_sort = cls.get_annotations_for_sort() - filter_conditions = cls.get_filter_conditions(**export_kwargs) - computed_fields = cls.get_computed_fields() + filter_conditions = cls.get_filter_conditions(**kwargs) + annotated_fields = cls.get_annotated_fields() related_table_fields = cls.get_related_table_fields() model_queryset = ( cls.model() - .objects.select_related(*select_related) + .objects + .select_related(*select_related) .prefetch_related(*prefetch_related) .filter(filter_conditions) .exclude(exclusions) @@ -219,13 +196,85 @@ def export_data_to_csv(cls, csv_file, **export_kwargs): .order_by(*sort_fields) .distinct() ) - - # Convert the queryset to a dictionary (including annotated fields) annotated_queryset = cls.annotate_and_retrieve_fields( - model_queryset, computed_fields, related_table_fields, **kwargs + model_queryset, annotated_fields, related_table_fields, **kwargs ) models_dict = convert_queryset_to_dict(annotated_queryset, is_model=False) + return models_dict + + +class UserPortfolioPermissionModelDict(BaseModelDict): + + @classmethod + def model(cls): + # Return the model class that this export handles + return UserPortfolioPermission + + @classmethod + def get_filter_conditions(cls, **export_kwargs): + """ + Get a Q object of filter conditions to filter when building queryset. + """ + return Q() + + @classmethod + def get_exclusions(cls): + """ + Get a Q object of exclusion conditions to pass to .exclude() when building queryset. + """ + return Q() + + @classmethod + def get_annotated_fields(cls): + """ + Get a dict of computed fields. These are fields that do not exist on the model normally + and will be passed to .annotate() when building a queryset. + """ + return {} + + @classmethod + def parse_row(cls, columns, model): + """ + Given a set of columns and a model dictionary, generate a new row from cleaned column data. + """ + FIELDS = {"Not yet defined": "Not yet defined"} + + row = [FIELDS.get(column, "") for column in columns] + return row + + +class BaseExport(BaseModelDict): + """ + A generic class for exporting data which returns a csv file for the given model. + Base class in an inheritance tree of 3. + """ + + @classmethod + def get_columns(cls): + """ + Returns the columns for CSV export. Override in subclasses as needed. + """ + return [] + + @classmethod + def write_csv_before(cls, csv_writer, **export_kwargs): + """ + Write to csv file before the write_csv method. + Override in subclasses where needed. + """ + pass + + @classmethod + def export_data_to_csv(cls, csv_file, **export_kwargs): + """ + All domain metadata: + Exports domains of all statuses plus domain managers. + """ + writer = csv.writer(csv_file) + columns = cls.get_columns() + models_dict = cls.get_model_dict() + # Write to csv file before the write_csv cls.write_csv_before(writer, **export_kwargs) @@ -534,7 +583,7 @@ def get_prefetch_related(cls): return ["permissions"] @classmethod - def get_computed_fields(cls, delimiter=", "): + def get_annotated_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ @@ -751,7 +800,7 @@ def get_filter_conditions(cls): ) @classmethod - def get_computed_fields(cls, delimiter=", "): + def get_annotated_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ @@ -846,7 +895,7 @@ def get_filter_conditions(cls): ) @classmethod - def get_computed_fields(cls, delimiter=", "): + def get_annotated_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ @@ -1465,7 +1514,7 @@ def get_sort_fields(cls): ] @classmethod - def get_computed_fields(cls, delimiter=", "): + def get_annotated_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ From 2265b70b5043c0efa6100f9ce4452f54427f9a30 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 13 Nov 2024 13:10:11 -0700 Subject: [PATCH 02/45] Refactor part 1 --- src/registrar/models/utility/orm_helper.py | 6 + src/registrar/utility/csv_export.py | 234 +++++++++++++++--- src/registrar/views/portfolio_members_json.py | 111 ++------- 3 files changed, 230 insertions(+), 121 deletions(-) create mode 100644 src/registrar/models/utility/orm_helper.py diff --git a/src/registrar/models/utility/orm_helper.py b/src/registrar/models/utility/orm_helper.py new file mode 100644 index 000000000..24f7982e7 --- /dev/null +++ b/src/registrar/models/utility/orm_helper.py @@ -0,0 +1,6 @@ +from django.db.models.expressions import Func + +class ArrayRemove(Func): + """Custom Func to use array_remove to remove null values""" + function = "array_remove" + template = "%(function)s(%(expressions)s, NULL)" \ No newline at end of file diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index e90b27c29..d39ab996c 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -10,16 +10,20 @@ DomainInformation, PublicContact, UserDomainRole, + PortfolioInvitation, ) -from django.db.models import Case, CharField, Count, DateField, F, ManyToManyField, Q, QuerySet, Value, When +from django.db.models import Case, CharField, Count, DateField, F, ManyToManyField, Q, QuerySet, Value, When, TextField, OuterRef, Subquery +from django.db.models.functions import Cast from django.utils import timezone from django.db.models.functions import Concat, Coalesce from django.contrib.postgres.aggregates import StringAgg from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.generic_helper import convert_queryset_to_dict +from registrar.models.utility.orm_helper import ArrayRemove from registrar.templatetags.custom_filters import get_region from registrar.utility.constants import BranchChoices from registrar.utility.enums import DefaultEmail +from django.contrib.postgres.aggregates import ArrayAgg logger = logging.getLogger(__name__) @@ -167,14 +171,7 @@ def annotate_and_retrieve_fields( return cls.update_queryset(queryset, **kwargs) @classmethod - def update_queryset(cls, queryset, **kwargs): - """ - Returns an updated queryset. Override in subclass to update queryset. - """ - return queryset - - @classmethod - def get_model_dict(cls): + def get_annotated_queryset(cls, request=None): sort_fields = cls.get_sort_fields() kwargs = cls.get_additional_args() select_related = cls.get_select_related() @@ -196,12 +193,21 @@ def get_model_dict(cls): .order_by(*sort_fields) .distinct() ) - annotated_queryset = cls.annotate_and_retrieve_fields( + + return cls.annotate_and_retrieve_fields( model_queryset, annotated_fields, related_table_fields, **kwargs ) - models_dict = convert_queryset_to_dict(annotated_queryset, is_model=False) - return models_dict + @classmethod + def update_queryset(cls, queryset, **kwargs): + """ + Returns an updated queryset. Override in subclass to update queryset. + """ + return queryset + + @classmethod + def get_models_dict(cls, request=None): + return convert_queryset_to_dict(cls.get_annotated_queryset(request), is_model=False) class UserPortfolioPermissionModelDict(BaseModelDict): @@ -212,36 +218,170 @@ def model(cls): return UserPortfolioPermission @classmethod - def get_filter_conditions(cls, **export_kwargs): + def get_select_related(cls): """ - Get a Q object of filter conditions to filter when building queryset. + Get a list of tables to pass to select_related when building queryset. """ - return Q() + return ["user"] @classmethod - def get_exclusions(cls): + def get_filter_conditions(cls, portfolio): """ - Get a Q object of exclusion conditions to pass to .exclude() when building queryset. + Get a Q object of filter conditions to filter when building queryset. """ - return Q() - + if not portfolio: + # Return nothing + return Q(id__in=[]) + + # Get all members on this portfolio + return Q(portfolio=portfolio) + @classmethod - def get_annotated_fields(cls): + def get_annotated_fields(cls, portfolio): """ Get a dict of computed fields. These are fields that do not exist on the model normally and will be passed to .annotate() when building a queryset. """ - return {} + if not portfolio: + # Return nothing + return {} + return { + "first_name": F("user__first_name"), + "last_name": F("user__last_name"), + "email_display": F("user__email"), + "last_active": Coalesce( + Cast(F("user__last_login"), output_field=TextField()), + Value("Invalid date"), + output_field=TextField(), + ), + "additional_permissions_display": F("additional_permissions"), + "member_display": Case( + When( + Q(user__email__isnull=False) & ~Q(user__email=""), + then=F("user__email") + ), + When( + Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), + then=Concat( + Coalesce(F("user__first_name"), Value("")), + Value(" "), + Coalesce(F("user__last_name"), Value("")), + ), + ), + default=Value(""), + output_field=CharField(), + ), + "domain_info": ArrayAgg( + Concat( + F("user__permissions__domain_id"), + Value(":"), + F("user__permissions__domain__name"), + output_field=CharField(), + ), + distinct=True, + filter=Q(user__permissions__domain__isnull=False) + & Q(user__permissions__domain__domain_info__portfolio=portfolio), + ), + "source": Value("permission", output_field=CharField()), + } + @classmethod - def parse_row(cls, columns, model): + def get_annotated_queryset(cls, portfolio): + """Override of the base annotated queryset to pass in portfolio""" + model_queryset = ( + cls.model() + .objects + .select_related(*cls.get_select_related()) + .prefetch_related(*cls.get_prefetch_related()) + .filter(cls.get_filter_conditions(portfolio)) + .exclude(cls.get_exclusions()) + .annotate(**cls.get_annotations_for_sort()) + .order_by(*cls.get_sort_fields()) + .distinct() + ) + + annotated_fields = cls.get_annotated_fields(portfolio) + related_table_fields = cls.get_related_table_fields() + return cls.annotate_and_retrieve_fields( + model_queryset, annotated_fields, related_table_fields + ) + + +class PortfolioInvitationModelDict(BaseModelDict): + + @classmethod + def model(cls): + # Return the model class that this export handles + return PortfolioInvitation + + @classmethod + def get_filter_conditions(cls, portfolio): """ - Given a set of columns and a model dictionary, generate a new row from cleaned column data. + Get a Q object of filter conditions to filter when building queryset. """ - FIELDS = {"Not yet defined": "Not yet defined"} + if not portfolio: + # Return nothing + return Q(id__in=[]) - row = [FIELDS.get(column, "") for column in columns] - return row + # Get all members on this portfolio + return Q( + # Check if email matches the OuterRef("email") + email=OuterRef("email"), + # Check if the domain's portfolio matches the given portfolio) + domain__domain_info__portfolio=portfolio, + ) + + @classmethod + def get_annotated_fields(cls, portfolio): + """ + Get a dict of computed fields. These are fields that do not exist on the model normally + and will be passed to .annotate() when building a queryset. + """ + if not portfolio: + # Return nothing + return {} + + domain_invitations = DomainInvitation.objects.filter( + email=OuterRef("email"), # Check if email matches the OuterRef("email") + domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio + ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) + return { + "first_name": Value(None, output_field=CharField()), + "last_name": Value(None, output_field=CharField()), + "email_display": F("email"), + "last_active": Value("Invited", output_field=TextField()), + "additional_permissions_display": F("additional_permissions"), + "member_display": F("email"), + "domain_info": ArrayRemove( + ArrayAgg( + Subquery(domain_invitations.values("domain_info")), + distinct=True, + ) + ), + "source": Value("invitation", output_field=CharField()), + } + + @classmethod + def get_annotated_queryset(cls, portfolio): + """Override of the base annotated queryset to pass in portfolio""" + model_queryset = ( + cls.model() + .objects + .select_related(*cls.get_select_related()) + .prefetch_related(*cls.get_prefetch_related()) + .filter(cls.get_filter_conditions(portfolio)) + .exclude(cls.get_exclusions()) + .annotate(**cls.get_annotations_for_sort()) + .order_by(*cls.get_sort_fields()) + .distinct() + ) + + annotated_fields = cls.get_annotated_fields(portfolio) + related_table_fields = cls.get_related_table_fields() + return cls.annotate_and_retrieve_fields( + model_queryset, annotated_fields, related_table_fields + ) class BaseExport(BaseModelDict): @@ -266,14 +406,14 @@ def write_csv_before(cls, csv_writer, **export_kwargs): pass @classmethod - def export_data_to_csv(cls, csv_file, **export_kwargs): + def export_data_to_csv(cls, csv_file, request=None, **export_kwargs): """ All domain metadata: Exports domains of all statuses plus domain managers. """ writer = csv.writer(csv_file) columns = cls.get_columns() - models_dict = cls.get_model_dict() + models_dict = cls.get_models_dict() # Write to csv file before the write_csv cls.write_csv_before(writer, **export_kwargs) @@ -321,6 +461,43 @@ def parse_row(cls, columns, model): """ pass +class MemberExport(BaseExport): + + @classmethod + def model(self): + """ + No model is defined for the member report as it is a combination of multiple fields. + This is a special edge case, but the base report requires this to be defined. + """ + return None + + @classmethod + def get_models_dict(cls, request=None): + portfolio = request.session.get("portfolio") + if not portfolio: + return {} + + # Union the two querysets to combine UserPortfolioPermission + invites + permissions = UserPortfolioPermissionModelDict.get_annotated_queryset(portfolio) + invitations = PortfolioInvitationModelDict.get_annotated_queryset(portfolio) + objects = permissions.union(invitations) + return convert_queryset_to_dict(objects, is_model=False) + + @classmethod + def get_columns(cls): + """ + Returns the columns for CSV export. Override in subclasses as needed. + """ + return [] + + @classmethod + @abstractmethod + def parse_row(cls, columns, model): + """ + Given a set of columns and a model dictionary, generate a new row from cleaned column data. + Must be implemented by subclasses + """ + pass class DomainExport(BaseExport): """ @@ -1597,3 +1774,4 @@ def get_creator_active_requests_count_query(cls): distinct=True, ) return query + diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 17209f0d9..408d37ff5 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -7,10 +7,9 @@ from django.urls import reverse from django.views import View -from registrar.models.domain_invitation import DomainInvitation -from registrar.models.portfolio_invitation import PortfolioInvitation -from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.utility.csv_export import UserPortfolioPermissionModelDict, PortfolioInvitationModelDict from registrar.views.utility.mixins import PortfolioMembersPermission @@ -39,7 +38,7 @@ def get(self, request): page_number = request.GET.get("page", 1) page_obj = paginator.get_page(page_number) - members = [self.serialize_members(request, portfolio, item, request.user) for item in page_obj.object_list] + members = [self.serialize_members(portfolio, item, request.user) for item in page_obj.object_list] return JsonResponse( { @@ -56,92 +55,25 @@ def get(self, request): def initial_permissions_search(self, portfolio): """Perform initial search for permissions before applying any filters.""" - permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio) - permissions = ( - permissions.select_related("user") - .annotate( - first_name=F("user__first_name"), - last_name=F("user__last_name"), - email_display=F("user__email"), - last_active=Coalesce( - Cast(F("user__last_login"), output_field=TextField()), # Cast last_login to text - Value("Invalid date"), - output_field=TextField(), - ), - additional_permissions_display=F("additional_permissions"), - member_display=Case( - # If email is present and not blank, use email - When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), - # If first name or last name is present, use concatenation of first_name + " " + last_name - When( - Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), - then=Concat( - Coalesce(F("user__first_name"), Value("")), - Value(" "), - Coalesce(F("user__last_name"), Value("")), - ), - ), - # If neither, use an empty string - default=Value(""), - output_field=CharField(), - ), - domain_info=ArrayAgg( - # an array of domains, with id and name, colon separated - Concat( - F("user__permissions__domain_id"), - Value(":"), - F("user__permissions__domain__name"), - # specify the output_field to ensure union has same column types - output_field=CharField(), - ), - distinct=True, - filter=Q(user__permissions__domain__isnull=False) # filter out null values - & Q( - user__permissions__domain__domain_info__portfolio=portfolio - ), # only include domains in portfolio - ), - source=Value("permission", output_field=CharField()), - ) - .values( - "id", - "first_name", - "last_name", - "email_display", - "last_active", - "roles", - "additional_permissions_display", - "member_display", - "domain_info", - "source", - ) + queryset = UserPortfolioPermissionModelDict.get_annotated_queryset(portfolio) + return queryset.values( + "id", + "first_name", + "last_name", + "email_display", + "last_active", + "roles", + "additional_permissions_display", + "member_display", + "domain_info", + "source", ) - return permissions def initial_invitations_search(self, portfolio): """Perform initial invitations search and get related DomainInvitation data based on the email.""" # Get DomainInvitation query for matching email and for the portfolio - domain_invitations = DomainInvitation.objects.filter( - email=OuterRef("email"), # Check if email matches the OuterRef("email") - domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio - ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) - # PortfolioInvitation query - invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) - invitations = invitations.annotate( - first_name=Value(None, output_field=CharField()), - last_name=Value(None, output_field=CharField()), - email_display=F("email"), - last_active=Value("Invited", output_field=TextField()), - additional_permissions_display=F("additional_permissions"), - member_display=F("email"), - # Use ArrayRemove to return an empty list when no domain invitations are found - domain_info=ArrayRemove( - ArrayAgg( - Subquery(domain_invitations.values("domain_info")), - distinct=True, - ) - ), - source=Value("invitation", output_field=CharField()), - ).values( + queryset = PortfolioInvitationModelDict.get_annotated_queryset(portfolio) + return queryset.values( "id", "first_name", "last_name", @@ -153,7 +85,6 @@ def initial_invitations_search(self, portfolio): "domain_info", "source", ) - return invitations def apply_search_term(self, queryset, request): """Apply search term to the queryset.""" @@ -179,7 +110,7 @@ def apply_sorting(self, queryset, request): queryset = queryset.order_by(sort_by) return queryset - def serialize_members(self, request, portfolio, item, user): + def serialize_members(self, portfolio, item, user): # Check if the user can edit other users user_can_edit_other_users = any( user.has_perm(perm) for perm in ["registrar.full_access_permission", "registrar.change_user"] @@ -213,9 +144,3 @@ def serialize_members(self, request, portfolio, item, user): "svg_icon": ("visibility" if view_only else "settings"), } return member_json - - -# Custom Func to use array_remove to remove null values -class ArrayRemove(Func): - function = "array_remove" - template = "%(function)s(%(expressions)s, NULL)" From 48ae7f4c117af8491870e22e6a0cc46ce102055e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 13 Nov 2024 14:31:59 -0700 Subject: [PATCH 03/45] Refactor part 2 --- .../templates/includes/members_table.html | 11 + src/registrar/utility/csv_export.py | 369 ++---------------- src/registrar/utility/model_dicts.py | 342 ++++++++++++++++ src/registrar/views/portfolio_members_json.py | 2 +- 4 files changed, 388 insertions(+), 336 deletions(-) create mode 100644 src/registrar/utility/model_dicts.py diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index 5e0dc6116..b1118abb4 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -36,6 +36,17 @@ + {% comment %} {% if user_domain_count and user_domain_count > 0 %} {% endcomment %} +
+
+ + Export as CSV + +
+
+ {% comment %} {% endif %} {% endcomment %} diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index d39ab996c..4b01c7e45 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -20,11 +20,14 @@ from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.models.utility.orm_helper import ArrayRemove +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.templatetags.custom_filters import get_region from registrar.utility.constants import BranchChoices from registrar.utility.enums import DefaultEmail from django.contrib.postgres.aggregates import ArrayAgg +from registrar.utility.model_dicts import BaseModelDict, PortfolioInvitationModelDict, UserPortfolioPermissionModelDict + logger = logging.getLogger(__name__) @@ -55,335 +58,6 @@ def format_end_date(end_date): return timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) if end_date else get_default_end_date() -class BaseModelDict(ABC): - - @classmethod - @abstractmethod - def model(self): - """ - Property to specify the model that the export class will handle. - Must be implemented by subclasses. - """ - pass - - @classmethod - def get_sort_fields(cls): - """ - Returns the sort fields for the CSV export. Override in subclasses as needed. - """ - return [] - - @classmethod - def get_additional_args(cls): - """ - Returns additional keyword arguments as an empty dictionary. - Override in subclasses to provide specific arguments. - """ - return {} - - @classmethod - def get_select_related(cls): - """ - Get a list of tables to pass to select_related when building queryset. - """ - return [] - - @classmethod - def get_prefetch_related(cls): - """ - Get a list of tables to pass to prefetch_related when building queryset. - """ - return [] - - @classmethod - def get_exclusions(cls): - """ - Get a Q object of exclusion conditions to pass to .exclude() when building queryset. - """ - return Q() - - @classmethod - def get_filter_conditions(cls, **export_kwargs): - """ - Get a Q object of filter conditions to filter when building queryset. - """ - return Q() - - @classmethod - def get_annotated_fields(cls): - """ - Get a dict of computed fields. These are fields that do not exist on the model normally - and will be passed to .annotate() when building a queryset. - """ - return {} - - @classmethod - def get_annotations_for_sort(cls): - """ - Get a dict of annotations to make available for order_by clause. - """ - return {} - - @classmethod - def get_related_table_fields(cls): - """ - Get a list of fields from related tables. - """ - return [] - - @classmethod - def annotate_and_retrieve_fields( - cls, initial_queryset, annotated_fields, related_table_fields=None, include_many_to_many=False, **kwargs - ) -> QuerySet: - """ - Applies annotations to a queryset and retrieves specified fields, - including class-defined and annotation-defined. - - Parameters: - initial_queryset (QuerySet): Initial queryset. - annotated_fields (dict, optional): Fields to compute {field_name: expression}. - related_table_fields (list, optional): Extra fields to retrieve; defaults to annotation keys if None. - include_many_to_many (bool, optional): Determines if we should include many to many fields or not - **kwargs: Additional keyword arguments for specific parameters (e.g., public_contacts, domain_invitations, - user_domain_roles). - - Returns: - QuerySet: Contains dictionaries with the specified fields for each record. - """ - if related_table_fields is None: - related_table_fields = [] - - # We can infer that if we're passing in annotations, - # we want to grab the result of said annotation. - if annotated_fields: - related_table_fields.extend(annotated_fields.keys()) - - # Get prexisting fields on the model - model_fields = set() - for field in cls.model()._meta.get_fields(): - # Exclude many to many fields unless we specify - many_to_many = isinstance(field, ManyToManyField) and include_many_to_many - if many_to_many or not isinstance(field, ManyToManyField): - model_fields.add(field.name) - - queryset = initial_queryset.annotate(**annotated_fields).values(*model_fields, *related_table_fields) - - return cls.update_queryset(queryset, **kwargs) - - @classmethod - def get_annotated_queryset(cls, request=None): - sort_fields = cls.get_sort_fields() - kwargs = cls.get_additional_args() - select_related = cls.get_select_related() - prefetch_related = cls.get_prefetch_related() - exclusions = cls.get_exclusions() - annotations_for_sort = cls.get_annotations_for_sort() - filter_conditions = cls.get_filter_conditions(**kwargs) - annotated_fields = cls.get_annotated_fields() - related_table_fields = cls.get_related_table_fields() - - model_queryset = ( - cls.model() - .objects - .select_related(*select_related) - .prefetch_related(*prefetch_related) - .filter(filter_conditions) - .exclude(exclusions) - .annotate(**annotations_for_sort) - .order_by(*sort_fields) - .distinct() - ) - - return cls.annotate_and_retrieve_fields( - model_queryset, annotated_fields, related_table_fields, **kwargs - ) - - @classmethod - def update_queryset(cls, queryset, **kwargs): - """ - Returns an updated queryset. Override in subclass to update queryset. - """ - return queryset - - @classmethod - def get_models_dict(cls, request=None): - return convert_queryset_to_dict(cls.get_annotated_queryset(request), is_model=False) - - -class UserPortfolioPermissionModelDict(BaseModelDict): - - @classmethod - def model(cls): - # Return the model class that this export handles - return UserPortfolioPermission - - @classmethod - def get_select_related(cls): - """ - Get a list of tables to pass to select_related when building queryset. - """ - return ["user"] - - @classmethod - def get_filter_conditions(cls, portfolio): - """ - Get a Q object of filter conditions to filter when building queryset. - """ - if not portfolio: - # Return nothing - return Q(id__in=[]) - - # Get all members on this portfolio - return Q(portfolio=portfolio) - - @classmethod - def get_annotated_fields(cls, portfolio): - """ - Get a dict of computed fields. These are fields that do not exist on the model normally - and will be passed to .annotate() when building a queryset. - """ - if not portfolio: - # Return nothing - return {} - - return { - "first_name": F("user__first_name"), - "last_name": F("user__last_name"), - "email_display": F("user__email"), - "last_active": Coalesce( - Cast(F("user__last_login"), output_field=TextField()), - Value("Invalid date"), - output_field=TextField(), - ), - "additional_permissions_display": F("additional_permissions"), - "member_display": Case( - When( - Q(user__email__isnull=False) & ~Q(user__email=""), - then=F("user__email") - ), - When( - Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), - then=Concat( - Coalesce(F("user__first_name"), Value("")), - Value(" "), - Coalesce(F("user__last_name"), Value("")), - ), - ), - default=Value(""), - output_field=CharField(), - ), - "domain_info": ArrayAgg( - Concat( - F("user__permissions__domain_id"), - Value(":"), - F("user__permissions__domain__name"), - output_field=CharField(), - ), - distinct=True, - filter=Q(user__permissions__domain__isnull=False) - & Q(user__permissions__domain__domain_info__portfolio=portfolio), - ), - "source": Value("permission", output_field=CharField()), - } - - @classmethod - def get_annotated_queryset(cls, portfolio): - """Override of the base annotated queryset to pass in portfolio""" - model_queryset = ( - cls.model() - .objects - .select_related(*cls.get_select_related()) - .prefetch_related(*cls.get_prefetch_related()) - .filter(cls.get_filter_conditions(portfolio)) - .exclude(cls.get_exclusions()) - .annotate(**cls.get_annotations_for_sort()) - .order_by(*cls.get_sort_fields()) - .distinct() - ) - - annotated_fields = cls.get_annotated_fields(portfolio) - related_table_fields = cls.get_related_table_fields() - return cls.annotate_and_retrieve_fields( - model_queryset, annotated_fields, related_table_fields - ) - - -class PortfolioInvitationModelDict(BaseModelDict): - - @classmethod - def model(cls): - # Return the model class that this export handles - return PortfolioInvitation - - @classmethod - def get_filter_conditions(cls, portfolio): - """ - Get a Q object of filter conditions to filter when building queryset. - """ - if not portfolio: - # Return nothing - return Q(id__in=[]) - - # Get all members on this portfolio - return Q( - # Check if email matches the OuterRef("email") - email=OuterRef("email"), - # Check if the domain's portfolio matches the given portfolio) - domain__domain_info__portfolio=portfolio, - ) - - @classmethod - def get_annotated_fields(cls, portfolio): - """ - Get a dict of computed fields. These are fields that do not exist on the model normally - and will be passed to .annotate() when building a queryset. - """ - if not portfolio: - # Return nothing - return {} - - domain_invitations = DomainInvitation.objects.filter( - email=OuterRef("email"), # Check if email matches the OuterRef("email") - domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio - ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) - return { - "first_name": Value(None, output_field=CharField()), - "last_name": Value(None, output_field=CharField()), - "email_display": F("email"), - "last_active": Value("Invited", output_field=TextField()), - "additional_permissions_display": F("additional_permissions"), - "member_display": F("email"), - "domain_info": ArrayRemove( - ArrayAgg( - Subquery(domain_invitations.values("domain_info")), - distinct=True, - ) - ), - "source": Value("invitation", output_field=CharField()), - } - - @classmethod - def get_annotated_queryset(cls, portfolio): - """Override of the base annotated queryset to pass in portfolio""" - model_queryset = ( - cls.model() - .objects - .select_related(*cls.get_select_related()) - .prefetch_related(*cls.get_prefetch_related()) - .filter(cls.get_filter_conditions(portfolio)) - .exclude(cls.get_exclusions()) - .annotate(**cls.get_annotations_for_sort()) - .order_by(*cls.get_sort_fields()) - .distinct() - ) - - annotated_fields = cls.get_annotated_fields(portfolio) - related_table_fields = cls.get_related_table_fields() - return cls.annotate_and_retrieve_fields( - model_queryset, annotated_fields, related_table_fields - ) - - class BaseExport(BaseModelDict): """ A generic class for exporting data which returns a csv file for the given model. @@ -413,7 +87,7 @@ def export_data_to_csv(cls, csv_file, request=None, **export_kwargs): """ writer = csv.writer(csv_file) columns = cls.get_columns() - models_dict = cls.get_models_dict() + models_dict = cls.get_models_dict(request=request) # Write to csv file before the write_csv cls.write_csv_before(writer, **export_kwargs) @@ -488,8 +162,18 @@ def get_columns(cls): """ Returns the columns for CSV export. Override in subclasses as needed. """ - return [] - + return [ + "Email", + "Organization admin", + "Invited by", + "Last active", + "Domain requests", + "Member management", + "Domain management", + "Number of domains", + "Domains", + ] + @classmethod @abstractmethod def parse_row(cls, columns, model): @@ -497,7 +181,22 @@ def parse_row(cls, columns, model): Given a set of columns and a model dictionary, generate a new row from cleaned column data. Must be implemented by subclasses """ - pass + + is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (model.get("roles") or []) + FIELDS = { + "Email": model.get("email"), + "Organization admin": is_admin, + "Invited by": "TODO", + "Last active": "TODO", + "Domain requests": "TODO", + "Member management": "TODO", + "Domain management": "TODO", + "Number of domains": "TODO", + "Domains": "TODO", + } + + row = [FIELDS.get(column, "") for column in columns] + return row class DomainExport(BaseExport): """ @@ -757,7 +456,7 @@ def get_prefetch_related(cls): """ Get a list of tables to pass to prefetch_related when building queryset. """ - return ["permissions"] + return ["domain__permissions"] @classmethod def get_annotated_fields(cls, delimiter=", "): @@ -797,7 +496,7 @@ class DomainDataTypeUser(DomainDataType): """ @classmethod - def get_filter_conditions(cls, request=None): + def get_filter_conditions(cls, request=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ diff --git a/src/registrar/utility/model_dicts.py b/src/registrar/utility/model_dicts.py new file mode 100644 index 000000000..d3f71aa1e --- /dev/null +++ b/src/registrar/utility/model_dicts.py @@ -0,0 +1,342 @@ +""" +TODO: explanation here +""" +from abc import ABC, abstractmethod +from registrar.models import ( + DomainInvitation, + PortfolioInvitation, +) +from django.db.models import Case, CharField, F, ManyToManyField, Q, QuerySet, Value, When, TextField, OuterRef, Subquery +from django.db.models.functions import Cast +from django.db.models.functions import Concat, Coalesce +from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models.utility.generic_helper import convert_queryset_to_dict +from registrar.models.utility.orm_helper import ArrayRemove +from django.contrib.postgres.aggregates import ArrayAgg + + +class BaseModelDict(ABC): + + @classmethod + @abstractmethod + def model(self): + """ + Property to specify the model that the export class will handle. + Must be implemented by subclasses. + """ + pass + + @classmethod + def get_sort_fields(cls): + """ + Returns the sort fields for the CSV export. Override in subclasses as needed. + """ + return [] + + @classmethod + def get_additional_args(cls): + """ + Returns additional keyword arguments as an empty dictionary. + Override in subclasses to provide specific arguments. + """ + return {} + + @classmethod + def get_select_related(cls): + """ + Get a list of tables to pass to select_related when building queryset. + """ + return [] + + @classmethod + def get_prefetch_related(cls): + """ + Get a list of tables to pass to prefetch_related when building queryset. + """ + return [] + + @classmethod + def get_exclusions(cls): + """ + Get a Q object of exclusion conditions to pass to .exclude() when building queryset. + """ + return Q() + + @classmethod + def get_filter_conditions(cls, **export_kwargs): + """ + Get a Q object of filter conditions to filter when building queryset. + """ + return Q() + + @classmethod + def get_annotated_fields(cls): + """ + Get a dict of computed fields. These are fields that do not exist on the model normally + and will be passed to .annotate() when building a queryset. + """ + return {} + + @classmethod + def get_annotations_for_sort(cls): + """ + Get a dict of annotations to make available for order_by clause. + """ + return {} + + @classmethod + def get_related_table_fields(cls): + """ + Get a list of fields from related tables. + """ + return [] + + @classmethod + def annotate_and_retrieve_fields( + cls, initial_queryset, annotated_fields, related_table_fields=None, include_many_to_many=False, **kwargs + ) -> QuerySet: + """ + Applies annotations to a queryset and retrieves specified fields, + including class-defined and annotation-defined. + + Parameters: + initial_queryset (QuerySet): Initial queryset. + annotated_fields (dict, optional): Fields to compute {field_name: expression}. + related_table_fields (list, optional): Extra fields to retrieve; defaults to annotation keys if None. + include_many_to_many (bool, optional): Determines if we should include many to many fields or not + **kwargs: Additional keyword arguments for specific parameters (e.g., public_contacts, domain_invitations, + user_domain_roles). + + Returns: + QuerySet: Contains dictionaries with the specified fields for each record. + """ + if related_table_fields is None: + related_table_fields = [] + + # We can infer that if we're passing in annotations, + # we want to grab the result of said annotation. + if annotated_fields: + related_table_fields.extend(annotated_fields.keys()) + + # Get prexisting fields on the model + model_fields = set() + for field in cls.model()._meta.get_fields(): + # Exclude many to many fields unless we specify + many_to_many = isinstance(field, ManyToManyField) and include_many_to_many + if many_to_many or not isinstance(field, ManyToManyField): + model_fields.add(field.name) + + queryset = initial_queryset.annotate(**annotated_fields).values(*model_fields, *related_table_fields) + + return cls.update_queryset(queryset, **kwargs) + + @classmethod + def get_annotated_queryset(cls, **kwargs): + sort_fields = cls.get_sort_fields() + # Get additional args and merge with incoming kwargs + additional_args = cls.get_additional_args() + kwargs.update(additional_args) + select_related = cls.get_select_related() + prefetch_related = cls.get_prefetch_related() + exclusions = cls.get_exclusions() + annotations_for_sort = cls.get_annotations_for_sort() + filter_conditions = cls.get_filter_conditions(**kwargs) + annotated_fields = cls.get_annotated_fields() + related_table_fields = cls.get_related_table_fields() + + model_queryset = ( + cls.model() + .objects + .select_related(*select_related) + .prefetch_related(*prefetch_related) + .filter(filter_conditions) + .exclude(exclusions) + .annotate(**annotations_for_sort) + .order_by(*sort_fields) + .distinct() + ) + return cls.annotate_and_retrieve_fields( + model_queryset, annotated_fields, related_table_fields, **kwargs + ) + + @classmethod + def update_queryset(cls, queryset, **kwargs): + """ + Returns an updated queryset. Override in subclass to update queryset. + """ + return queryset + + @classmethod + def get_models_dict(cls, **kwargs): + request = kwargs.get("request") + print(f"get_models_dict => request is: {request}") + return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) + + +class UserPortfolioPermissionModelDict(BaseModelDict): + + @classmethod + def model(cls): + # Return the model class that this export handles + return UserPortfolioPermission + + @classmethod + def get_select_related(cls): + """ + Get a list of tables to pass to select_related when building queryset. + """ + return ["user"] + + @classmethod + def get_filter_conditions(cls, portfolio): + """ + Get a Q object of filter conditions to filter when building queryset. + """ + if not portfolio: + # Return nothing + return Q(id__in=[]) + + # Get all members on this portfolio + return Q(portfolio=portfolio) + + @classmethod + def get_annotated_fields(cls, portfolio): + """ + Get a dict of computed fields. These are fields that do not exist on the model normally + and will be passed to .annotate() when building a queryset. + """ + if not portfolio: + # Return nothing + return {} + + return { + "first_name": F("user__first_name"), + "last_name": F("user__last_name"), + "email_display": F("user__email"), + "last_active": Coalesce( + Cast(F("user__last_login"), output_field=TextField()), + Value("Invalid date"), + output_field=TextField(), + ), + "additional_permissions_display": F("additional_permissions"), + "member_display": Case( + When( + Q(user__email__isnull=False) & ~Q(user__email=""), + then=F("user__email") + ), + When( + Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), + then=Concat( + Coalesce(F("user__first_name"), Value("")), + Value(" "), + Coalesce(F("user__last_name"), Value("")), + ), + ), + default=Value(""), + output_field=CharField(), + ), + "domain_info": ArrayAgg( + Concat( + F("user__permissions__domain_id"), + Value(":"), + F("user__permissions__domain__name"), + output_field=CharField(), + ), + distinct=True, + filter=Q(user__permissions__domain__isnull=False) + & Q(user__permissions__domain__domain_info__portfolio=portfolio), + ), + "source": Value("permission", output_field=CharField()), + } + + @classmethod + def get_annotated_queryset(cls, portfolio): + """Override of the base annotated queryset to pass in portfolio""" + model_queryset = ( + cls.model() + .objects + .select_related(*cls.get_select_related()) + .prefetch_related(*cls.get_prefetch_related()) + .filter(cls.get_filter_conditions(portfolio)) + .exclude(cls.get_exclusions()) + .annotate(**cls.get_annotations_for_sort()) + .order_by(*cls.get_sort_fields()) + .distinct() + ) + + annotated_fields = cls.get_annotated_fields(portfolio) + related_table_fields = cls.get_related_table_fields() + return cls.annotate_and_retrieve_fields( + model_queryset, annotated_fields, related_table_fields + ) + + +class PortfolioInvitationModelDict(BaseModelDict): + + @classmethod + def model(cls): + # Return the model class that this export handles + return PortfolioInvitation + + @classmethod + def get_filter_conditions(cls, portfolio): + """ + Get a Q object of filter conditions to filter when building queryset. + """ + if not portfolio: + # Return nothing + return Q(id__in=[]) + + # Get all members on this portfolio + return Q(portfolio=portfolio) + + @classmethod + def get_annotated_fields(cls, portfolio): + """ + Get a dict of computed fields. These are fields that do not exist on the model normally + and will be passed to .annotate() when building a queryset. + """ + if not portfolio: + # Return nothing + return {} + + domain_invitations = DomainInvitation.objects.filter( + email=OuterRef("email"), # Check if email matches the OuterRef("email") + domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio + ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) + return { + "first_name": Value(None, output_field=CharField()), + "last_name": Value(None, output_field=CharField()), + "email_display": F("email"), + "last_active": Value("Invited", output_field=TextField()), + "additional_permissions_display": F("additional_permissions"), + "member_display": F("email"), + "domain_info": ArrayRemove( + ArrayAgg( + Subquery(domain_invitations.values("domain_info")), + distinct=True, + ) + ), + "source": Value("invitation", output_field=CharField()), + } + + @classmethod + def get_annotated_queryset(cls, portfolio): + """Override of the base annotated queryset to pass in portfolio""" + model_queryset = ( + cls.model() + .objects + .select_related(*cls.get_select_related()) + .prefetch_related(*cls.get_prefetch_related()) + .filter(cls.get_filter_conditions(portfolio)) + .exclude(cls.get_exclusions()) + .annotate(**cls.get_annotations_for_sort()) + .order_by(*cls.get_sort_fields()) + .distinct() + ) + + annotated_fields = cls.get_annotated_fields(portfolio) + related_table_fields = cls.get_related_table_fields() + return cls.annotate_and_retrieve_fields( + model_queryset, annotated_fields, related_table_fields + ) diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 408d37ff5..3a909810c 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -9,7 +9,7 @@ from registrar.models import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from registrar.utility.csv_export import UserPortfolioPermissionModelDict, PortfolioInvitationModelDict +from registrar.utility.model_dicts import PortfolioInvitationModelDict, UserPortfolioPermissionModelDict from registrar.views.utility.mixins import PortfolioMembersPermission From 1e43974a111beb419c595c2cd853d4b194f2f7fc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 08:11:38 -0700 Subject: [PATCH 04/45] view --- src/registrar/config/urls.py | 6 ++++++ src/registrar/templates/includes/members_table.html | 2 +- src/registrar/views/report_views.py | 12 ++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index d289eaf90..e5606b986 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -21,6 +21,7 @@ ExportDomainRequestDataFull, ExportDataTypeUser, ExportDataTypeRequests, + ExportMembersPortfolio, ) # --jsons @@ -216,6 +217,11 @@ name="get-rejection-email-for-user-json", ), path("admin/", admin.site.urls), + path( + "reports/export_members_portfolio/", + ExportMembersPortfolio.as_view(), + name="export_members_portfolio", + ), path( "reports/export_data_type_user/", ExportDataTypeUser.as_view(), diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index b1118abb4..ae430501d 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -39,7 +39,7 @@ {% comment %} {% if user_domain_count and user_domain_count > 0 %} {% endcomment %}
- + Export as CSV diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index d9c4d192c..d30852540 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -169,6 +169,18 @@ def get(self, request, *args, **kwargs): return response +class ExportMembersPortfolio(View): + """Returns a a members report for a given portfolio""" + + def get(self, request, *args, **kwargs): + portfolio = request.session.get("portfolio") + # match the CSV example with all the fields + response = HttpResponse(content_type="text/csv") + response["Content-Disposition"] = f'attachment; filename="members-for-{portfolio}.csv"' + csv_export.MemberExport.export_data_to_csv(response, request=request) + return response + + class ExportDataTypeRequests(View): """Returns a domain requests report for a given user on the request""" From 9ac8a3e8bcbc0b5955b08e0c7243e162f1ce7617 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 10:22:41 -0700 Subject: [PATCH 05/45] Add some data to report --- src/registrar/utility/csv_export.py | 44 ++++++++++++++++----- src/registrar/utility/model_dicts.py | 58 ++++++++++++++++++++-------- src/registrar/views/report_views.py | 10 +++-- 3 files changed, 83 insertions(+), 29 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 4b01c7e45..2006a0b8d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -151,11 +151,23 @@ def get_models_dict(cls, request=None): if not portfolio: return {} - # Union the two querysets to combine UserPortfolioPermission + invites - permissions = UserPortfolioPermissionModelDict.get_annotated_queryset(portfolio) - invitations = PortfolioInvitationModelDict.get_annotated_queryset(portfolio) - objects = permissions.union(invitations) - return convert_queryset_to_dict(objects, is_model=False) + # Union the two querysets to combine UserPortfolioPermission + invites. + # Unions cannot have a col mismatch, so we must clamp what is returned here. + shared_columns = [ + "id", + "first_name", + "last_name", + "email_display", + "last_active", + "roles", + "additional_permissions_display", + "member_display", + "domain_info", + "source", + ] + permissions = UserPortfolioPermissionModelDict.get_annotated_queryset(portfolio, csv_report=True).values(*shared_columns) + invitations = PortfolioInvitationModelDict.get_annotated_queryset(portfolio, csv_report=True).values(*shared_columns) + return convert_queryset_to_dict(permissions.union(invitations), is_model=False) @classmethod def get_columns(cls): @@ -183,18 +195,32 @@ def parse_row(cls, columns, model): """ is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (model.get("roles") or []) + domains = ",".join(model.get("domain_info")) if model.get("domain_info") else "" FIELDS = { - "Email": model.get("email"), + "Email": model.get("email_display"), "Organization admin": is_admin, - "Invited by": "TODO", - "Last active": "TODO", + "Invited by": model.get("source"), + "Last active": model.get("last_active"), "Domain requests": "TODO", "Member management": "TODO", "Domain management": "TODO", "Number of domains": "TODO", - "Domains": "TODO", + # Quote enclose the domain list + # Note: this will only enclose when more than two items exist + "Domains": domains, } + # "id", + # "first_name", + # "last_name", + # "email_display", + # "last_active", + # "roles", + # "additional_permissions_display", + # "member_display", + # "domain_info", + # "source", + row = [FIELDS.get(column, "") for column in columns] return row diff --git a/src/registrar/utility/model_dicts.py b/src/registrar/utility/model_dicts.py index d3f71aa1e..859e8d3c1 100644 --- a/src/registrar/utility/model_dicts.py +++ b/src/registrar/utility/model_dicts.py @@ -6,9 +6,8 @@ DomainInvitation, PortfolioInvitation, ) -from django.db.models import Case, CharField, F, ManyToManyField, Q, QuerySet, Value, When, TextField, OuterRef, Subquery -from django.db.models.functions import Cast -from django.db.models.functions import Concat, Coalesce +from django.db.models import CharField, F, ManyToManyField, Q, QuerySet, Value, TextField, OuterRef, Subquery, Func, Case, When +from django.db.models.functions import Concat, Coalesce, Cast from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.models.utility.orm_helper import ArrayRemove @@ -200,7 +199,7 @@ def get_filter_conditions(cls, portfolio): return Q(portfolio=portfolio) @classmethod - def get_annotated_fields(cls, portfolio): + def get_annotated_fields(cls, portfolio, csv_report=False): """ Get a dict of computed fields. These are fields that do not exist on the model normally and will be passed to .annotate() when building a queryset. @@ -209,12 +208,34 @@ def get_annotated_fields(cls, portfolio): # Return nothing return {} + # Tweak the queries slightly to only return the data we need. + # When returning data for the csv report we: + # 1. Only return the domain name for 'domain_info' + # 2. Return a formatted date for 'last_active' + # These are just optimizations that are better done in SQL as opposed to python. + if csv_report: + domain_query = F("user__permissions__domain__name") + last_active_query = Func( + F("user__last_login"), + Value("FMMonth DD, YYYY"), + function="to_char", + output_field=TextField() + ) + else: + domain_query = Concat( + F("user__permissions__domain_id"), + Value(":"), + F("user__permissions__domain__name"), + output_field=CharField(), + ) + last_active_query = Cast(F("user__last_login"), output_field=TextField()) + return { "first_name": F("user__first_name"), "last_name": F("user__last_name"), "email_display": F("user__email"), "last_active": Coalesce( - Cast(F("user__last_login"), output_field=TextField()), + last_active_query, Value("Invalid date"), output_field=TextField(), ), @@ -236,12 +257,7 @@ def get_annotated_fields(cls, portfolio): output_field=CharField(), ), "domain_info": ArrayAgg( - Concat( - F("user__permissions__domain_id"), - Value(":"), - F("user__permissions__domain__name"), - output_field=CharField(), - ), + domain_query, distinct=True, filter=Q(user__permissions__domain__isnull=False) & Q(user__permissions__domain__domain_info__portfolio=portfolio), @@ -250,7 +266,7 @@ def get_annotated_fields(cls, portfolio): } @classmethod - def get_annotated_queryset(cls, portfolio): + def get_annotated_queryset(cls, portfolio, csv_report=False): """Override of the base annotated queryset to pass in portfolio""" model_queryset = ( cls.model() @@ -264,7 +280,7 @@ def get_annotated_queryset(cls, portfolio): .distinct() ) - annotated_fields = cls.get_annotated_fields(portfolio) + annotated_fields = cls.get_annotated_fields(portfolio, csv_report) related_table_fields = cls.get_related_table_fields() return cls.annotate_and_retrieve_fields( model_queryset, annotated_fields, related_table_fields @@ -291,7 +307,7 @@ def get_filter_conditions(cls, portfolio): return Q(portfolio=portfolio) @classmethod - def get_annotated_fields(cls, portfolio): + def get_annotated_fields(cls, portfolio, csv_report=False): """ Get a dict of computed fields. These are fields that do not exist on the model normally and will be passed to .annotate() when building a queryset. @@ -300,10 +316,18 @@ def get_annotated_fields(cls, portfolio): # Return nothing return {} + # Tweak the queries slightly to only return the data we need + if csv_report: + domain_query = F("domain__name") + else: + domain_query = Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField()) + + # Get all existing domain invitations and search on that domain_invitations = DomainInvitation.objects.filter( email=OuterRef("email"), # Check if email matches the OuterRef("email") domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio - ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) + ).annotate(domain_info=domain_query) + return { "first_name": Value(None, output_field=CharField()), "last_name": Value(None, output_field=CharField()), @@ -321,7 +345,7 @@ def get_annotated_fields(cls, portfolio): } @classmethod - def get_annotated_queryset(cls, portfolio): + def get_annotated_queryset(cls, portfolio, csv_report=False): """Override of the base annotated queryset to pass in portfolio""" model_queryset = ( cls.model() @@ -335,7 +359,7 @@ def get_annotated_queryset(cls, portfolio): .distinct() ) - annotated_fields = cls.get_annotated_fields(portfolio) + annotated_fields = cls.get_annotated_fields(portfolio, csv_report) related_table_fields = cls.get_related_table_fields() return cls.annotate_and_retrieve_fields( model_queryset, annotated_fields, related_table_fields diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index d30852540..3b0f790d3 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -173,10 +173,14 @@ class ExportMembersPortfolio(View): """Returns a a members report for a given portfolio""" def get(self, request, *args, **kwargs): - portfolio = request.session.get("portfolio") - # match the CSV example with all the fields + """Returns the members report""" + + portfolio_display = "portfolio" + if request.session.get("portfolio"): + portfolio_display = str(request.session.get("portfolio")).replace(" ", "-") + response = HttpResponse(content_type="text/csv") - response["Content-Disposition"] = f'attachment; filename="members-for-{portfolio}.csv"' + response["Content-Disposition"] = f'attachment; filename="members-for-{portfolio_display}.csv"' csv_export.MemberExport.export_data_to_csv(response, request=request) return response From 10c01870d70b35332f055b2376728632100e9429 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 12:53:13 -0700 Subject: [PATCH 06/45] Add some additional data + migration --- src/registrar/admin.py | 2 + ...0137_userportfoliopermission_invitation.py | 25 ++++ src/registrar/models/portfolio_invitation.py | 18 ++- .../models/user_portfolio_permission.py | 10 ++ src/registrar/utility/csv_export.py | 39 +++--- .../{model_dicts.py => model_annotations.py} | 112 ++++++++++++++++-- src/registrar/views/portfolio_members_json.py | 6 +- 7 files changed, 184 insertions(+), 28 deletions(-) create mode 100644 src/registrar/migrations/0137_userportfoliopermission_invitation.py rename src/registrar/utility/{model_dicts.py => model_annotations.py} (72%) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0cab01d31..2d05e760b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1275,6 +1275,8 @@ class Meta: "get_roles", ] + readonly_fields = ["invitation"] + autocomplete_fields = ["user", "portfolio"] search_fields = ["user__first_name", "user__last_name", "user__email", "portfolio__organization_name"] search_help_text = "Search by first name, last name, email, or portfolio." diff --git a/src/registrar/migrations/0137_userportfoliopermission_invitation.py b/src/registrar/migrations/0137_userportfoliopermission_invitation.py new file mode 100644 index 000000000..d0bc6dd63 --- /dev/null +++ b/src/registrar/migrations/0137_userportfoliopermission_invitation.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.10 on 2024-11-14 17:54 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0136_domainrequest_requested_suborganization_and_more"), + ] + + 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 61a6b7397..544d7d145 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -9,6 +9,8 @@ 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__) @@ -65,6 +67,20 @@ class PortfolioInvitationStatus(models.TextChoices): protected=True, # can't alter state except through transition methods! ) + # TODO - replace this with a "creator" field on portfolio invitation. This should be another ticket. + @property + def creator(self): + """Get the user who created this invitation from the audit log""" + content_type = ContentType.objects.get_for_model(self) + log_entry = LogEntry.objects.filter( + content_type=content_type, + object_id=self.pk, + action_flag=ADDITION + ).order_by("action_time").first() + + return log_entry.user if log_entry else None + + def __str__(self): return f"Invitation for {self.email} on {self.portfolio} is {self.status}" @@ -101,7 +117,7 @@ def retrieve(self): # and create a role for that user on this portfolio user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( - portfolio=self.portfolio, user=user + portfolio=self.portfolio, user=user, invitation=self ) 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 8d09562c2..e54bb5af5 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -65,6 +65,16 @@ class Meta: 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/csv_export.py b/src/registrar/utility/csv_export.py index 2006a0b8d..6abc54f5d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -26,7 +26,7 @@ from registrar.utility.enums import DefaultEmail from django.contrib.postgres.aggregates import ArrayAgg -from registrar.utility.model_dicts import BaseModelDict, PortfolioInvitationModelDict, UserPortfolioPermissionModelDict +from registrar.utility.model_annotations import BaseModelAnnotation, PortfolioInvitationModelAnnotation, UserPortfolioPermissionModelAnnotation logger = logging.getLogger(__name__) @@ -58,7 +58,7 @@ def format_end_date(end_date): return timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) if end_date else get_default_end_date() -class BaseExport(BaseModelDict): +class BaseExport(BaseModelAnnotation): """ A generic class for exporting data which returns a csv file for the given model. Base class in an inheritance tree of 3. @@ -87,13 +87,13 @@ def export_data_to_csv(cls, csv_file, request=None, **export_kwargs): """ writer = csv.writer(csv_file) columns = cls.get_columns() - models_dict = cls.get_models_dict(request=request) + model_dict = cls.get_model_dict(request=request) # Write to csv file before the write_csv cls.write_csv_before(writer, **export_kwargs) # Write the csv file - rows = cls.write_csv(writer, columns, models_dict) + rows = cls.write_csv(writer, columns, model_dict) # Return rows that for easier parsing and testing return rows @@ -146,7 +146,7 @@ def model(self): return None @classmethod - def get_models_dict(cls, request=None): + def get_model_dict(cls, request=None): portfolio = request.session.get("portfolio") if not portfolio: return {} @@ -164,10 +164,13 @@ def get_models_dict(cls, request=None): "member_display", "domain_info", "source", + "invitation_date", + "invited_by", ] - permissions = UserPortfolioPermissionModelDict.get_annotated_queryset(portfolio, csv_report=True).values(*shared_columns) - invitations = PortfolioInvitationModelDict.get_annotated_queryset(portfolio, csv_report=True).values(*shared_columns) - return convert_queryset_to_dict(permissions.union(invitations), is_model=False) + permissions = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values(*shared_columns) + invitations = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values(*shared_columns) + queryset_dict = convert_queryset_to_dict(permissions.union(invitations), is_model=False) + return queryset_dict @classmethod def get_columns(cls): @@ -178,6 +181,7 @@ def get_columns(cls): "Email", "Organization admin", "Invited by", + "Invitation date", "Last active", "Domain requests", "Member management", @@ -195,19 +199,26 @@ def parse_row(cls, columns, model): """ is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (model.get("roles") or []) - domains = ",".join(model.get("domain_info")) if model.get("domain_info") else "" + # Tracks if they can view, create requests, or not do anything + x = model.get("roles") + print(f"what are the roles? {x}") + domain_request_user_permission = None + + user_managed_domains = model.get("domain_info", []) + managed_domains_as_csv = ",".join(user_managed_domains) + # Whether they can make domain requests. Tentatively, I think the options as we currently understand would be: None, Viewer, Viewer Requester FIELDS = { "Email": model.get("email_display"), "Organization admin": is_admin, - "Invited by": model.get("source"), + "Invited by": model.get("invited_by"), + "Invitation date": model.get("invitation_date"), "Last active": model.get("last_active"), "Domain requests": "TODO", "Member management": "TODO", "Domain management": "TODO", - "Number of domains": "TODO", - # Quote enclose the domain list - # Note: this will only enclose when more than two items exist - "Domains": domains, + "Number of domains": len(user_managed_domains), + # TODO - this doesn't quote enclose with one record + "Domains": managed_domains_as_csv, } # "id", diff --git a/src/registrar/utility/model_dicts.py b/src/registrar/utility/model_annotations.py similarity index 72% rename from src/registrar/utility/model_dicts.py rename to src/registrar/utility/model_annotations.py index 859e8d3c1..7b6b2bf54 100644 --- a/src/registrar/utility/model_dicts.py +++ b/src/registrar/utility/model_annotations.py @@ -1,5 +1,24 @@ """ -TODO: explanation here +Model annotation classes. Intended to return django querysets with computed fields for api endpoints and our csv reports. + +Created to manage the complexity of the MembersTable and Members CSV report, as they require complex but common annotations. + +These classes provide consistent, reusable query transformations that: +1. Add computed fields via annotations +2. Handle related model data +3. Format fields for display +4. Standardize field names across different contexts + +Used by both API endpoints (e.g. portfolio members JSON) and data exports (e.g. CSV reports). + +Example: + # For a JSON table endpoint + permissions = UserPortfolioPermissionAnnotation.get_annotated_queryset(portfolio) + # Returns queryset with standardized fields for member tables + + # For a CSV export + permissions = UserPortfolioPermissionAnnotation.get_annotated_queryset(portfolio, csv_report=True) + # Returns same fields but formatted for CSV export """ from abc import ABC, abstractmethod from registrar.models import ( @@ -12,9 +31,19 @@ from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.models.utility.orm_helper import ArrayRemove from django.contrib.postgres.aggregates import ArrayAgg +from django.contrib.admin.models import LogEntry, ADDITION +from django.contrib.contenttypes.models import ContentType -class BaseModelDict(ABC): +class BaseModelAnnotation(ABC): + """ + Abstract base class that standardizes how models are annotated for csv exports or complex annotation queries. + For example, the Members table / csv export. + + Subclasses define model-specific annotations, filters, and field formatting while inheriting + common queryset building logic. + Intended ensure consistent data presentation across both table UI components and CSV exports. + """ @classmethod @abstractmethod @@ -166,14 +195,16 @@ def update_queryset(cls, queryset, **kwargs): return queryset @classmethod - def get_models_dict(cls, **kwargs): - request = kwargs.get("request") - print(f"get_models_dict => request is: {request}") + def get_model_dict(cls, **kwargs): return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) -class UserPortfolioPermissionModelDict(BaseModelDict): - +class UserPortfolioPermissionModelAnnotation(BaseModelAnnotation): + """ + Annotates UserPortfolioPermission querysets with computed fields for member tables. + Handles formatting of user details, permissions, and related domain information + for both UI display and CSV export. + """ @classmethod def model(cls): # Return the model class that this export handles @@ -217,7 +248,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): domain_query = F("user__permissions__domain__name") last_active_query = Func( F("user__last_login"), - Value("FMMonth DD, YYYY"), + Value("YYYY-MM-DD"), function="to_char", output_field=TextField() ) @@ -263,6 +294,30 @@ def get_annotated_fields(cls, portfolio, csv_report=False): & 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(), + ), + # 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": Coalesce( + Subquery( + LogEntry.objects.filter( + content_type=ContentType.objects.get_for_model(PortfolioInvitation), + object_id=Cast(OuterRef("invitation__id"), output_field=TextField()), # Look up the invitation's ID + action_flag=ADDITION + ).order_by("action_time").values("user__email")[:1] + ), + Value("Unknown"), + output_field=CharField() + ), } @classmethod @@ -287,13 +342,25 @@ def get_annotated_queryset(cls, portfolio, csv_report=False): ) -class PortfolioInvitationModelDict(BaseModelDict): +class PortfolioInvitationModelAnnotation(BaseModelAnnotation): + """ + Annotates PortfolioInvitation querysets with computed fields for the member table. + Handles formatting of user details, permissions, and related domain information + for both UI display and CSV export. + """ @classmethod def model(cls): # Return the model class that this export handles return PortfolioInvitation + @classmethod + def get_exclusions(cls): + """ + Get a Q object of exclusion conditions to pass to .exclude() when building queryset. + """ + return Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) + @classmethod def get_filter_conditions(cls, portfolio): """ @@ -322,7 +389,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): else: domain_query = Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField()) - # Get all existing domain invitations and search on that + # Get all existing domain invitations and search on that for domains the user exists on domain_invitations = DomainInvitation.objects.filter( email=OuterRef("email"), # Check if email matches the OuterRef("email") domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio @@ -342,6 +409,31 @@ def get_annotated_fields(cls, portfolio, csv_report=False): ) ), "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(), + ), + # 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": Coalesce( + Subquery( + LogEntry.objects.filter( + content_type=ContentType.objects.get_for_model(PortfolioInvitation), + # Look up the invitation's ID. LogEntry expects a string as this it is stored as json. + object_id=Cast(OuterRef("id"), output_field=TextField()), + action_flag=ADDITION + ).order_by("action_time").values("user__email")[:1] + ), + Value("Unknown"), + output_field=CharField() + ), } @classmethod diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 3a909810c..2fe0492d6 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -9,7 +9,7 @@ from registrar.models import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from registrar.utility.model_dicts import PortfolioInvitationModelDict, UserPortfolioPermissionModelDict +from registrar.utility.model_annotations import PortfolioInvitationModelAnnotation, UserPortfolioPermissionModelAnnotation from registrar.views.utility.mixins import PortfolioMembersPermission @@ -55,7 +55,7 @@ def get(self, request): def initial_permissions_search(self, portfolio): """Perform initial search for permissions before applying any filters.""" - queryset = UserPortfolioPermissionModelDict.get_annotated_queryset(portfolio) + queryset = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio) return queryset.values( "id", "first_name", @@ -72,7 +72,7 @@ def initial_permissions_search(self, portfolio): def initial_invitations_search(self, portfolio): """Perform initial invitations search and get related DomainInvitation data based on the email.""" # Get DomainInvitation query for matching email and for the portfolio - queryset = PortfolioInvitationModelDict.get_annotated_queryset(portfolio) + queryset = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio) return queryset.values( "id", "first_name", From b7f3f083fcc14a03068ae2eab9cb46e4b4f437bb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:22:05 -0700 Subject: [PATCH 07/45] Cleanup --- src/registrar/utility/csv_export.py | 8 ++-- src/registrar/utility/model_annotations.py | 48 ++++------------------ 2 files changed, 12 insertions(+), 44 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 6abc54f5d..a9d5c9770 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -496,7 +496,7 @@ def get_prefetch_related(cls): return ["domain__permissions"] @classmethod - def get_annotated_fields(cls, delimiter=", "): + def get_annotated_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -713,7 +713,7 @@ def get_filter_conditions(cls): ) @classmethod - def get_annotated_fields(cls, delimiter=", "): + def get_annotated_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -808,7 +808,7 @@ def get_filter_conditions(cls): ) @classmethod - def get_annotated_fields(cls, delimiter=", "): + def get_annotated_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -1427,7 +1427,7 @@ def get_sort_fields(cls): ] @classmethod - def get_annotated_fields(cls, delimiter=", "): + def get_annotated_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index 7b6b2bf54..14056a531 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -91,14 +91,14 @@ def get_exclusions(cls): return Q() @classmethod - def get_filter_conditions(cls, **export_kwargs): + def get_filter_conditions(cls, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ return Q() @classmethod - def get_annotated_fields(cls): + def get_annotated_fields(cls, **kwargs): """ Get a dict of computed fields. These are fields that do not exist on the model normally and will be passed to .annotate() when building a queryset. @@ -169,7 +169,7 @@ def get_annotated_queryset(cls, **kwargs): exclusions = cls.get_exclusions() annotations_for_sort = cls.get_annotations_for_sort() filter_conditions = cls.get_filter_conditions(**kwargs) - annotated_fields = cls.get_annotated_fields() + annotated_fields = cls.get_annotated_fields(**kwargs) related_table_fields = cls.get_related_table_fields() model_queryset = ( @@ -218,7 +218,7 @@ def get_select_related(cls): return ["user"] @classmethod - def get_filter_conditions(cls, portfolio): + def get_filter_conditions(cls, portfolio, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ @@ -319,27 +319,11 @@ def get_annotated_fields(cls, portfolio, csv_report=False): output_field=CharField() ), } - + @classmethod def get_annotated_queryset(cls, portfolio, csv_report=False): """Override of the base annotated queryset to pass in portfolio""" - model_queryset = ( - cls.model() - .objects - .select_related(*cls.get_select_related()) - .prefetch_related(*cls.get_prefetch_related()) - .filter(cls.get_filter_conditions(portfolio)) - .exclude(cls.get_exclusions()) - .annotate(**cls.get_annotations_for_sort()) - .order_by(*cls.get_sort_fields()) - .distinct() - ) - - annotated_fields = cls.get_annotated_fields(portfolio, csv_report) - related_table_fields = cls.get_related_table_fields() - return cls.annotate_and_retrieve_fields( - model_queryset, annotated_fields, related_table_fields - ) + return super().get_annotated_queryset(portfolio=portfolio, csv_report=csv_report) class PortfolioInvitationModelAnnotation(BaseModelAnnotation): @@ -362,7 +346,7 @@ def get_exclusions(cls): return Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) @classmethod - def get_filter_conditions(cls, portfolio): + def get_filter_conditions(cls, portfolio, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ @@ -439,20 +423,4 @@ def get_annotated_fields(cls, portfolio, csv_report=False): @classmethod def get_annotated_queryset(cls, portfolio, csv_report=False): """Override of the base annotated queryset to pass in portfolio""" - model_queryset = ( - cls.model() - .objects - .select_related(*cls.get_select_related()) - .prefetch_related(*cls.get_prefetch_related()) - .filter(cls.get_filter_conditions(portfolio)) - .exclude(cls.get_exclusions()) - .annotate(**cls.get_annotations_for_sort()) - .order_by(*cls.get_sort_fields()) - .distinct() - ) - - annotated_fields = cls.get_annotated_fields(portfolio, csv_report) - related_table_fields = cls.get_related_table_fields() - return cls.annotate_and_retrieve_fields( - model_queryset, annotated_fields, related_table_fields - ) + return super().get_annotated_queryset(portfolio=portfolio, csv_report=csv_report) From 174d217315ddc52564f311e45ae24492cf16fcfa Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:52:45 -0700 Subject: [PATCH 08/45] add info about roles / perms --- .../models/user_portfolio_permission.py | 27 +++++++++++++++++++ src/registrar/utility/csv_export.py | 20 ++++++-------- src/registrar/utility/model_annotations.py | 2 +- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index e54bb5af5..5a26f350e 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -115,6 +115,33 @@ def get_portfolio_permissions(cls, roles, additional_permissions): if additional_permissions: portfolio_permissions.update(additional_permissions) return list(portfolio_permissions) + + @classmethod + def get_domain_request_permission_display(cls, roles, additional_permissions): + """Class method to return a readable string for domain request permissions""" + # Tracks if they can view, create requests, or not do anything + all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, additional_permissions) + all_domain_perms = [UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS] + if (all(perm in all_permissions for perm in all_domain_perms)): + return "Viewer Requester" + elif (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS in all_permissions): + return "Viewer" + else: + return "None" + + @classmethod + def get_member_permission_display(cls, roles, additional_permissions): + """Class method to return a readable string for member permissions""" + # Tracks if they can view, create requests, or not do anything + all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, additional_permissions) + # Note for reviewers: the reason why this isn't checking on "all" is because + # the way perms work for members is different than requests. We need to consolidate this. + if (UserPortfolioPermissionChoices.EDIT_MEMBERS in all_permissions): + return "Manager" + elif (UserPortfolioPermissionChoices.VIEW_MEMBERS in all_permissions): + return "Viewer" + else: + return "None" def clean(self): """Extends clean method to perform additional validation, which can raise errors in django admin.""" diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index a9d5c9770..401cf37b5 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -10,7 +10,6 @@ DomainInformation, PublicContact, UserDomainRole, - PortfolioInvitation, ) from django.db.models import Case, CharField, Count, DateField, F, ManyToManyField, Q, QuerySet, Value, When, TextField, OuterRef, Subquery from django.db.models.functions import Cast @@ -20,7 +19,7 @@ from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.models.utility.orm_helper import ArrayRemove -from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices, UserPortfolioPermissionChoices from registrar.templatetags.custom_filters import get_region from registrar.utility.constants import BranchChoices from registrar.utility.enums import DefaultEmail @@ -197,24 +196,21 @@ def parse_row(cls, columns, model): Given a set of columns and a model dictionary, generate a new row from cleaned column data. Must be implemented by subclasses """ - - is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (model.get("roles") or []) - # Tracks if they can view, create requests, or not do anything - x = model.get("roles") - print(f"what are the roles? {x}") - domain_request_user_permission = None - + roles = model.get("roles") + additional_permissions = model.get("additional_permissions_display") + is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (roles or []) + domain_request_display = UserPortfolioPermission.get_domain_request_permission_display(roles, additional_permissions) + member_perm_display = UserPortfolioPermission.get_member_permission_display(roles, additional_permissions) user_managed_domains = model.get("domain_info", []) managed_domains_as_csv = ",".join(user_managed_domains) - # Whether they can make domain requests. Tentatively, I think the options as we currently understand would be: None, Viewer, Viewer Requester FIELDS = { "Email": model.get("email_display"), "Organization admin": is_admin, "Invited by": model.get("invited_by"), "Invitation date": model.get("invitation_date"), "Last active": model.get("last_active"), - "Domain requests": "TODO", - "Member management": "TODO", + "Domain requests": domain_request_display, + "Member management": member_perm_display, "Domain management": "TODO", "Number of domains": len(user_managed_domains), # TODO - this doesn't quote enclose with one record diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index 14056a531..5fc868462 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -241,7 +241,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): # Tweak the queries slightly to only return the data we need. # When returning data for the csv report we: - # 1. Only return the domain name for 'domain_info' + # 1. Only return the domain name for 'domain_info' rather than also add ':' seperated id # 2. Return a formatted date for 'last_active' # These are just optimizations that are better done in SQL as opposed to python. if csv_report: From bf585bec2afb794d036ec1118a23f5376e7082c4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:57:28 -0700 Subject: [PATCH 09/45] add domain management portion --- src/registrar/utility/csv_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 401cf37b5..c8dde131e 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -211,7 +211,7 @@ def parse_row(cls, columns, model): "Last active": model.get("last_active"), "Domain requests": domain_request_display, "Member management": member_perm_display, - "Domain management": "TODO", + "Domain management": len(user_managed_domains) > 0, "Number of domains": len(user_managed_domains), # TODO - this doesn't quote enclose with one record "Domains": managed_domains_as_csv, From 0ca0602b0735b8b2c9695626655a953d7fc85ebe Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:01:15 -0700 Subject: [PATCH 10/45] Update portfolio_invitation.py --- src/registrar/models/portfolio_invitation.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 544d7d145..0ea410211 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -67,20 +67,6 @@ class PortfolioInvitationStatus(models.TextChoices): protected=True, # can't alter state except through transition methods! ) - # TODO - replace this with a "creator" field on portfolio invitation. This should be another ticket. - @property - def creator(self): - """Get the user who created this invitation from the audit log""" - content_type = ContentType.objects.get_for_model(self) - log_entry = LogEntry.objects.filter( - content_type=content_type, - object_id=self.pk, - action_flag=ADDITION - ).order_by("action_time").first() - - return log_entry.user if log_entry else None - - def __str__(self): return f"Invitation for {self.email} on {self.portfolio} is {self.status}" From 78e03160aa485e60f0949ebc685f088b96d5a452 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:04:51 -0700 Subject: [PATCH 11/45] Delete 0137_userportfoliopermission_invitation.py --- ...0137_userportfoliopermission_invitation.py | 25 ------------------- 1 file changed, 25 deletions(-) delete mode 100644 src/registrar/migrations/0137_userportfoliopermission_invitation.py diff --git a/src/registrar/migrations/0137_userportfoliopermission_invitation.py b/src/registrar/migrations/0137_userportfoliopermission_invitation.py deleted file mode 100644 index d0bc6dd63..000000000 --- a/src/registrar/migrations/0137_userportfoliopermission_invitation.py +++ /dev/null @@ -1,25 +0,0 @@ -# Generated by Django 4.2.10 on 2024-11-14 17:54 - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0136_domainrequest_requested_suborganization_and_more"), - ] - - 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", - ), - ), - ] From 17acb664a510391a219987f32b95f80619850b6b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:05:32 -0700 Subject: [PATCH 12/45] readd migration --- ...0138_userportfoliopermission_invitation.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/registrar/migrations/0138_userportfoliopermission_invitation.py diff --git a/src/registrar/migrations/0138_userportfoliopermission_invitation.py b/src/registrar/migrations/0138_userportfoliopermission_invitation.py new file mode 100644 index 000000000..abac42ae2 --- /dev/null +++ b/src/registrar/migrations/0138_userportfoliopermission_invitation.py @@ -0,0 +1,25 @@ +# 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", + ), + ), + ] From ae6ecabfc020c3dc7a4927e2219800f87ddefb62 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:19:33 -0700 Subject: [PATCH 13/45] Update csv_export.py --- src/registrar/utility/csv_export.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index c8dde131e..a6cf53c0f 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -547,7 +547,7 @@ class DomainRequestsDataType: """ @classmethod - def get_filter_conditions(cls, request=None): + def get_filter_conditions(cls, request=None, **kwargs): if request is None or not hasattr(request, "user") or not request.user.is_authenticated: return Q(id__in=[]) @@ -697,7 +697,7 @@ def get_select_related(cls): return ["domain"] @classmethod - def get_filter_conditions(cls): + def get_filter_conditions(cls, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ @@ -791,7 +791,7 @@ def get_select_related(cls): return ["domain"] @classmethod - def get_filter_conditions(cls): + def get_filter_conditions(cls, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ @@ -888,7 +888,7 @@ def get_select_related(cls): return ["domain"] @classmethod - def get_filter_conditions(cls, start_date=None, end_date=None): + def get_filter_conditions(cls, start_date=None, end_date=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ @@ -960,7 +960,7 @@ def get_prefetch_related(cls): return ["permissions"] @classmethod - def get_filter_conditions(cls, start_date=None, end_date=None): + def get_filter_conditions(cls, start_date=None, end_date=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ @@ -1095,7 +1095,7 @@ def get_prefetch_related(cls): return ["permissions"] @classmethod - def get_filter_conditions(cls, start_date=None, end_date=None): + def get_filter_conditions(cls, start_date=None, end_date=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ @@ -1327,7 +1327,7 @@ def get_sort_fields(cls): ] @classmethod - def get_filter_conditions(cls, start_date=None, end_date=None): + def get_filter_conditions(cls, start_date=None, end_date=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ From e915937510bb5d3242f8bbc030eccafa31ef546f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Nov 2024 10:39:44 -0700 Subject: [PATCH 14/45] Update csv_export.py --- src/registrar/utility/csv_export.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index a6cf53c0f..a85732a3e 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -892,6 +892,10 @@ def get_filter_conditions(cls, start_date=None, end_date=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ + if not start_date or not end_date: + # Return nothing + return Q(id__in=[]) + filter_ready = Q( domain__state__in=[Domain.State.READY], domain__first_ready__gte=start_date, @@ -960,10 +964,14 @@ def get_prefetch_related(cls): return ["permissions"] @classmethod - def get_filter_conditions(cls, start_date=None, end_date=None, **kwargs): + def get_filter_conditions(cls, end_date=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ + if not end_date: + # Return nothing + return Q(id__in=[]) + end_date_formatted = format_end_date(end_date) return Q( domain__permissions__isnull=False, @@ -1095,10 +1103,14 @@ def get_prefetch_related(cls): return ["permissions"] @classmethod - def get_filter_conditions(cls, start_date=None, end_date=None, **kwargs): + def get_filter_conditions(cls, end_date=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ + if not end_date: + # Return nothing + return Q(id__in=[]) + end_date_formatted = format_end_date(end_date) return Q( domain__permissions__isnull=True, @@ -1331,6 +1343,9 @@ def get_filter_conditions(cls, start_date=None, end_date=None, **kwargs): """ Get a Q object of filter conditions to filter when building queryset. """ + if not start_date or not end_date: + # Return nothing + return Q(id__in=[]) start_date_formatted = format_start_date(start_date) end_date_formatted = format_end_date(end_date) From 9374d91c4b05370527b325fe2ded4e4b3dfc9284 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Nov 2024 12:41:29 -0700 Subject: [PATCH 15/45] Fix failing tests (hopefully) --- src/registrar/utility/csv_export.py | 12 ++++++------ src/registrar/utility/model_annotations.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index a85732a3e..cf9cd5a2b 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -71,7 +71,7 @@ def get_columns(cls): return [] @classmethod - def write_csv_before(cls, csv_writer, **export_kwargs): + def write_csv_before(cls, csv_writer, **kwargs): """ Write to csv file before the write_csv method. Override in subclasses where needed. @@ -79,20 +79,20 @@ def write_csv_before(cls, csv_writer, **export_kwargs): pass @classmethod - def export_data_to_csv(cls, csv_file, request=None, **export_kwargs): + def export_data_to_csv(cls, csv_file, **kwargs): """ All domain metadata: Exports domains of all statuses plus domain managers. """ writer = csv.writer(csv_file) columns = cls.get_columns() - model_dict = cls.get_model_dict(request=request) + models_dict = cls.get_model_annotation_dict(**kwargs) # Write to csv file before the write_csv - cls.write_csv_before(writer, **export_kwargs) + cls.write_csv_before(writer, **kwargs) # Write the csv file - rows = cls.write_csv(writer, columns, model_dict) + rows = cls.write_csv(writer, columns, models_dict) # Return rows that for easier parsing and testing return rows @@ -145,7 +145,7 @@ def model(self): return None @classmethod - def get_model_dict(cls, request=None): + def get_model_annotation_dict(cls, request=None, **kwargs): portfolio = request.session.get("portfolio") if not portfolio: return {} diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index 5fc868462..38f00b072 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -195,7 +195,7 @@ def update_queryset(cls, queryset, **kwargs): return queryset @classmethod - def get_model_dict(cls, **kwargs): + def get_model_annotation_dict(cls, **kwargs): return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) From 7075b72533a464b1b34682fc5598c5fbcea97b22 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:30:36 -0700 Subject: [PATCH 16/45] lint + add invitation script --- ...te_user_portfolio_permission_invitation.py | 27 +++++++ .../models/user_portfolio_permission.py | 17 ++-- src/registrar/models/utility/orm_helper.py | 4 +- .../templates/includes/members_table.html | 4 +- src/registrar/utility/csv_export.py | 39 ++++++++-- src/registrar/utility/model_annotations.py | 78 ++++++++++--------- src/registrar/views/portfolio_members_json.py | 5 +- src/registrar/views/portfolios.py | 15 ++++ 8 files changed, 133 insertions(+), 56 deletions(-) create mode 100644 src/registrar/management/commands/populate_user_portfolio_permission_invitation.py diff --git a/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py b/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py new file mode 100644 index 000000000..6e72792ac --- /dev/null +++ b/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py @@ -0,0 +1,27 @@ +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/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 5a26f350e..424f09a17 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -115,16 +115,19 @@ def get_portfolio_permissions(cls, roles, additional_permissions): if additional_permissions: portfolio_permissions.update(additional_permissions) return list(portfolio_permissions) - + @classmethod def get_domain_request_permission_display(cls, roles, additional_permissions): """Class method to return a readable string for domain request permissions""" # Tracks if they can view, create requests, or not do anything all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, additional_permissions) - all_domain_perms = [UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS] - if (all(perm in all_permissions for perm in all_domain_perms)): + all_domain_perms = [ + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.EDIT_REQUESTS, + ] + if all(perm in all_permissions for perm in all_domain_perms): return "Viewer Requester" - elif (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS in all_permissions): + elif UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS in all_permissions: return "Viewer" else: return "None" @@ -134,11 +137,11 @@ def get_member_permission_display(cls, roles, additional_permissions): """Class method to return a readable string for member permissions""" # Tracks if they can view, create requests, or not do anything all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, additional_permissions) - # Note for reviewers: the reason why this isn't checking on "all" is because + # Note for reviewers: the reason why this isn't checking on "all" is because # the way perms work for members is different than requests. We need to consolidate this. - if (UserPortfolioPermissionChoices.EDIT_MEMBERS in all_permissions): + if UserPortfolioPermissionChoices.EDIT_MEMBERS in all_permissions: return "Manager" - elif (UserPortfolioPermissionChoices.VIEW_MEMBERS in all_permissions): + elif UserPortfolioPermissionChoices.VIEW_MEMBERS in all_permissions: return "Viewer" else: return "None" diff --git a/src/registrar/models/utility/orm_helper.py b/src/registrar/models/utility/orm_helper.py index 24f7982e7..4f4665216 100644 --- a/src/registrar/models/utility/orm_helper.py +++ b/src/registrar/models/utility/orm_helper.py @@ -1,6 +1,8 @@ from django.db.models.expressions import Func + class ArrayRemove(Func): """Custom Func to use array_remove to remove null values""" + function = "array_remove" - template = "%(function)s(%(expressions)s, NULL)" \ No newline at end of file + template = "%(function)s(%(expressions)s, NULL)" diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index ae430501d..09cebda2e 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -36,7 +36,7 @@
- {% comment %} {% if user_domain_count and user_domain_count > 0 %} {% endcomment %} + {% if member_count and member_count > 0 %}
@@ -46,7 +46,7 @@
- {% comment %} {% endif %} {% endcomment %} + {% endif %} diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index cf9cd5a2b..76a84094c 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -11,7 +11,21 @@ PublicContact, UserDomainRole, ) -from django.db.models import Case, CharField, Count, DateField, F, ManyToManyField, Q, QuerySet, Value, When, TextField, OuterRef, Subquery +from django.db.models import ( + Case, + CharField, + Count, + DateField, + F, + ManyToManyField, + Q, + QuerySet, + Value, + When, + TextField, + OuterRef, + Subquery, +) from django.db.models.functions import Cast from django.utils import timezone from django.db.models.functions import Concat, Coalesce @@ -25,7 +39,11 @@ from registrar.utility.enums import DefaultEmail from django.contrib.postgres.aggregates import ArrayAgg -from registrar.utility.model_annotations import BaseModelAnnotation, PortfolioInvitationModelAnnotation, UserPortfolioPermissionModelAnnotation +from registrar.utility.model_annotations import ( + BaseModelAnnotation, + PortfolioInvitationModelAnnotation, + UserPortfolioPermissionModelAnnotation, +) logger = logging.getLogger(__name__) @@ -134,6 +152,7 @@ def parse_row(cls, columns, model): """ pass + class MemberExport(BaseExport): @classmethod @@ -143,7 +162,7 @@ def model(self): This is a special edge case, but the base report requires this to be defined. """ return None - + @classmethod def get_model_annotation_dict(cls, request=None, **kwargs): portfolio = request.session.get("portfolio") @@ -166,8 +185,12 @@ def get_model_annotation_dict(cls, request=None, **kwargs): "invitation_date", "invited_by", ] - permissions = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values(*shared_columns) - invitations = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values(*shared_columns) + permissions = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values( + *shared_columns + ) + invitations = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values( + *shared_columns + ) queryset_dict = convert_queryset_to_dict(permissions.union(invitations), is_model=False) return queryset_dict @@ -199,7 +222,9 @@ def parse_row(cls, columns, model): roles = model.get("roles") additional_permissions = model.get("additional_permissions_display") is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (roles or []) - domain_request_display = UserPortfolioPermission.get_domain_request_permission_display(roles, additional_permissions) + domain_request_display = UserPortfolioPermission.get_domain_request_permission_display( + roles, additional_permissions + ) member_perm_display = UserPortfolioPermission.get_member_permission_display(roles, additional_permissions) user_managed_domains = model.get("domain_info", []) managed_domains_as_csv = ",".join(user_managed_domains) @@ -231,6 +256,7 @@ def parse_row(cls, columns, model): row = [FIELDS.get(column, "") for column in columns] return row + class DomainExport(BaseExport): """ A collection of functions which return csv files regarding Domains. Although class is @@ -1521,4 +1547,3 @@ def get_creator_active_requests_count_query(cls): distinct=True, ) return query - diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index 38f00b072..dc6e6ea87 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -20,12 +20,26 @@ permissions = UserPortfolioPermissionAnnotation.get_annotated_queryset(portfolio, csv_report=True) # Returns same fields but formatted for CSV export """ + from abc import ABC, abstractmethod from registrar.models import ( DomainInvitation, PortfolioInvitation, ) -from django.db.models import CharField, F, ManyToManyField, Q, QuerySet, Value, TextField, OuterRef, Subquery, Func, Case, When +from django.db.models import ( + CharField, + F, + ManyToManyField, + Q, + QuerySet, + Value, + TextField, + OuterRef, + Subquery, + Func, + Case, + When, +) from django.db.models.functions import Concat, Coalesce, Cast from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.generic_helper import convert_queryset_to_dict @@ -39,9 +53,9 @@ class BaseModelAnnotation(ABC): """ Abstract base class that standardizes how models are annotated for csv exports or complex annotation queries. For example, the Members table / csv export. - + Subclasses define model-specific annotations, filters, and field formatting while inheriting - common queryset building logic. + common queryset building logic. Intended ensure consistent data presentation across both table UI components and CSV exports. """ @@ -118,7 +132,7 @@ def get_related_table_fields(cls): Get a list of fields from related tables. """ return [] - + @classmethod def annotate_and_retrieve_fields( cls, initial_queryset, annotated_fields, related_table_fields=None, include_many_to_many=False, **kwargs @@ -174,8 +188,7 @@ def get_annotated_queryset(cls, **kwargs): model_queryset = ( cls.model() - .objects - .select_related(*select_related) + .objects.select_related(*select_related) .prefetch_related(*prefetch_related) .filter(filter_conditions) .exclude(exclusions) @@ -183,9 +196,7 @@ def get_annotated_queryset(cls, **kwargs): .order_by(*sort_fields) .distinct() ) - return cls.annotate_and_retrieve_fields( - model_queryset, annotated_fields, related_table_fields, **kwargs - ) + return cls.annotate_and_retrieve_fields(model_queryset, annotated_fields, related_table_fields, **kwargs) @classmethod def update_queryset(cls, queryset, **kwargs): @@ -193,7 +204,7 @@ def update_queryset(cls, queryset, **kwargs): Returns an updated queryset. Override in subclass to update queryset. """ return queryset - + @classmethod def get_model_annotation_dict(cls, **kwargs): return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) @@ -205,6 +216,7 @@ class UserPortfolioPermissionModelAnnotation(BaseModelAnnotation): Handles formatting of user details, permissions, and related domain information for both UI display and CSV export. """ + @classmethod def model(cls): # Return the model class that this export handles @@ -247,10 +259,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): if csv_report: domain_query = F("user__permissions__domain__name") last_active_query = Func( - F("user__last_login"), - Value("YYYY-MM-DD"), - function="to_char", - output_field=TextField() + F("user__last_login"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField() ) else: domain_query = Concat( @@ -272,10 +281,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): ), "additional_permissions_display": F("additional_permissions"), "member_display": Case( - When( - Q(user__email__isnull=False) & ~Q(user__email=""), - then=F("user__email") - ), + When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), When( Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), then=Concat( @@ -290,17 +296,12 @@ def get_annotated_fields(cls, portfolio, csv_report=False): "domain_info": ArrayAgg( domain_query, distinct=True, - filter=Q(user__permissions__domain__isnull=False) + filter=Q(user__permissions__domain__isnull=False) & 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() - ), + Func(F("invitation__created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField()), Value("Invalid date"), output_field=TextField(), ), @@ -311,12 +312,16 @@ def get_annotated_fields(cls, portfolio, csv_report=False): Subquery( LogEntry.objects.filter( content_type=ContentType.objects.get_for_model(PortfolioInvitation), - object_id=Cast(OuterRef("invitation__id"), output_field=TextField()), # Look up the invitation's ID - action_flag=ADDITION - ).order_by("action_time").values("user__email")[:1] + object_id=Cast( + OuterRef("invitation__id"), output_field=TextField() + ), # Look up the invitation's ID + action_flag=ADDITION, + ) + .order_by("action_time") + .values("user__email")[:1] ), Value("Unknown"), - output_field=CharField() + output_field=CharField(), ), } @@ -394,12 +399,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): ), "source": Value("invitation", output_field=CharField()), "invitation_date": Coalesce( - Func( - F("created_at"), - Value("YYYY-MM-DD"), - function="to_char", - output_field=TextField() - ), + Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField()), Value("Invalid date"), output_field=TextField(), ), @@ -412,11 +412,13 @@ def get_annotated_fields(cls, portfolio, csv_report=False): content_type=ContentType.objects.get_for_model(PortfolioInvitation), # Look up the invitation's ID. LogEntry expects a string as this it is stored as json. object_id=Cast(OuterRef("id"), output_field=TextField()), - action_flag=ADDITION - ).order_by("action_time").values("user__email")[:1] + action_flag=ADDITION, + ) + .order_by("action_time") + .values("user__email")[:1] ), Value("Unknown"), - output_field=CharField() + output_field=CharField(), ), } diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 2fe0492d6..ebe537247 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -9,7 +9,10 @@ from registrar.models import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from registrar.utility.model_annotations import PortfolioInvitationModelAnnotation, UserPortfolioPermissionModelAnnotation +from registrar.utility.model_annotations import ( + PortfolioInvitationModelAnnotation, + UserPortfolioPermissionModelAnnotation, +) from registrar.views.utility.mixins import PortfolioMembersPermission diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 7839d209e..2ad36b71e 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -386,6 +386,21 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View): def get(self, request): """Add additional context data to the template.""" return render(request, "portfolio_members.html") + + def get_context_data(self, **kwargs): + """Add additional context data to the template.""" + + context = super().get_context_data(**kwargs) + portfolio = self.request.session.get("portfolio") + user_count = portfolio.portfolio_users.count() + invitation_count = PortfolioInvitation.objects.filter( + portfolio=portfolio + ).count() + context["member_count"] = user_count + invitation_count + + # check if any portfolio invitations exist 4 portfolio + # check if any userportfolioroles exist 4 portfolio + return context class NewMemberView(PortfolioMembersPermissionView, FormMixin): From 3b378d3c81fbcf53753067260343ebecda8279d1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Nov 2024 11:12:43 -0700 Subject: [PATCH 17/45] Cleanup --- src/registrar/admin.py | 11 ++- ...te_user_portfolio_permission_invitation.py | 13 +++- src/registrar/utility/model_annotations.py | 68 ++++++++++--------- src/registrar/views/portfolio_members_json.py | 5 +- src/registrar/views/portfolios.py | 28 ++++---- 5 files changed, 69 insertions(+), 56 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2d05e760b..1679eeed2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1268,6 +1268,15 @@ class Meta: _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", @@ -1275,8 +1284,6 @@ class Meta: "get_roles", ] - readonly_fields = ["invitation"] - autocomplete_fields = ["user", "portfolio"] search_fields = ["user__first_name", "user__last_name", "user__email", "portfolio__organization_name"] search_help_text = "Search by first name, last name, email, or portfolio." diff --git a/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py b/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py index 6e72792ac..7a73f7710 100644 --- a/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py +++ b/src/registrar/management/commands/populate_user_portfolio_permission_invitation.py @@ -13,15 +13,22 @@ class Command(BaseCommand, PopulateScriptTemplate): 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') + 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() + 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() + return ( + not record + or not self.existing_invitations.filter(email=record.user.email, portfolio=record.portfolio).exists() + ) diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index dc6e6ea87..105296855 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -39,8 +39,10 @@ Func, Case, When, + Exists, ) from django.db.models.functions import Concat, Coalesce, Cast +from registrar.models.user_group import UserGroup from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.models.utility.orm_helper import ArrayRemove @@ -305,23 +307,8 @@ def get_annotated_fields(cls, portfolio, csv_report=False): Value("Invalid date"), 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": Coalesce( - Subquery( - LogEntry.objects.filter( - content_type=ContentType.objects.get_for_model(PortfolioInvitation), - object_id=Cast( - OuterRef("invitation__id"), output_field=TextField() - ), # Look up the invitation's ID - action_flag=ADDITION, - ) - .order_by("action_time") - .values("user__email")[:1] - ), - Value("Unknown"), - output_field=CharField(), + "invited_by": PortfolioInvitationModelAnnotation.get_invited_by_from_audit_log_query( + object_id_query=Cast(OuterRef("invitation__id"), output_field=TextField()) ), } @@ -362,6 +349,37 @@ def get_filter_conditions(cls, portfolio, **kwargs): # Get all members on this portfolio return Q(portfolio=portfolio) + @classmethod + def get_invited_by_from_audit_log_query(cls, 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( + display_email=Case( + When( + Exists( + UserGroup.objects.filter( + name__in=["cisa_analysts_group", "full_access_group"], + user=OuterRef("user"), + ) + ), + then=Value("help@get.gov") + ), + default=F("user__email"), + output_field=CharField() + ) + ) + .order_by("action_time") + .values("display_email") + ), + Value("Unknown"), + output_field=CharField(), + ) + @classmethod def get_annotated_fields(cls, portfolio, csv_report=False): """ @@ -383,7 +401,6 @@ def get_annotated_fields(cls, portfolio, csv_report=False): email=OuterRef("email"), # Check if email matches the OuterRef("email") domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio ).annotate(domain_info=domain_query) - return { "first_name": Value(None, output_field=CharField()), "last_name": Value(None, output_field=CharField()), @@ -406,19 +423,8 @@ def get_annotated_fields(cls, portfolio, csv_report=False): # 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": Coalesce( - Subquery( - LogEntry.objects.filter( - content_type=ContentType.objects.get_for_model(PortfolioInvitation), - # Look up the invitation's ID. LogEntry expects a string as this it is stored as json. - object_id=Cast(OuterRef("id"), output_field=TextField()), - action_flag=ADDITION, - ) - .order_by("action_time") - .values("user__email")[:1] - ), - Value("Unknown"), - output_field=CharField(), + "invited_by": cls.get_invited_by_from_audit_log_query( + object_id_query=Cast(OuterRef("id"), output_field=TextField()) ), } diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index ebe537247..3bf761858 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -1,9 +1,6 @@ from django.http import JsonResponse from django.core.paginator import Paginator -from django.db.models import Value, F, CharField, TextField, Q, Case, When, OuterRef, Subquery -from django.db.models.expressions import Func -from django.db.models.functions import Cast, Coalesce, Concat -from django.contrib.postgres.aggregates import ArrayAgg +from django.db.models import F, Q from django.urls import reverse from django.views import View diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 2ad36b71e..09ff159c5 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -385,22 +385,18 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View): def get(self, request): """Add additional context data to the template.""" - return render(request, "portfolio_members.html") - - def get_context_data(self, **kwargs): - """Add additional context data to the template.""" - - context = super().get_context_data(**kwargs) - portfolio = self.request.session.get("portfolio") - user_count = portfolio.portfolio_users.count() - invitation_count = PortfolioInvitation.objects.filter( - portfolio=portfolio - ).count() - context["member_count"] = user_count + invitation_count - - # check if any portfolio invitations exist 4 portfolio - # check if any userportfolioroles exist 4 portfolio - return context + # Get portfolio from session + portfolio = request.session.get("portfolio") + context = {} + if portfolio: + user_count = portfolio.portfolio_users.count() + invitation_count = PortfolioInvitation.objects.filter( + portfolio=portfolio + ).count() + context.update({ + "member_count": user_count + invitation_count + }) + return render(request, "portfolio_members.html", context=context) class NewMemberView(PortfolioMembersPermissionView, FormMixin): From 1c0f99eedc2f39502ecdde28df6dc011cc8af1c6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:01:00 -0700 Subject: [PATCH 18/45] Use audit log instead --- src/registrar/admin.py | 9 --- ...te_user_portfolio_permission_invitation.py | 34 --------- ...0138_userportfoliopermission_invitation.py | 25 ------- src/registrar/models/portfolio_invitation.py | 4 +- .../models/user_portfolio_permission.py | 10 --- src/registrar/utility/model_annotations.py | 70 +++++++++++++++---- 6 files changed, 58 insertions(+), 94 deletions(-) delete mode 100644 src/registrar/management/commands/populate_user_portfolio_permission_invitation.py delete mode 100644 src/registrar/migrations/0138_userportfoliopermission_invitation.py 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 Meta: _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 .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 @@ def retrieve(self): # 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 Meta: 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 @@ def get_filter_conditions(cls, portfolio, **kwargs): # 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 @@ def get_annotated_fields(cls, portfolio, csv_report=False): & 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 @@ def get_filter_conditions(cls, portfolio, **kwargs): 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 @@ def get_invited_by_from_audit_log_query(cls, object_id_query): ) ) .order_by("action_time") - .values("display_email") + .values("display_email")[:1] ), Value("Unknown"), output_field=CharField(), @@ -415,15 +461,13 @@ def get_annotated_fields(cls, portfolio, csv_report=False): ) ), "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()) ), } From 5f5bc4f616c95d2f739790614f53ed5de24961a9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:17:19 -0700 Subject: [PATCH 19/45] lint Just need unit tests! --- src/registrar/models/utility/orm_helper.py | 2 +- .../templates/includes/members_table.html | 2 +- src/registrar/utility/csv_export.py | 12 ++------ src/registrar/utility/model_annotations.py | 30 +++++++++---------- src/registrar/views/portfolios.py | 8 ++--- 5 files changed, 20 insertions(+), 34 deletions(-) diff --git a/src/registrar/models/utility/orm_helper.py b/src/registrar/models/utility/orm_helper.py index 4f4665216..63ff41d28 100644 --- a/src/registrar/models/utility/orm_helper.py +++ b/src/registrar/models/utility/orm_helper.py @@ -1,7 +1,7 @@ from django.db.models.expressions import Func -class ArrayRemove(Func): +class ArrayRemoveNull(Func): """Custom Func to use array_remove to remove null values""" function = "array_remove" diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index 09cebda2e..c0c6d803c 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -8,7 +8,7 @@
- diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 664a038f2..c1a11d85c 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -812,7 +812,7 @@ def setUp(self): @override_flag("organization_members", active=True) @less_console_noise_decorator def test_member_export(self): - """Tests the member export report""" + """Tests the member export report by comparing the csv output.""" content_type = ContentType.objects.get_for_model(PortfolioInvitation) LogEntry.objects.create( user=self.lebowski_user, diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 537d61538..ab806f81a 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -175,7 +175,7 @@ def get_model_annotation_dict(cls, request=None, **kwargs): "additional_permissions_display", "member_display", "domain_info", - "source", + "type", "invitation_date", "invited_by", ] diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index 6fff51c2c..453653b52 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -280,10 +280,12 @@ def get_annotated_fields(cls, portfolio, csv_report=False): F("user__last_login"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField() ) else: + # an array of domains, with id and name, colon separated domain_query = Concat( F("user__permissions__domain_id"), Value(":"), F("user__permissions__domain__name"), + # specify the output_field to ensure union has same column types output_field=CharField(), ) last_active_query = Cast(F("user__last_login"), output_field=TextField()) @@ -299,7 +301,9 @@ def get_annotated_fields(cls, portfolio, csv_report=False): ), "additional_permissions_display": F("additional_permissions"), "member_display": Case( + # If email is present and not blank, use email When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), + # If first name or last name is present, use concatenation of first_name + " " + last_name When( Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), then=Concat( @@ -308,16 +312,18 @@ def get_annotated_fields(cls, portfolio, csv_report=False): Coalesce(F("user__last_name"), Value("")), ), ), + # If neither, use an empty string default=Value(""), output_field=CharField(), ), "domain_info": ArrayAgg( domain_query, distinct=True, + # only include domains in portfolio filter=Q(user__permissions__domain__isnull=False) & Q(user__permissions__domain__domain_info__portfolio=portfolio), ), - "source": Value("permission", output_field=CharField()), + "type": Value("member", output_field=CharField()), "invitation_date": PortfolioInvitationModelAnnotation.get_invitation_date_query( object_id_query=cls.get_portfolio_invitation_id_query() ), @@ -452,13 +458,14 @@ def get_annotated_fields(cls, portfolio, csv_report=False): "last_active": Value("Invited", output_field=TextField()), "additional_permissions_display": F("additional_permissions"), "member_display": F("email"), + # Use ArrayRemove to return an empty list when no domain invitations are found "domain_info": ArrayRemoveNull( ArrayAgg( Subquery(domain_invitations.values("domain_info")), distinct=True, ) ), - "source": Value("invitation", output_field=CharField()), + "type": Value("invitedmember", output_field=CharField()), "invitation_date": cls.get_invitation_date_query( object_id_query=Cast(OuterRef("id"), output_field=TextField()) ), diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 3bf761858..bf89dcd82 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -66,7 +66,7 @@ def initial_permissions_search(self, portfolio): "additional_permissions_display", "member_display", "domain_info", - "source", + "type", ) def initial_invitations_search(self, portfolio): @@ -83,7 +83,7 @@ def initial_invitations_search(self, portfolio): "additional_permissions_display", "member_display", "domain_info", - "source", + "type", ) def apply_search_term(self, queryset, request): @@ -119,12 +119,12 @@ def serialize_members(self, portfolio, item, user): view_only = not user.has_edit_members_portfolio_permission(portfolio) or not user_can_edit_other_users is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (item.get("roles") or []) - action_url = reverse("member" if item["source"] == "permission" else "invitedmember", kwargs={"pk": item["id"]}) + action_url = reverse(item["type"], kwargs={"pk": item["id"]}) # Serialize member data member_json = { - "id": item.get("id", ""), - "source": item.get("source", ""), + "id": item.get("id", ""), # id is id of UserPortfolioPermission or PortfolioInvitation + "type": item.get("type", ""), # source is member or invitedmember "name": " ".join(filter(None, [item.get("first_name", ""), item.get("last_name", "")])), "email": item.get("email_display", ""), "member_display": item.get("member_display", ""), diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index d35ae20dd..373d1619d 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -461,14 +461,7 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View): def get(self, request): """Add additional context data to the template.""" - # Get portfolio from session - portfolio = request.session.get("portfolio") - context = {} - if portfolio: - user_count = portfolio.portfolio_users.count() - invitation_count = PortfolioInvitation.objects.filter(portfolio=portfolio).count() - context.update({"member_count": user_count + invitation_count}) - return render(request, "portfolio_members.html", context=context) + return render(request, "portfolio_members.html") class NewMemberView(PortfolioMembersPermissionView, FormMixin): diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index 880de5d79..cff177d6d 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -175,9 +175,10 @@ class ExportMembersPortfolio(View): def get(self, request, *args, **kwargs): """Returns the members report""" - portfolio_display = "portfolio" + # Swap the spaces for dashes to make the formatted name look prettier + portfolio_display = "organization" if request.session.get("portfolio"): - portfolio_display = str(request.session.get("portfolio")).replace(" ", "-") + portfolio_display = str(request.session.get("portfolio")).lower().replace(" ", "-") response = HttpResponse(content_type="text/csv") response["Content-Disposition"] = f'attachment; filename="members-for-{portfolio_display}.csv"' From b791daf10cbeeaf82c19e6caa036e00fcc7897db Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 20 Nov 2024 10:04:24 -0700 Subject: [PATCH 29/45] Guard report behind perms check Not strictly necessary as we check anyway, but double security --- src/registrar/views/report_views.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index cff177d6d..56867216e 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -174,11 +174,23 @@ class ExportMembersPortfolio(View): def get(self, request, *args, **kwargs): """Returns the members report""" + portfolio = request.session.get("portfolio") + + # Check if the user has organization access + if not request.user.is_org_user(request): + return render(request, "403.html", status=403) + + # Check if the user has member permissions + if ( + not request.user.has_view_members_portfolio_permission(portfolio) + and not request.user.has_edit_members_portfolio_permission(portfolio) + ): + return render(request, "403.html", status=403) # Swap the spaces for dashes to make the formatted name look prettier portfolio_display = "organization" - if request.session.get("portfolio"): - portfolio_display = str(request.session.get("portfolio")).lower().replace(" ", "-") + if portfolio: + portfolio_display = str(portfolio).lower().replace(" ", "-") response = HttpResponse(content_type="text/csv") response["Content-Disposition"] = f'attachment; filename="members-for-{portfolio_display}.csv"' From 4ab526ceb5629c2bfc20e0366d4c2b19e0567669 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 20 Nov 2024 10:50:27 -0700 Subject: [PATCH 30/45] Lint! --- src/registrar/views/report_views.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index 56867216e..1b1798d69 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -179,12 +179,11 @@ def get(self, request, *args, **kwargs): # Check if the user has organization access if not request.user.is_org_user(request): return render(request, "403.html", status=403) - + # Check if the user has member permissions - if ( - not request.user.has_view_members_portfolio_permission(portfolio) - and not request.user.has_edit_members_portfolio_permission(portfolio) - ): + if not request.user.has_view_members_portfolio_permission( + portfolio + ) and not request.user.has_edit_members_portfolio_permission(portfolio): return render(request, "403.html", status=403) # Swap the spaces for dashes to make the formatted name look prettier From 981cdb31b69458fb505897885a9391ae339f277c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:29:53 -0700 Subject: [PATCH 31/45] Replace invitation date => joined date --- src/registrar/tests/test_reports.py | 42 +++++++++++++--------- src/registrar/utility/csv_export.py | 6 ++-- src/registrar/utility/model_annotations.py | 38 ++------------------ 3 files changed, 31 insertions(+), 55 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index c1a11d85c..82942603a 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -805,6 +805,7 @@ def test_domain_request_data_full(self): class MemberExportTest(MockDbForIndividualTests, MockEppLib): def setUp(self): + """Override of the base setUp to add a request factory""" super().setUp() self.factory = RequestFactory() @@ -813,6 +814,12 @@ def setUp(self): @less_console_noise_decorator def test_member_export(self): """Tests the member export report by comparing the csv output.""" + # == Data setup == # + # Set last_login for some users + active_date = timezone.make_aware(datetime(2024, 2, 1)) + User.objects.filter(id__in=[self.custom_superuser.id, self.custom_staffuser.id]).update(last_login=active_date) + + # Create a logentry for meoward, created by lebowski to test invited_by. content_type = ContentType.objects.get_for_model(PortfolioInvitation) LogEntry.objects.create( user=self.lebowski_user, @@ -824,8 +831,7 @@ def test_member_export(self): action_time=timezone.make_aware(datetime(2023, 4, 12)), ) - # Create log entries for each remaining invitation. - # Exclude meoward and tired_user (to test null dates, etc). + # Create log entries for each remaining invitation. Exclude meoward and tired_user. for invitation in PortfolioInvitation.objects.exclude( id__in=[self.portfolio_invitation_1.id, self.portfolio_invitation_3.id] ): @@ -838,9 +844,7 @@ def test_member_export(self): change_message="Created invitation", action_time=timezone.make_aware(datetime(2024, 1, 15)), ) - # Set last_login for some users - active_date = timezone.make_aware(datetime(2024, 2, 1)) - User.objects.filter(id__in=[self.custom_superuser.id, self.custom_staffuser.id]).update(last_login=active_date) + # Retrieve invitations with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): self.meoward_user.check_portfolio_invitations_on_login() @@ -849,6 +853,12 @@ def test_member_export(self): self.custom_superuser.check_portfolio_invitations_on_login() self.custom_staffuser.check_portfolio_invitations_on_login() + # Update the created at date on UserPortfolioPermission, so we can test a consistent date. + UserPortfolioPermission.objects.filter(portfolio=self.portfolio_1).update( + created_at=timezone.make_aware(datetime(2022, 4, 1)) + ) + # == End of data setup == # + # Create a request and add the user to the request request = self.factory.get("/") request.user = self.user @@ -867,20 +877,20 @@ def test_member_export(self): csv_content = csv_file.read() expected_content = ( # Header - "Email,Organization admin,Invited by,Invitation date,Last active,Domain requests," + "Email,Organization admin,Invited by,Joined date,Last active,Domain requests," "Member management,Domain management,Number of domains,Domains\n" # Content - "meoward@rocks.com,False,big_lebowski@dude.co,2023-04-12,Invalid date,None," + "meoward@rocks.com,False,big_lebowski@dude.co,2022-04-01,Invalid date,None," 'Manager,True,2,"adomain2.gov,cdomain1.gov"\n' - "big_lebowski@dude.co,False,help@get.gov,2024-01-15,Invalid date,None,Viewer,True,1,cdomain1.gov\n" - "tired_sleepy@igorville.gov,False,System,Unknown,Invalid date,Viewer,None,False,0,\n" - "icy_superuser@igorville.gov,True,help@get.gov,2024-01-15,2024-02-01,Viewer Requester,Manager,False,0,\n" - "cozy_staffuser@igorville.gov,True,help@get.gov,2024-01-15,2024-02-01,Viewer Requester,None,False,0,\n" - "nonexistentmember_1@igorville.gov,False,help@get.gov,2024-01-15,Invited,None,Manager,False,0,\n" - "nonexistentmember_2@igorville.gov,False,help@get.gov,2024-01-15,Invited,None,Viewer,False,0,\n" - "nonexistentmember_3@igorville.gov,False,help@get.gov,2024-01-15,Invited,Viewer,None,False,0,\n" - "nonexistentmember_4@igorville.gov,True,help@get.gov,2024-01-15,Invited,Viewer Requester,Manager,False,0,\n" - "nonexistentmember_5@igorville.gov,True,help@get.gov,2024-01-15,Invited,Viewer Requester,None,False,0,\n" + "big_lebowski@dude.co,False,help@get.gov,2022-04-01,Invalid date,None,Viewer,True,1,cdomain1.gov\n" + "tired_sleepy@igorville.gov,False,System,2022-04-01,Invalid date,Viewer,None,False,0,\n" + "icy_superuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer Requester,Manager,False,0,\n" + "cozy_staffuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer Requester,None,False,0,\n" + "nonexistentmember_1@igorville.gov,False,help@get.gov,Unretrieved,Invited,None,Manager,False,0,\n" + "nonexistentmember_2@igorville.gov,False,help@get.gov,Unretrieved,Invited,None,Viewer,False,0,\n" + "nonexistentmember_3@igorville.gov,False,help@get.gov,Unretrieved,Invited,Viewer,None,False,0,\n" + "nonexistentmember_4@igorville.gov,True,help@get.gov,Unretrieved,Invited,Viewer Requester,Manager,False,0,\n" + "nonexistentmember_5@igorville.gov,True,help@get.gov,Unretrieved,Invited,Viewer Requester,None,False,0,\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index ab806f81a..bd666923c 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -176,7 +176,7 @@ def get_model_annotation_dict(cls, request=None, **kwargs): "member_display", "domain_info", "type", - "invitation_date", + "joined_date", "invited_by", ] permissions = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values( @@ -197,7 +197,7 @@ def get_columns(cls): "Email", "Organization admin", "Invited by", - "Invitation date", + "Joined date", "Last active", "Domain requests", "Member management", @@ -226,7 +226,7 @@ def parse_row(cls, columns, model): "Email": model.get("email_display"), "Organization admin": is_admin, "Invited by": model.get("invited_by"), - "Invitation date": model.get("invitation_date"), + "Joined date": model.get("joined_date"), "Last active": model.get("last_active"), "Domain requests": domain_request_display, "Member management": member_perm_display, diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index 453653b52..f0ef70396 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -324,9 +324,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): & Q(user__permissions__domain__domain_info__portfolio=portfolio), ), "type": Value("member", output_field=CharField()), - "invitation_date": PortfolioInvitationModelAnnotation.get_invitation_date_query( - object_id_query=cls.get_portfolio_invitation_id_query() - ), + "joined_date": Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField()), "invited_by": PortfolioInvitationModelAnnotation.get_invited_by_query( object_id_query=cls.get_portfolio_invitation_id_query() ), @@ -369,33 +367,6 @@ def get_filter_conditions(cls, portfolio, **kwargs): # Get all members on this portfolio return Q(portfolio=portfolio) - @classmethod - 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. - # Using this instead of created_at is a lot simpler and more performant, - # as otherwise a Case and Subquery need to be used. - 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("Unknown"), - output_field=TextField(), - ) - @classmethod def get_invited_by_query(cls, object_id_query): """Returns the user that created the given portfolio invitation. @@ -466,12 +437,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): ) ), "type": Value("invitedmember", output_field=CharField()), - "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") + "joined_date": Value("Unretrieved", output_field=CharField()), "invited_by": cls.get_invited_by_query(object_id_query=Cast(OuterRef("id"), output_field=TextField())), } From 3f8db08fb82936a21360292d611c3028e7568d83 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:49:58 -0700 Subject: [PATCH 32/45] Code cleanup --- src/registrar/tests/test_reports.py | 3 +- src/registrar/utility/csv_export.py | 32 +++++++++------------- src/registrar/utility/model_annotations.py | 10 +++---- 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 82942603a..1289c2467 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -889,7 +889,8 @@ def test_member_export(self): "nonexistentmember_1@igorville.gov,False,help@get.gov,Unretrieved,Invited,None,Manager,False,0,\n" "nonexistentmember_2@igorville.gov,False,help@get.gov,Unretrieved,Invited,None,Viewer,False,0,\n" "nonexistentmember_3@igorville.gov,False,help@get.gov,Unretrieved,Invited,Viewer,None,False,0,\n" - "nonexistentmember_4@igorville.gov,True,help@get.gov,Unretrieved,Invited,Viewer Requester,Manager,False,0,\n" + "nonexistentmember_4@igorville.gov,True,help@get.gov,Unretrieved," + "Invited,Viewer Requester,Manager,False,0,\n" "nonexistentmember_5@igorville.gov,True,help@get.gov,Unretrieved,Invited,Viewer Requester,None,False,0,\n" ) # Normalize line endings and remove commas, diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index bd666923c..db5eb1657 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -146,6 +146,8 @@ def parse_row(cls, columns, model): class MemberExport(BaseExport): + """CSV export for the MembersTable. The members table combines the content + of three tables: PortfolioInvitation, UserPortfolioPermission, and DomainInvitation.""" @classmethod def model(self): @@ -185,8 +187,7 @@ def get_model_annotation_dict(cls, request=None, **kwargs): invitations = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values( *shared_columns ) - queryset_dict = convert_queryset_to_dict(permissions.union(invitations), is_model=False) - return queryset_dict + return convert_queryset_to_dict(permissions.union(invitations), is_model=False) @classmethod def get_columns(cls): @@ -213,30 +214,23 @@ def parse_row(cls, columns, model): Given a set of columns and a model dictionary, generate a new row from cleaned column data. Must be implemented by subclasses """ - roles = model.get("roles") - additional_permissions = model.get("additional_permissions_display") - is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (roles or []) - domain_request_display = UserPortfolioPermission.get_domain_request_permission_display( - roles, additional_permissions - ) - member_perm_display = UserPortfolioPermission.get_member_permission_display(roles, additional_permissions) + roles = model.get("roles", []) + permissions = model.get("additional_permissions_display") user_managed_domains = model.get("domain_info", []) - managed_domains_as_csv = ",".join(user_managed_domains) + length_user_managed_domains = len(user_managed_domains) FIELDS = { "Email": model.get("email_display"), - "Organization admin": is_admin, + "Organization admin": bool(UserPortfolioRoleChoices.ORGANIZATION_ADMIN in roles), "Invited by": model.get("invited_by"), "Joined date": model.get("joined_date"), "Last active": model.get("last_active"), - "Domain requests": domain_request_display, - "Member management": member_perm_display, - "Domain management": len(user_managed_domains) > 0, - "Number of domains": len(user_managed_domains), - "Domains": managed_domains_as_csv, + "Domain requests": UserPortfolioPermission.get_domain_request_permission_display(roles, permissions), + "Member management": UserPortfolioPermission.get_member_permission_display(roles, permissions), + "Domain management": bool(length_user_managed_domains > 0), + "Number of domains": length_user_managed_domains, + "Domains": ",".join(user_managed_domains), } - - row = [FIELDS.get(column, "") for column in columns] - return row + return [FIELDS.get(column, "") for column in columns] class DomainExport(BaseExport): diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index f0ef70396..0fc430f4f 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -256,7 +256,7 @@ def get_portfolio_invitation_id_query(cls): portfolio=OuterRef(OuterRef("portfolio")), ).values("id")[:1] ), - output_field=TextField(), + output_field=CharField(), ) @classmethod @@ -297,7 +297,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): "last_active": Coalesce( last_active_query, Value("Invalid date"), - output_field=TextField(), + output_field=CharField(), ), "additional_permissions_display": F("additional_permissions"), "member_display": Case( @@ -324,7 +324,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): & Q(user__permissions__domain__domain_info__portfolio=portfolio), ), "type": Value("member", output_field=CharField()), - "joined_date": Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField()), + "joined_date": Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=CharField()), "invited_by": PortfolioInvitationModelAnnotation.get_invited_by_query( object_id_query=cls.get_portfolio_invitation_id_query() ), @@ -426,7 +426,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): "first_name": Value(None, output_field=CharField()), "last_name": Value(None, output_field=CharField()), "email_display": F("email"), - "last_active": Value("Invited", output_field=TextField()), + "last_active": Value("Invited", output_field=CharField()), "additional_permissions_display": F("additional_permissions"), "member_display": F("email"), # Use ArrayRemove to return an empty list when no domain invitations are found @@ -438,7 +438,7 @@ def get_annotated_fields(cls, portfolio, csv_report=False): ), "type": Value("invitedmember", output_field=CharField()), "joined_date": Value("Unretrieved", output_field=CharField()), - "invited_by": cls.get_invited_by_query(object_id_query=Cast(OuterRef("id"), output_field=TextField())), + "invited_by": cls.get_invited_by_query(object_id_query=Cast(OuterRef("id"), output_field=CharField())), } @classmethod From 4eb54319b783e6b02ed92dd276e570379815de43 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 21 Nov 2024 08:32:42 -0700 Subject: [PATCH 33/45] Update src/registrar/utility/csv_export.py --- src/registrar/utility/csv_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index db5eb1657..f0fb80ed5 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -70,7 +70,7 @@ def format_end_date(end_date): class BaseExport(BaseModelAnnotation): """ A generic class for exporting data which returns a csv file for the given model. - 3rd class in an inheritance tree of 4. + 2nd class in an inheritance tree of 4. """ @classmethod From 859e8b3bfaa16367a9b4e793475e1bc810a03a48 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:26:04 -0700 Subject: [PATCH 34/45] remove model annotations file --- src/registrar/utility/csv_export.py | 310 +++++++++++- src/registrar/utility/model_annotations.py | 447 ------------------ src/registrar/views/portfolio_members_json.py | 115 ++++- 3 files changed, 393 insertions(+), 479 deletions(-) delete mode 100644 src/registrar/utility/model_annotations.py diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index f0fb80ed5..2f255fd49 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -21,6 +21,7 @@ Value, When, ) +from abc import ABC, abstractmethod from django.utils import timezone from django.db.models.functions import Concat, Coalesce from django.contrib.postgres.aggregates import StringAgg @@ -30,13 +31,34 @@ from registrar.templatetags.custom_filters import get_region from registrar.utility.constants import BranchChoices from registrar.utility.enums import DefaultEmail - -from registrar.utility.model_annotations import ( - BaseModelAnnotation, - PortfolioInvitationModelAnnotation, - UserPortfolioPermissionModelAnnotation, +from abc import ABC, abstractmethod +from registrar.models import ( + DomainInvitation, + PortfolioInvitation, ) - +from django.db.models import ( + CharField, + F, + ManyToManyField, + Q, + QuerySet, + Value, + TextField, + OuterRef, + Subquery, + Func, + Case, + When, + Exists, +) +from django.db.models.functions import Concat, Coalesce, Cast +from registrar.models.user_group import UserGroup +from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models.utility.generic_helper import convert_queryset_to_dict +from registrar.models.utility.orm_helper import ArrayRemoveNull +from django.contrib.postgres.aggregates import ArrayAgg +from django.contrib.admin.models import LogEntry, ADDITION +from django.contrib.contenttypes.models import ContentType logger = logging.getLogger(__name__) @@ -67,12 +89,162 @@ def format_end_date(end_date): return timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) if end_date else get_default_end_date() -class BaseExport(BaseModelAnnotation): +class BaseExport(ABC): """ A generic class for exporting data which returns a csv file for the given model. 2nd class in an inheritance tree of 4. """ + @classmethod + @abstractmethod + def model(self): + """ + Property to specify the model that the export class will handle. + Must be implemented by subclasses. + """ + pass + + @classmethod + def get_sort_fields(cls): + """ + Returns the sort fields for the CSV export. Override in subclasses as needed. + """ + return [] + + @classmethod + def get_additional_args(cls): + """ + Returns additional keyword arguments as an empty dictionary. + Override in subclasses to provide specific arguments. + """ + return {} + + @classmethod + def get_select_related(cls): + """ + Get a list of tables to pass to select_related when building queryset. + """ + return [] + + @classmethod + def get_prefetch_related(cls): + """ + Get a list of tables to pass to prefetch_related when building queryset. + """ + return [] + + @classmethod + def get_exclusions(cls): + """ + Get a Q object of exclusion conditions to pass to .exclude() when building queryset. + """ + return Q() + + @classmethod + def get_filter_conditions(cls, **kwargs): + """ + Get a Q object of filter conditions to filter when building queryset. + """ + return Q() + + @classmethod + def get_annotated_fields(cls, **kwargs): + """ + Get a dict of computed fields. These are fields that do not exist on the model normally + and will be passed to .annotate() when building a queryset. + """ + return {} + + @classmethod + def get_annotations_for_sort(cls): + """ + Get a dict of annotations to make available for order_by clause. + """ + return {} + + @classmethod + def get_related_table_fields(cls): + """ + Get a list of fields from related tables. + """ + return [] + + @classmethod + def annotate_and_retrieve_fields( + cls, initial_queryset, annotated_fields, related_table_fields=None, include_many_to_many=False, **kwargs + ) -> QuerySet: + """ + Applies annotations to a queryset and retrieves specified fields, + including class-defined and annotation-defined. + + Parameters: + initial_queryset (QuerySet): Initial queryset. + annotated_fields (dict, optional): Fields to compute {field_name: expression}. + related_table_fields (list, optional): Extra fields to retrieve; defaults to annotation keys if None. + include_many_to_many (bool, optional): Determines if we should include many to many fields or not + **kwargs: Additional keyword arguments for specific parameters (e.g., public_contacts, domain_invitations, + user_domain_roles). + + Returns: + QuerySet: Contains dictionaries with the specified fields for each record. + """ + if related_table_fields is None: + related_table_fields = [] + + # We can infer that if we're passing in annotations, + # we want to grab the result of said annotation. + if annotated_fields: + related_table_fields.extend(annotated_fields.keys()) + + # Get prexisting fields on the model + model_fields = set() + for field in cls.model()._meta.get_fields(): + # Exclude many to many fields unless we specify + many_to_many = isinstance(field, ManyToManyField) and include_many_to_many + if many_to_many or not isinstance(field, ManyToManyField): + model_fields.add(field.name) + + queryset = initial_queryset.annotate(**annotated_fields).values(*model_fields, *related_table_fields) + + return cls.update_queryset(queryset, **kwargs) + + @classmethod + def get_annotated_queryset(cls, **kwargs): + sort_fields = cls.get_sort_fields() + # Get additional args and merge with incoming kwargs + additional_args = cls.get_additional_args() + kwargs.update(additional_args) + select_related = cls.get_select_related() + prefetch_related = cls.get_prefetch_related() + exclusions = cls.get_exclusions() + annotations_for_sort = cls.get_annotations_for_sort() + filter_conditions = cls.get_filter_conditions(**kwargs) + annotated_fields = cls.get_annotated_fields(**kwargs) + related_table_fields = cls.get_related_table_fields() + + model_queryset = ( + cls.model() + .objects.select_related(*select_related) + .prefetch_related(*prefetch_related) + .filter(filter_conditions) + .exclude(exclusions) + .annotate(**annotations_for_sort) + .order_by(*sort_fields) + .distinct() + ) + return cls.annotate_and_retrieve_fields(model_queryset, annotated_fields, related_table_fields, **kwargs) + + @classmethod + def update_queryset(cls, queryset, **kwargs): + """ + Returns an updated queryset. Override in subclass to update queryset. + """ + return queryset + + @classmethod + def get_model_annotation_dict(cls, **kwargs): + return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) + @classmethod def get_columns(cls): """ @@ -181,13 +353,127 @@ def get_model_annotation_dict(cls, request=None, **kwargs): "joined_date", "invited_by", ] - permissions = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values( - *shared_columns + + # Permissions + permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio).select_related("user").annotate( + first_name=F("user__first_name"), + last_name=F("user__last_name"), + email_display=F("user__email"), + last_active=Coalesce( + Func( + F("user__last_login"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField() + ), + Value("Invalid date"), + output_field=CharField(), + ), + additional_permissions_display=F("additional_permissions"), + member_display=Case( + # If email is present and not blank, use email + When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), + # If first name or last name is present, use concatenation of first_name + " " + last_name + When( + Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), + then=Concat( + Coalesce(F("user__first_name"), Value("")), + Value(" "), + Coalesce(F("user__last_name"), Value("")), + ), + ), + # If neither, use an empty string + default=Value(""), + output_field=CharField(), + ), + domain_info=ArrayAgg( + F("user__permissions__domain__name"), + distinct=True, + # only include domains in portfolio + filter=Q(user__permissions__domain__isnull=False) + & Q(user__permissions__domain__domain_info__portfolio=portfolio), + ), + type=Value("member", output_field=CharField()), + joined_date=Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=CharField()), + invited_by=cls.get_invited_by_query( + object_id_query=cls.get_portfolio_invitation_id_query() + ), + ).values(*shared_columns) + + # Invitations + domain_invitations = DomainInvitation.objects.filter( + email=OuterRef("email"), # Check if email matches the OuterRef("email") + domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio + ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) + invitations = PortfolioInvitation.objects.filter(portfolio=portfolio).annotate( + first_name=Value(None, output_field=CharField()), + last_name=Value(None, output_field=CharField()), + email_display=F("email"), + last_active=Value("Invited", output_field=CharField()), + additional_permissions_display=F("additional_permissions"), + member_display=F("email"), + # Use ArrayRemove to return an empty list when no domain invitations are found + domain_info=ArrayRemoveNull( + ArrayAgg( + Subquery(domain_invitations.values("domain_info")), + distinct=True, + ) + ), + type=Value("invitedmember", output_field=CharField()), + joined_date=Value("Unretrieved", output_field=CharField()), + invited_by=cls.get_invited_by_query(object_id_query=Cast(OuterRef("id"), output_field=CharField())), + ).values(*shared_columns) + + return convert_queryset_to_dict(permissions.union(invitations), is_model=False) + + @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( + content_type=ContentType.objects.get_for_model(PortfolioInvitation), + object_id=object_id_query, + action_flag=ADDITION, + ) + .annotate( + display_email=Case( + When( + Exists( + UserGroup.objects.filter( + name__in=["cisa_analysts_group", "full_access_group"], + user=OuterRef("user"), + ) + ), + then=Value("help@get.gov"), + ), + default=F("user__email"), + output_field=CharField(), + ) + ) + .order_by("action_time") + .values("display_email")[:1] + ), + Value("System"), + output_field=CharField(), ) - invitations = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio, csv_report=True).values( - *shared_columns + + @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, + # Double outer ref because we first go into the LogEntry query, + # then into the parent UserPortfolioPermission. + email=OuterRef(OuterRef("user__email")), + portfolio=OuterRef(OuterRef("portfolio")), + ).values("id")[:1] + ), + output_field=CharField(), ) - return convert_queryset_to_dict(permissions.union(invitations), is_model=False) @classmethod def get_columns(cls): diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py deleted file mode 100644 index 0fc430f4f..000000000 --- a/src/registrar/utility/model_annotations.py +++ /dev/null @@ -1,447 +0,0 @@ -""" -Model annotation classes. - -Intended to return django querysets with computed fields for api endpoints and our csv reports. -Used by both API endpoints (e.g. portfolio members JSON) and data exports (e.g. CSV reports). - -Initially created to manage the complexity of the MembersTable and Members CSV report, -as they require complex but common annotations. - -Example: - # For a JSON table endpoint - permissions = UserPortfolioPermissionAnnotation.get_annotated_queryset(portfolio) - # Returns queryset with standardized fields for member tables - - # For a CSV export - permissions = UserPortfolioPermissionAnnotation.get_annotated_queryset(portfolio, csv_report=True) - # Returns same fields but formatted for CSV export -""" - -from abc import ABC, abstractmethod -from registrar.models import ( - DomainInvitation, - PortfolioInvitation, -) -from django.db.models import ( - CharField, - F, - ManyToManyField, - Q, - QuerySet, - Value, - TextField, - OuterRef, - Subquery, - Func, - Case, - When, - Exists, -) -from django.db.models.functions import Concat, Coalesce, Cast -from registrar.models.user_group import UserGroup -from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.utility.generic_helper import convert_queryset_to_dict -from registrar.models.utility.orm_helper import ArrayRemoveNull -from django.contrib.postgres.aggregates import ArrayAgg -from django.contrib.admin.models import LogEntry, ADDITION -from django.contrib.contenttypes.models import ContentType - - -class BaseModelAnnotation(ABC): - """ - Abstract base class that standardizes how models are annotated for csv exports or complex annotation queries. - For example, the Members table / csv export. - - Subclasses define model-specific annotations, filters, and field formatting while inheriting - common queryset building logic. - Intended ensure consistent data presentation across both table UI components and CSV exports. - - Base class in an inheritance tree of 4. - """ - - @classmethod - @abstractmethod - def model(self): - """ - Property to specify the model that the export class will handle. - Must be implemented by subclasses. - """ - pass - - @classmethod - def get_sort_fields(cls): - """ - Returns the sort fields for the CSV export. Override in subclasses as needed. - """ - return [] - - @classmethod - def get_additional_args(cls): - """ - Returns additional keyword arguments as an empty dictionary. - Override in subclasses to provide specific arguments. - """ - return {} - - @classmethod - def get_select_related(cls): - """ - Get a list of tables to pass to select_related when building queryset. - """ - return [] - - @classmethod - def get_prefetch_related(cls): - """ - Get a list of tables to pass to prefetch_related when building queryset. - """ - return [] - - @classmethod - def get_exclusions(cls): - """ - Get a Q object of exclusion conditions to pass to .exclude() when building queryset. - """ - return Q() - - @classmethod - def get_filter_conditions(cls, **kwargs): - """ - Get a Q object of filter conditions to filter when building queryset. - """ - return Q() - - @classmethod - def get_annotated_fields(cls, **kwargs): - """ - Get a dict of computed fields. These are fields that do not exist on the model normally - and will be passed to .annotate() when building a queryset. - """ - return {} - - @classmethod - def get_annotations_for_sort(cls): - """ - Get a dict of annotations to make available for order_by clause. - """ - return {} - - @classmethod - def get_related_table_fields(cls): - """ - Get a list of fields from related tables. - """ - return [] - - @classmethod - def annotate_and_retrieve_fields( - cls, initial_queryset, annotated_fields, related_table_fields=None, include_many_to_many=False, **kwargs - ) -> QuerySet: - """ - Applies annotations to a queryset and retrieves specified fields, - including class-defined and annotation-defined. - - Parameters: - initial_queryset (QuerySet): Initial queryset. - annotated_fields (dict, optional): Fields to compute {field_name: expression}. - related_table_fields (list, optional): Extra fields to retrieve; defaults to annotation keys if None. - include_many_to_many (bool, optional): Determines if we should include many to many fields or not - **kwargs: Additional keyword arguments for specific parameters (e.g., public_contacts, domain_invitations, - user_domain_roles). - - Returns: - QuerySet: Contains dictionaries with the specified fields for each record. - """ - if related_table_fields is None: - related_table_fields = [] - - # We can infer that if we're passing in annotations, - # we want to grab the result of said annotation. - if annotated_fields: - related_table_fields.extend(annotated_fields.keys()) - - # Get prexisting fields on the model - model_fields = set() - for field in cls.model()._meta.get_fields(): - # Exclude many to many fields unless we specify - many_to_many = isinstance(field, ManyToManyField) and include_many_to_many - if many_to_many or not isinstance(field, ManyToManyField): - model_fields.add(field.name) - - queryset = initial_queryset.annotate(**annotated_fields).values(*model_fields, *related_table_fields) - - return cls.update_queryset(queryset, **kwargs) - - @classmethod - def get_annotated_queryset(cls, **kwargs): - sort_fields = cls.get_sort_fields() - # Get additional args and merge with incoming kwargs - additional_args = cls.get_additional_args() - kwargs.update(additional_args) - select_related = cls.get_select_related() - prefetch_related = cls.get_prefetch_related() - exclusions = cls.get_exclusions() - annotations_for_sort = cls.get_annotations_for_sort() - filter_conditions = cls.get_filter_conditions(**kwargs) - annotated_fields = cls.get_annotated_fields(**kwargs) - related_table_fields = cls.get_related_table_fields() - - model_queryset = ( - cls.model() - .objects.select_related(*select_related) - .prefetch_related(*prefetch_related) - .filter(filter_conditions) - .exclude(exclusions) - .annotate(**annotations_for_sort) - .order_by(*sort_fields) - .distinct() - ) - return cls.annotate_and_retrieve_fields(model_queryset, annotated_fields, related_table_fields, **kwargs) - - @classmethod - def update_queryset(cls, queryset, **kwargs): - """ - Returns an updated queryset. Override in subclass to update queryset. - """ - return queryset - - @classmethod - def get_model_annotation_dict(cls, **kwargs): - return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) - - -class UserPortfolioPermissionModelAnnotation(BaseModelAnnotation): - """ - Annotates UserPortfolioPermission querysets with computed fields for member tables. - Handles formatting of user details, permissions, and related domain information - for both UI display and CSV export. - """ - - @classmethod - def model(cls): - # Return the model class that this export handles - return UserPortfolioPermission - - @classmethod - def get_select_related(cls): - """ - Get a list of tables to pass to select_related when building queryset. - """ - return ["user"] - - @classmethod - def get_filter_conditions(cls, portfolio, **kwargs): - """ - Get a Q object of filter conditions to filter when building queryset. - """ - if not portfolio: - # Return nothing - return Q(id__in=[]) - - # 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, - # Double outer ref because we first go into the LogEntry query, - # then into the parent UserPortfolioPermission. - email=OuterRef(OuterRef("user__email")), - portfolio=OuterRef(OuterRef("portfolio")), - ).values("id")[:1] - ), - output_field=CharField(), - ) - - @classmethod - def get_annotated_fields(cls, portfolio, csv_report=False): - """ - Get a dict of computed fields. These are fields that do not exist on the model normally - and will be passed to .annotate() when building a queryset. - """ - if not portfolio: - # Return nothing - return {} - - # Tweak the queries slightly to only return the data we need. - # When returning data for the csv report we: - # 1. Only return the domain name for 'domain_info' rather than also add ':' seperated id - # 2. Return a formatted date for 'last_active' - # These are just optimizations that are better done in SQL as opposed to python. - if csv_report: - domain_query = F("user__permissions__domain__name") - last_active_query = Func( - F("user__last_login"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField() - ) - else: - # an array of domains, with id and name, colon separated - domain_query = Concat( - F("user__permissions__domain_id"), - Value(":"), - F("user__permissions__domain__name"), - # specify the output_field to ensure union has same column types - output_field=CharField(), - ) - last_active_query = Cast(F("user__last_login"), output_field=TextField()) - - return { - "first_name": F("user__first_name"), - "last_name": F("user__last_name"), - "email_display": F("user__email"), - "last_active": Coalesce( - last_active_query, - Value("Invalid date"), - output_field=CharField(), - ), - "additional_permissions_display": F("additional_permissions"), - "member_display": Case( - # If email is present and not blank, use email - When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), - # If first name or last name is present, use concatenation of first_name + " " + last_name - When( - Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), - then=Concat( - Coalesce(F("user__first_name"), Value("")), - Value(" "), - Coalesce(F("user__last_name"), Value("")), - ), - ), - # If neither, use an empty string - default=Value(""), - output_field=CharField(), - ), - "domain_info": ArrayAgg( - domain_query, - distinct=True, - # only include domains in portfolio - filter=Q(user__permissions__domain__isnull=False) - & Q(user__permissions__domain__domain_info__portfolio=portfolio), - ), - "type": Value("member", output_field=CharField()), - "joined_date": Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=CharField()), - "invited_by": PortfolioInvitationModelAnnotation.get_invited_by_query( - object_id_query=cls.get_portfolio_invitation_id_query() - ), - } - - @classmethod - def get_annotated_queryset(cls, portfolio, csv_report=False): - """Override of the base annotated queryset to pass in portfolio""" - return super().get_annotated_queryset(portfolio=portfolio, csv_report=csv_report) - - -class PortfolioInvitationModelAnnotation(BaseModelAnnotation): - """ - Annotates PortfolioInvitation querysets with computed fields for the member table. - Handles formatting of user details, permissions, and related domain information - for both UI display and CSV export. - """ - - @classmethod - def model(cls): - # Return the model class that this export handles - return PortfolioInvitation - - @classmethod - def get_exclusions(cls): - """ - Get a Q object of exclusion conditions to pass to .exclude() when building queryset. - """ - return Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) - - @classmethod - def get_filter_conditions(cls, portfolio, **kwargs): - """ - Get a Q object of filter conditions to filter when building queryset. - """ - if not portfolio: - # Return nothing - return Q(id__in=[]) - - # Get all members on this portfolio - return Q(portfolio=portfolio) - - @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( - content_type=ContentType.objects.get_for_model(PortfolioInvitation), - object_id=object_id_query, - action_flag=ADDITION, - ) - .annotate( - display_email=Case( - When( - Exists( - UserGroup.objects.filter( - name__in=["cisa_analysts_group", "full_access_group"], - user=OuterRef("user"), - ) - ), - then=Value("help@get.gov"), - ), - default=F("user__email"), - output_field=CharField(), - ) - ) - .order_by("action_time") - .values("display_email")[:1] - ), - Value("System"), - output_field=CharField(), - ) - - @classmethod - def get_annotated_fields(cls, portfolio, csv_report=False): - """ - Get a dict of computed fields. These are fields that do not exist on the model normally - and will be passed to .annotate() when building a queryset. - """ - if not portfolio: - # Return nothing - return {} - - # Tweak the queries slightly to only return the data we need - if csv_report: - domain_query = F("domain__name") - else: - domain_query = Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField()) - - # Get all existing domain invitations and search on that for domains the user exists on - domain_invitations = DomainInvitation.objects.filter( - email=OuterRef("email"), # Check if email matches the OuterRef("email") - domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio - ).annotate(domain_info=domain_query) - return { - "first_name": Value(None, output_field=CharField()), - "last_name": Value(None, output_field=CharField()), - "email_display": F("email"), - "last_active": Value("Invited", output_field=CharField()), - "additional_permissions_display": F("additional_permissions"), - "member_display": F("email"), - # Use ArrayRemove to return an empty list when no domain invitations are found - "domain_info": ArrayRemoveNull( - ArrayAgg( - Subquery(domain_invitations.values("domain_info")), - distinct=True, - ) - ), - "type": Value("invitedmember", output_field=CharField()), - "joined_date": Value("Unretrieved", output_field=CharField()), - "invited_by": cls.get_invited_by_query(object_id_query=Cast(OuterRef("id"), output_field=CharField())), - } - - @classmethod - def get_annotated_queryset(cls, portfolio, csv_report=False): - """Override of the base annotated queryset to pass in portfolio""" - return super().get_annotated_queryset(portfolio=portfolio, csv_report=csv_report) diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index bf89dcd82..f02aedbfa 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -3,15 +3,16 @@ from django.db.models import F, Q from django.urls import reverse from django.views import View - +from django.db.models.expressions import Func +from django.db.models import Value, F, CharField, TextField, Q, Case, When, OuterRef, Subquery +from django.db.models.functions import Cast, Coalesce, Concat +from django.contrib.postgres.aggregates import ArrayAgg from registrar.models import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from registrar.utility.model_annotations import ( - PortfolioInvitationModelAnnotation, - UserPortfolioPermissionModelAnnotation, -) from registrar.views.utility.mixins import PortfolioMembersPermission - +from registrar.models.domain_invitation import DomainInvitation +from registrar.models.portfolio_invitation import PortfolioInvitation +from registrar.models.user_portfolio_permission import UserPortfolioPermission class PortfolioMembersJson(PortfolioMembersPermission, View): @@ -55,25 +56,92 @@ def get(self, request): def initial_permissions_search(self, portfolio): """Perform initial search for permissions before applying any filters.""" - queryset = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio) - return queryset.values( - "id", - "first_name", - "last_name", - "email_display", - "last_active", - "roles", - "additional_permissions_display", - "member_display", - "domain_info", - "type", + permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio) + permissions = ( + permissions.select_related("user") + .annotate( + first_name=F("user__first_name"), + last_name=F("user__last_name"), + email_display=F("user__email"), + last_active=Coalesce( + Cast(F("user__last_login"), output_field=TextField()), # Cast last_login to text + Value("Invalid date"), + output_field=TextField(), + ), + additional_permissions_display=F("additional_permissions"), + member_display=Case( + # If email is present and not blank, use email + When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), + # If first name or last name is present, use concatenation of first_name + " " + last_name + When( + Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), + then=Concat( + Coalesce(F("user__first_name"), Value("")), + Value(" "), + Coalesce(F("user__last_name"), Value("")), + ), + ), + # If neither, use an empty string + default=Value(""), + output_field=CharField(), + ), + domain_info=ArrayAgg( + # an array of domains, with id and name, colon separated + Concat( + F("user__permissions__domain_id"), + Value(":"), + F("user__permissions__domain__name"), + # specify the output_field to ensure union has same column types + output_field=CharField(), + ), + distinct=True, + filter=Q(user__permissions__domain__isnull=False) # filter out null values + & Q( + user__permissions__domain__domain_info__portfolio=portfolio + ), # only include domains in portfolio + ), + type=Value("member", output_field=CharField()), + ) + .values( + "id", + "first_name", + "last_name", + "email_display", + "last_active", + "roles", + "additional_permissions_display", + "member_display", + "domain_info", + "type", + ) ) + return permissions def initial_invitations_search(self, portfolio): """Perform initial invitations search and get related DomainInvitation data based on the email.""" # Get DomainInvitation query for matching email and for the portfolio - queryset = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio) - return queryset.values( + domain_invitations = DomainInvitation.objects.filter( + email=OuterRef("email"), # Check if email matches the OuterRef("email") + domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio + ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) + # PortfolioInvitation query + invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) + invitations = invitations.annotate( + first_name=Value(None, output_field=CharField()), + last_name=Value(None, output_field=CharField()), + email_display=F("email"), + last_active=Value("Invited", output_field=TextField()), + additional_permissions_display=F("additional_permissions"), + member_display=F("email"), + # Use ArrayRemove to return an empty list when no domain invitations are found + domain_info=ArrayRemove( + ArrayAgg( + Subquery(domain_invitations.values("domain_info")), + distinct=True, + ) + ), + type=Value("invitedmember", output_field=CharField()), + ).values( "id", "first_name", "last_name", @@ -85,6 +153,7 @@ def initial_invitations_search(self, portfolio): "domain_info", "type", ) + return invitations def apply_search_term(self, queryset, request): """Apply search term to the queryset.""" @@ -144,3 +213,9 @@ def serialize_members(self, portfolio, item, user): "svg_icon": ("visibility" if view_only else "settings"), } return member_json + + +# Custom Func to use array_remove to remove null values +class ArrayRemove(Func): + function = "array_remove" + template = "%(function)s(%(expressions)s, NULL)" From e813791033eed161c5f32e78320d1484d9f0a106 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:26:56 -0700 Subject: [PATCH 35/45] Update csv_export.py --- src/registrar/utility/csv_export.py | 64 +++++++++++------------------ 1 file changed, 24 insertions(+), 40 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 2f255fd49..286b970ce 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1,64 +1,48 @@ -from abc import abstractmethod -from collections import defaultdict import csv import logging +from abc import ABC, abstractmethod +from collections import defaultdict from datetime import datetime -from registrar.models import ( - Domain, - DomainInvitation, - DomainRequest, - DomainInformation, - PublicContact, - UserDomainRole, -) + +from django.contrib.admin.models import LogEntry, ADDITION +from django.contrib.contenttypes.models import ContentType +from django.contrib.postgres.aggregates import ArrayAgg, StringAgg from django.db.models import ( Case, CharField, Count, DateField, F, + ManyToManyField, Q, + QuerySet, + TextField, Value, When, + OuterRef, + Subquery, + Exists, ) -from abc import ABC, abstractmethod +from django.db.models.functions import Concat, Coalesce, Cast, Func from django.utils import timezone -from django.db.models.functions import Concat, Coalesce -from django.contrib.postgres.aggregates import StringAgg -from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.utility.generic_helper import convert_queryset_to_dict -from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices -from registrar.templatetags.custom_filters import get_region -from registrar.utility.constants import BranchChoices -from registrar.utility.enums import DefaultEmail -from abc import ABC, abstractmethod + from registrar.models import ( + Domain, DomainInvitation, + DomainRequest, + DomainInformation, + PublicContact, + UserDomainRole, PortfolioInvitation, + UserGroup, ) -from django.db.models import ( - CharField, - F, - ManyToManyField, - Q, - QuerySet, - Value, - TextField, - OuterRef, - Subquery, - Func, - Case, - When, - Exists, -) -from django.db.models.functions import Concat, Coalesce, Cast -from registrar.models.user_group import UserGroup from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.models.utility.orm_helper import ArrayRemoveNull -from django.contrib.postgres.aggregates import ArrayAgg -from django.contrib.admin.models import LogEntry, ADDITION -from django.contrib.contenttypes.models import ContentType +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices +from registrar.templatetags.custom_filters import get_region +from registrar.utility.constants import BranchChoices +from registrar.utility.enums import DefaultEmail logger = logging.getLogger(__name__) From e2bd982c27d339f041101f394b3b0326bb6d6f12 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:27:34 -0700 Subject: [PATCH 36/45] Update portfolio_members_json.py --- src/registrar/views/portfolio_members_json.py | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index f02aedbfa..8e5ee902f 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -1,18 +1,32 @@ -from django.http import JsonResponse +from django.contrib.postgres.aggregates import ArrayAgg from django.core.paginator import Paginator -from django.db.models import F, Q -from django.urls import reverse -from django.views import View +from django.db.models import ( + Case, + CharField, + F, + Q, + TextField, + Value, + When, + OuterRef, + Subquery, +) from django.db.models.expressions import Func -from django.db.models import Value, F, CharField, TextField, Q, Case, When, OuterRef, Subquery from django.db.models.functions import Cast, Coalesce, Concat -from django.contrib.postgres.aggregates import ArrayAgg -from registrar.models import UserPortfolioPermission -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from django.http import JsonResponse +from django.urls import reverse +from django.views import View + +from registrar.models import ( + DomainInvitation, + PortfolioInvitation, + UserPortfolioPermission, +) +from registrar.models.utility.portfolio_helper import ( + UserPortfolioPermissionChoices, + UserPortfolioRoleChoices, +) from registrar.views.utility.mixins import PortfolioMembersPermission -from registrar.models.domain_invitation import DomainInvitation -from registrar.models.portfolio_invitation import PortfolioInvitation -from registrar.models.user_portfolio_permission import UserPortfolioPermission class PortfolioMembersJson(PortfolioMembersPermission, View): From 4013815a04c7ece11c68f0c571007f33c18285e2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:33:47 -0700 Subject: [PATCH 37/45] Update csv_export.py --- src/registrar/utility/csv_export.py | 44 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 286b970ce..916242682 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -88,6 +88,13 @@ def model(self): """ pass + @classmethod + def get_columns(cls): + """ + Returns the columns for CSV export. Override in subclasses as needed. + """ + return [] + @classmethod def get_sort_fields(cls): """ @@ -152,6 +159,21 @@ def get_related_table_fields(cls): Get a list of fields from related tables. """ return [] + + @classmethod + def update_queryset(cls, queryset, **kwargs): + """ + Returns an updated queryset. Override in subclass to update queryset. + """ + return queryset + + @classmethod + def write_csv_before(cls, csv_writer, **kwargs): + """ + Write to csv file before the write_csv method. + Override in subclasses where needed. + """ + pass @classmethod def annotate_and_retrieve_fields( @@ -218,32 +240,10 @@ def get_annotated_queryset(cls, **kwargs): ) return cls.annotate_and_retrieve_fields(model_queryset, annotated_fields, related_table_fields, **kwargs) - @classmethod - def update_queryset(cls, queryset, **kwargs): - """ - Returns an updated queryset. Override in subclass to update queryset. - """ - return queryset - @classmethod def get_model_annotation_dict(cls, **kwargs): return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) - @classmethod - def get_columns(cls): - """ - Returns the columns for CSV export. Override in subclasses as needed. - """ - return [] - - @classmethod - def write_csv_before(cls, csv_writer, **kwargs): - """ - Write to csv file before the write_csv method. - Override in subclasses where needed. - """ - pass - @classmethod def export_data_to_csv(cls, csv_file, **kwargs): """ From 958e010c5a744e0fceeb6a0394b9c24b4e8a4c68 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:36:43 -0700 Subject: [PATCH 38/45] Update csv_export.py --- src/registrar/utility/csv_export.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 916242682..ea56f9b27 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -214,6 +214,25 @@ def annotate_and_retrieve_fields( return cls.update_queryset(queryset, **kwargs) + @classmethod + def export_data_to_csv(cls, csv_file, **kwargs): + """ + All domain metadata: + Exports domains of all statuses plus domain managers. + """ + writer = csv.writer(csv_file) + columns = cls.get_columns() + models_dict = cls.get_model_annotation_dict(**kwargs) + + # Write to csv file before the write_csv + cls.write_csv_before(writer, **kwargs) + + # Write the csv file + rows = cls.write_csv(writer, columns, models_dict) + + # Return rows that for easier parsing and testing + return rows + @classmethod def get_annotated_queryset(cls, **kwargs): sort_fields = cls.get_sort_fields() From 2b5a5fe7ca06ee128b5022ac64435e44f0a40014 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:00:20 -0700 Subject: [PATCH 39/45] undo some past changes --- src/registrar/utility/csv_export.py | 51 +++++++++---------- src/registrar/views/portfolio_members_json.py | 42 +++++---------- 2 files changed, 37 insertions(+), 56 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index ea56f9b27..598395b54 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1,12 +1,19 @@ -import csv -import logging from abc import ABC, abstractmethod from collections import defaultdict +import csv +import logging from datetime import datetime - -from django.contrib.admin.models import LogEntry, ADDITION -from django.contrib.contenttypes.models import ContentType -from django.contrib.postgres.aggregates import ArrayAgg, StringAgg +from registrar.models import ( + Domain, + DomainInvitation, + DomainRequest, + DomainInformation, + PublicContact, + UserDomainRole, + PortfolioInvitation, + UserGroup, + UserPortfolioPermission, +) from django.db.models import ( Case, CharField, @@ -22,21 +29,13 @@ OuterRef, Subquery, Exists, + Func, ) -from django.db.models.functions import Concat, Coalesce, Cast, Func from django.utils import timezone - -from registrar.models import ( - Domain, - DomainInvitation, - DomainRequest, - DomainInformation, - PublicContact, - UserDomainRole, - PortfolioInvitation, - UserGroup, -) -from registrar.models.user_portfolio_permission import UserPortfolioPermission +from django.db.models.functions import Concat, Coalesce, Cast +from django.contrib.postgres.aggregates import ArrayAgg, StringAgg +from django.contrib.admin.models import LogEntry, ADDITION +from django.contrib.contenttypes.models import ContentType from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.models.utility.orm_helper import ArrayRemoveNull from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices @@ -76,7 +75,7 @@ def format_end_date(end_date): class BaseExport(ABC): """ A generic class for exporting data which returns a csv file for the given model. - 2nd class in an inheritance tree of 4. + Base class in an inheritance tree of 3. """ @classmethod @@ -139,7 +138,7 @@ def get_filter_conditions(cls, **kwargs): return Q() @classmethod - def get_annotated_fields(cls, **kwargs): + def get_computed_fields(cls, **kwargs): """ Get a dict of computed fields. These are fields that do not exist on the model normally and will be passed to .annotate() when building a queryset. @@ -244,7 +243,7 @@ def get_annotated_queryset(cls, **kwargs): exclusions = cls.get_exclusions() annotations_for_sort = cls.get_annotations_for_sort() filter_conditions = cls.get_filter_conditions(**kwargs) - annotated_fields = cls.get_annotated_fields(**kwargs) + annotated_fields = cls.get_computed_fields(**kwargs) related_table_fields = cls.get_related_table_fields() model_queryset = ( @@ -783,7 +782,7 @@ def get_prefetch_related(cls): return ["domain__permissions"] @classmethod - def get_annotated_fields(cls, delimiter=", ", **kwargs): + def get_computed_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -1000,7 +999,7 @@ def get_filter_conditions(cls, **kwargs): ) @classmethod - def get_annotated_fields(cls, delimiter=", ", **kwargs): + def get_computed_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -1095,7 +1094,7 @@ def get_filter_conditions(cls, **kwargs): ) @classmethod - def get_annotated_fields(cls, delimiter=", ", **kwargs): + def get_computed_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -1729,7 +1728,7 @@ def get_sort_fields(cls): ] @classmethod - def get_annotated_fields(cls, delimiter=", ", **kwargs): + def get_computed_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 8e5ee902f..232ca2e6c 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -1,32 +1,19 @@ -from django.contrib.postgres.aggregates import ArrayAgg +from django.http import JsonResponse from django.core.paginator import Paginator -from django.db.models import ( - Case, - CharField, - F, - Q, - TextField, - Value, - When, - OuterRef, - Subquery, -) +from django.db.models import Value, F, CharField, TextField, Q, Case, When, OuterRef, Subquery from django.db.models.expressions import Func from django.db.models.functions import Cast, Coalesce, Concat -from django.http import JsonResponse +from django.contrib.postgres.aggregates import ArrayAgg from django.urls import reverse from django.views import View -from registrar.models import ( - DomainInvitation, - PortfolioInvitation, - UserPortfolioPermission, -) -from registrar.models.utility.portfolio_helper import ( - UserPortfolioPermissionChoices, - UserPortfolioRoleChoices, -) +from registrar.models.domain_invitation import DomainInvitation +from registrar.models.portfolio_invitation import PortfolioInvitation +from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.views.utility.mixins import PortfolioMembersPermission +from registrar.models.utility.orm_helper import ArrayRemoveNull + class PortfolioMembersJson(PortfolioMembersPermission, View): @@ -53,7 +40,7 @@ def get(self, request): page_number = request.GET.get("page", 1) page_obj = paginator.get_page(page_number) - members = [self.serialize_members(portfolio, item, request.user) for item in page_obj.object_list] + members = [self.serialize_members(request, portfolio, item, request.user) for item in page_obj.object_list] return JsonResponse( { @@ -148,7 +135,7 @@ def initial_invitations_search(self, portfolio): additional_permissions_display=F("additional_permissions"), member_display=F("email"), # Use ArrayRemove to return an empty list when no domain invitations are found - domain_info=ArrayRemove( + domain_info=ArrayRemoveNull( ArrayAgg( Subquery(domain_invitations.values("domain_info")), distinct=True, @@ -193,7 +180,7 @@ def apply_sorting(self, queryset, request): queryset = queryset.order_by(sort_by) return queryset - def serialize_members(self, portfolio, item, user): + def serialize_members(self, request, portfolio, item, user): # Check if the user can edit other users user_can_edit_other_users = any( user.has_perm(perm) for perm in ["registrar.full_access_permission", "registrar.change_user"] @@ -228,8 +215,3 @@ def serialize_members(self, portfolio, item, user): } return member_json - -# Custom Func to use array_remove to remove null values -class ArrayRemove(Func): - function = "array_remove" - template = "%(function)s(%(expressions)s, NULL)" From d354f04bf156bc26141be4e6ea4ea7d5b80577be Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Nov 2024 08:48:02 -0700 Subject: [PATCH 40/45] Update src/registrar/utility/csv_export.py Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- src/registrar/utility/csv_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index f0fb80ed5..f2caeff1c 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -192,7 +192,7 @@ def get_model_annotation_dict(cls, request=None, **kwargs): @classmethod def get_columns(cls): """ - Returns the columns for CSV export. Override in subclasses as needed. + Returns the list of column string names for CSV export. Override in subclasses as needed. """ return [ "Email", From 2f93ad935646d090ca50986d2f0d1175ef88ba97 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Nov 2024 10:15:25 -0700 Subject: [PATCH 41/45] enums and the like --- .../models/user_portfolio_permission.py | 21 ++++++++------- .../models/utility/portfolio_helper.py | 27 +++++++++++++++++++ src/registrar/utility/csv_export.py | 6 ++++- src/registrar/utility/enums.py | 12 +++++++++ src/registrar/utility/model_annotations.py | 6 +++-- src/registrar/views/portfolio_members_json.py | 11 ++++---- 6 files changed, 65 insertions(+), 18 deletions(-) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index cd48b1b71..51f3fa3fe 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -2,7 +2,7 @@ from django.forms import ValidationError from registrar.models.user_domain_role import UserDomainRole from registrar.utility.waffle import flag_is_active_for_user -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices, DomainRequestPermissionDisplay, MemberPermissionDisplay from .utility.time_stamped_model import TimeStampedModel from django.contrib.postgres.fields import ArrayField @@ -115,26 +115,27 @@ def get_domain_request_permission_display(cls, roles, additional_permissions): UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS, ] + if all(perm in all_permissions for perm in all_domain_perms): - return "Viewer Requester" + return DomainRequestPermissionDisplay.VIEWER_REQUESTER elif UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS in all_permissions: - return "Viewer" + return DomainRequestPermissionDisplay.VIEWER else: - return "None" + return DomainRequestPermissionDisplay.NONE @classmethod def get_member_permission_display(cls, roles, additional_permissions): """Class method to return a readable string for member permissions""" - # Tracks if they can view, create requests, or not do anything + # Tracks if they can view, create requests, or not do anything. + # This is different than get_domain_request_permission_display because member tracks + # permissions slightly differently. all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, additional_permissions) - # Note for reviewers: the reason why this isn't checking on "all" is because - # the way perms work for members is different than requests. We need to consolidate this. if UserPortfolioPermissionChoices.EDIT_MEMBERS in all_permissions: - return "Manager" + return MemberPermissionDisplay.MANAGER elif UserPortfolioPermissionChoices.VIEW_MEMBERS in all_permissions: - return "Viewer" + return MemberPermissionDisplay.VIEWER else: - return "None" + return MemberPermissionDisplay.NONE def clean(self): """Extends clean method to perform additional validation, which can raise errors in django admin.""" diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index d998d7ffa..cdbda798b 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -1,3 +1,4 @@ +from enum import Enum from django.db import models @@ -40,3 +41,29 @@ def get_user_portfolio_permission_label(cls, user_portfolio_permission): @classmethod def to_dict(cls): return {key: value.value for key, value in cls.__members__.items()} + + +class DomainRequestPermissionDisplay(Enum): + """Stores display values for domain request permission combinations. + + Overview of values: + - VIEWER_REQUESTER: "Viewer Requester" + - VIEWER: "VIEWER" + - NONE: "None" + """ + VIEWER_REQUESTER = "Viewer Requester" + VIEWER = "VIEWER" + NONE = "None" + + +class MemberPermissionDisplay(Enum): + """Stores display values for member permission combinations. + + Overview of values: + - MANAGER: "Manager" + - VIEWER: "Viewer" + - NONE: "None" + """ + MANAGER = "Manager" + VIEWER = "Viewer" + NONE = "None" diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index f2caeff1c..268cb7ec8 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -160,7 +160,11 @@ def model(self): @classmethod def get_model_annotation_dict(cls, request=None, **kwargs): """Combines the permissions and invitation model annotations for - the final returned csv export which combines both of these contexts""" + the final returned csv export which combines both of these contexts. + Returns a dictionary of a union between: + - UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio, csv_report=True) + - PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio, csv_report=True) + """ portfolio = request.session.get("portfolio") if not portfolio: return {} diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index e430a4881..137680a23 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -35,10 +35,22 @@ class DefaultEmail(Enum): Overview of emails: - PUBLIC_CONTACT_DEFAULT: "dotgov@cisa.dhs.gov" - LEGACY_DEFAULT: "registrar@dotgov.gov" + - HELP_EMAIL: "help@get.gov" """ PUBLIC_CONTACT_DEFAULT = "dotgov@cisa.dhs.gov" LEGACY_DEFAULT = "registrar@dotgov.gov" + HELP_EMAIL = "help@get.gov" + + +class DefaultUser(Enum): + """Stores default values for a default user. + + Overview of defaults: + - SYSTEM: "System" + """ + + SYSTEM = "System" class Step(StrEnum): diff --git a/src/registrar/utility/model_annotations.py b/src/registrar/utility/model_annotations.py index 0fc430f4f..a90354bda 100644 --- a/src/registrar/utility/model_annotations.py +++ b/src/registrar/utility/model_annotations.py @@ -46,6 +46,8 @@ from django.contrib.admin.models import LogEntry, ADDITION from django.contrib.contenttypes.models import ContentType +from registrar.utility.enums import DefaultEmail, DefaultUser + class BaseModelAnnotation(ABC): """ @@ -388,7 +390,7 @@ def get_invited_by_query(cls, object_id_query): user=OuterRef("user"), ) ), - then=Value("help@get.gov"), + then=Value(DefaultEmail.HELP_EMAIL), ), default=F("user__email"), output_field=CharField(), @@ -397,7 +399,7 @@ def get_invited_by_query(cls, object_id_query): .order_by("action_time") .values("display_email")[:1] ), - Value("System"), + Value(DefaultUser.SYSTEM), output_field=CharField(), ) diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index bf89dcd82..29af2b4ad 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -55,8 +55,9 @@ def get(self, request): def initial_permissions_search(self, portfolio): """Perform initial search for permissions before applying any filters.""" - queryset = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio) - return queryset.values( + # Get UserPortfolioPermission query for getting active members on the portfolio + permissions = UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio) + return permissions.values( "id", "first_name", "last_name", @@ -71,9 +72,9 @@ def initial_permissions_search(self, portfolio): def initial_invitations_search(self, portfolio): """Perform initial invitations search and get related DomainInvitation data based on the email.""" - # Get DomainInvitation query for matching email and for the portfolio - queryset = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio) - return queryset.values( + # Get PortfolioInvitation query for getting active invitations on the portfolio + invitations = PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio) + return invitations.values( "id", "first_name", "last_name", From e91526e602bceca2bef38542d24eec0fe1e5eb81 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Nov 2024 10:20:25 -0700 Subject: [PATCH 42/45] cleanup --- src/registrar/utility/csv_export.py | 8 ++++---- src/registrar/utility/enums.py | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 4cbd026d8..532edd759 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -41,7 +41,7 @@ from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.templatetags.custom_filters import get_region from registrar.utility.constants import BranchChoices -from registrar.utility.enums import DefaultEmail +from registrar.utility.enums import DefaultEmail, DefaultUserValues logger = logging.getLogger(__name__) @@ -423,7 +423,7 @@ def get_model_annotation_dict(cls, request=None, **kwargs): ) ), type=Value("invitedmember", output_field=CharField()), - joined_date=Value("Unretrieved", output_field=CharField()), + joined_date=Value(DefaultUserValues.UNRETRIEVED, output_field=CharField()), invited_by=cls.get_invited_by_query(object_id_query=Cast(OuterRef("id"), output_field=CharField())), ).values(*shared_columns) @@ -450,7 +450,7 @@ def get_invited_by_query(cls, object_id_query): user=OuterRef("user"), ) ), - then=Value("help@get.gov"), + then=Value(DefaultEmail.HELP_EMAIL), ), default=F("user__email"), output_field=CharField(), @@ -459,7 +459,7 @@ def get_invited_by_query(cls, object_id_query): .order_by("action_time") .values("display_email")[:1] ), - Value("System"), + Value(DefaultUserValues.SYSTEM), output_field=CharField(), ) diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index 137680a23..fb3e20577 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -43,14 +43,17 @@ class DefaultEmail(Enum): HELP_EMAIL = "help@get.gov" -class DefaultUser(Enum): +class DefaultUserValues(Enum): """Stores default values for a default user. Overview of defaults: - - SYSTEM: "System" + - SYSTEM: "System" <= Default username + - UNRETRIEVED: "Unretrieved" <= Default email state """ SYSTEM = "System" + UNRETRIEVED = "Unretrieved" + class Step(StrEnum): From 62f0205b4ccaffed6faebe484181aa06d618b76b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Nov 2024 10:41:53 -0700 Subject: [PATCH 43/45] Fix merge conflict + enum --- .../models/utility/portfolio_helper.py | 10 +- src/registrar/tests/test_reports.py | 2 +- src/registrar/utility/csv_export.py | 130 +++++++++--------- src/registrar/utility/enums.py | 5 +- 4 files changed, 76 insertions(+), 71 deletions(-) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index cdbda798b..9b661b316 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -1,4 +1,4 @@ -from enum import Enum +from registrar.utility import StrEnum from django.db import models @@ -43,20 +43,20 @@ def to_dict(cls): return {key: value.value for key, value in cls.__members__.items()} -class DomainRequestPermissionDisplay(Enum): +class DomainRequestPermissionDisplay(StrEnum): """Stores display values for domain request permission combinations. Overview of values: - VIEWER_REQUESTER: "Viewer Requester" - - VIEWER: "VIEWER" + - VIEWER: "Viewer" - NONE: "None" """ VIEWER_REQUESTER = "Viewer Requester" - VIEWER = "VIEWER" + VIEWER = "Viewer" NONE = "None" -class MemberPermissionDisplay(Enum): +class MemberPermissionDisplay(StrEnum): """Stores display values for member permission combinations. Overview of values: diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 1289c2467..8265e3563 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -862,7 +862,7 @@ def test_member_export(self): # Create a request and add the user to the request request = self.factory.get("/") request.user = self.user - + self.maxDiff = None # Add portfolio to session request = GenericTestHelper._mock_user_request_for_factory(request) request.session["portfolio"] = self.portfolio_1 diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 532edd759..f3315e4a0 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -158,14 +158,14 @@ def get_related_table_fields(cls): Get a list of fields from related tables. """ return [] - + @classmethod def update_queryset(cls, queryset, **kwargs): """ Returns an updated queryset. Override in subclass to update queryset. """ return queryset - + @classmethod def write_csv_before(cls, csv_writer, **kwargs): """ @@ -335,7 +335,7 @@ def model(self): def get_model_annotation_dict(cls, request=None, **kwargs): """Combines the permissions and invitation model annotations for the final returned csv export which combines both of these contexts. - Returns a dictionary of a union between: + Returns a dictionary of a union between: - UserPortfolioPermissionModelAnnotation.get_annotated_queryset(portfolio, csv_report=True) - PortfolioInvitationModelAnnotation.get_annotated_queryset(portfolio, csv_report=True) """ @@ -361,71 +361,77 @@ def get_model_annotation_dict(cls, request=None, **kwargs): ] # Permissions - permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio).select_related("user").annotate( - first_name=F("user__first_name"), - last_name=F("user__last_name"), - email_display=F("user__email"), - last_active=Coalesce( - Func( - F("user__last_login"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField() + permissions = ( + UserPortfolioPermission.objects.filter(portfolio=portfolio) + .select_related("user") + .annotate( + first_name=F("user__first_name"), + last_name=F("user__last_name"), + email_display=F("user__email"), + last_active=Coalesce( + Func(F("user__last_login"), Value("YYYY-MM-DD"), function="to_char", output_field=TextField()), + Value("Invalid date"), + output_field=CharField(), ), - Value("Invalid date"), - output_field=CharField(), - ), - additional_permissions_display=F("additional_permissions"), - member_display=Case( - # If email is present and not blank, use email - When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), - # If first name or last name is present, use concatenation of first_name + " " + last_name - When( - Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), - then=Concat( - Coalesce(F("user__first_name"), Value("")), - Value(" "), - Coalesce(F("user__last_name"), Value("")), + additional_permissions_display=F("additional_permissions"), + member_display=Case( + # If email is present and not blank, use email + When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), + # If first name or last name is present, use concatenation of first_name + " " + last_name + When( + Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), + then=Concat( + Coalesce(F("user__first_name"), Value("")), + Value(" "), + Coalesce(F("user__last_name"), Value("")), + ), ), + # If neither, use an empty string + default=Value(""), + output_field=CharField(), ), - # If neither, use an empty string - default=Value(""), - output_field=CharField(), - ), - domain_info=ArrayAgg( - F("user__permissions__domain__name"), - distinct=True, - # only include domains in portfolio - filter=Q(user__permissions__domain__isnull=False) - & Q(user__permissions__domain__domain_info__portfolio=portfolio), - ), - type=Value("member", output_field=CharField()), - joined_date=Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=CharField()), - invited_by=cls.get_invited_by_query( - object_id_query=cls.get_portfolio_invitation_id_query() - ), - ).values(*shared_columns) + domain_info=ArrayAgg( + F("user__permissions__domain__name"), + distinct=True, + # only include domains in portfolio + filter=Q(user__permissions__domain__isnull=False) + & Q(user__permissions__domain__domain_info__portfolio=portfolio), + ), + type=Value("member", output_field=CharField()), + joined_date=Func(F("created_at"), Value("YYYY-MM-DD"), function="to_char", output_field=CharField()), + invited_by=cls.get_invited_by_query(object_id_query=cls.get_portfolio_invitation_id_query()), + ) + .values(*shared_columns) + ) # Invitations domain_invitations = DomainInvitation.objects.filter( email=OuterRef("email"), # Check if email matches the OuterRef("email") domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio - ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) - invitations = PortfolioInvitation.objects.filter(portfolio=portfolio).annotate( - first_name=Value(None, output_field=CharField()), - last_name=Value(None, output_field=CharField()), - email_display=F("email"), - last_active=Value("Invited", output_field=CharField()), - additional_permissions_display=F("additional_permissions"), - member_display=F("email"), - # Use ArrayRemove to return an empty list when no domain invitations are found - domain_info=ArrayRemoveNull( - ArrayAgg( - Subquery(domain_invitations.values("domain_info")), - distinct=True, - ) - ), - type=Value("invitedmember", output_field=CharField()), - joined_date=Value(DefaultUserValues.UNRETRIEVED, output_field=CharField()), - invited_by=cls.get_invited_by_query(object_id_query=Cast(OuterRef("id"), output_field=CharField())), - ).values(*shared_columns) + ).annotate(domain_info=F("domain__name")) + invitations = ( + PortfolioInvitation.objects.exclude(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) + .filter(portfolio=portfolio) + .annotate( + first_name=Value(None, output_field=CharField()), + last_name=Value(None, output_field=CharField()), + email_display=F("email"), + last_active=Value("Invited", output_field=CharField()), + additional_permissions_display=F("additional_permissions"), + member_display=F("email"), + # Use ArrayRemove to return an empty list when no domain invitations are found + domain_info=ArrayRemoveNull( + ArrayAgg( + Subquery(domain_invitations.values("domain_info")), + distinct=True, + ) + ), + type=Value("invitedmember", output_field=CharField()), + joined_date=Value("Unretrieved", output_field=CharField()), + invited_by=cls.get_invited_by_query(object_id_query=Cast(OuterRef("id"), output_field=CharField())), + ) + .values(*shared_columns) + ) return convert_queryset_to_dict(permissions.union(invitations), is_model=False) @@ -450,7 +456,7 @@ def get_invited_by_query(cls, object_id_query): user=OuterRef("user"), ) ), - then=Value(DefaultEmail.HELP_EMAIL), + then=Value(DefaultEmail.HELP_EMAIL.value), ), default=F("user__email"), output_field=CharField(), @@ -459,7 +465,7 @@ def get_invited_by_query(cls, object_id_query): .order_by("action_time") .values("display_email")[:1] ), - Value(DefaultUserValues.SYSTEM), + Value(DefaultUserValues.SYSTEM.value), output_field=CharField(), ) diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index fb3e20577..14e1e87ee 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -29,7 +29,7 @@ class LogCode(Enum): DEFAULT = 5 -class DefaultEmail(Enum): +class DefaultEmail(StrEnum): """Stores the string values of default emails Overview of emails: @@ -43,7 +43,7 @@ class DefaultEmail(Enum): HELP_EMAIL = "help@get.gov" -class DefaultUserValues(Enum): +class DefaultUserValues(StrEnum): """Stores default values for a default user. Overview of defaults: @@ -55,7 +55,6 @@ class DefaultUserValues(Enum): UNRETRIEVED = "Unretrieved" - class Step(StrEnum): """ Names for each page of the domain request wizard. From c4a27489cc61805745bbce8c956f29aceb3ade44 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Nov 2024 10:45:59 -0700 Subject: [PATCH 44/45] more cleanup --- src/registrar/utility/csv_export.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index f3315e4a0..d7dc68bb9 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -176,7 +176,7 @@ def write_csv_before(cls, csv_writer, **kwargs): @classmethod def annotate_and_retrieve_fields( - cls, initial_queryset, annotated_fields, related_table_fields=None, include_many_to_many=False, **kwargs + cls, initial_queryset, computed_fields, related_table_fields=None, include_many_to_many=False, **kwargs ) -> QuerySet: """ Applies annotations to a queryset and retrieves specified fields, @@ -184,7 +184,7 @@ def annotate_and_retrieve_fields( Parameters: initial_queryset (QuerySet): Initial queryset. - annotated_fields (dict, optional): Fields to compute {field_name: expression}. + computed_fields (dict, optional): Fields to compute {field_name: expression}. related_table_fields (list, optional): Extra fields to retrieve; defaults to annotation keys if None. include_many_to_many (bool, optional): Determines if we should include many to many fields or not **kwargs: Additional keyword arguments for specific parameters (e.g., public_contacts, domain_invitations, @@ -198,8 +198,8 @@ def annotate_and_retrieve_fields( # We can infer that if we're passing in annotations, # we want to grab the result of said annotation. - if annotated_fields: - related_table_fields.extend(annotated_fields.keys()) + if computed_fields : + related_table_fields.extend(computed_fields .keys()) # Get prexisting fields on the model model_fields = set() @@ -209,7 +209,7 @@ def annotate_and_retrieve_fields( if many_to_many or not isinstance(field, ManyToManyField): model_fields.add(field.name) - queryset = initial_queryset.annotate(**annotated_fields).values(*model_fields, *related_table_fields) + queryset = initial_queryset.annotate(**computed_fields).values(*model_fields, *related_table_fields) return cls.update_queryset(queryset, **kwargs) @@ -234,6 +234,7 @@ def export_data_to_csv(cls, csv_file, **kwargs): @classmethod def get_annotated_queryset(cls, **kwargs): + """Returns an annotated queryset based off of all query conditions.""" sort_fields = cls.get_sort_fields() # Get additional args and merge with incoming kwargs additional_args = cls.get_additional_args() @@ -243,7 +244,7 @@ def get_annotated_queryset(cls, **kwargs): exclusions = cls.get_exclusions() annotations_for_sort = cls.get_annotations_for_sort() filter_conditions = cls.get_filter_conditions(**kwargs) - annotated_fields = cls.get_computed_fields(**kwargs) + computed_fields = cls.get_computed_fields(**kwargs) related_table_fields = cls.get_related_table_fields() model_queryset = ( @@ -256,7 +257,7 @@ def get_annotated_queryset(cls, **kwargs): .order_by(*sort_fields) .distinct() ) - return cls.annotate_and_retrieve_fields(model_queryset, annotated_fields, related_table_fields, **kwargs) + return cls.annotate_and_retrieve_fields(model_queryset, computed_fields, related_table_fields, **kwargs) @classmethod def get_model_annotation_dict(cls, **kwargs): From d0f0d56620945680869a3fca01d92bfc10ca3323 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Nov 2024 10:58:12 -0700 Subject: [PATCH 45/45] change enum --- src/registrar/utility/csv_export.py | 2 +- src/registrar/utility/enums.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index d7dc68bb9..6f6b2c744 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -457,7 +457,7 @@ def get_invited_by_query(cls, object_id_query): user=OuterRef("user"), ) ), - then=Value(DefaultEmail.HELP_EMAIL.value), + then=Value(DefaultUserValues.HELP_EMAIL.value), ), default=F("user__email"), output_field=CharField(), diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index 14e1e87ee..232c4056f 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -29,7 +29,7 @@ class LogCode(Enum): DEFAULT = 5 -class DefaultEmail(StrEnum): +class DefaultEmail(Enum): """Stores the string values of default emails Overview of emails: @@ -40,7 +40,6 @@ class DefaultEmail(StrEnum): PUBLIC_CONTACT_DEFAULT = "dotgov@cisa.dhs.gov" LEGACY_DEFAULT = "registrar@dotgov.gov" - HELP_EMAIL = "help@get.gov" class DefaultUserValues(StrEnum): @@ -50,7 +49,7 @@ class DefaultUserValues(StrEnum): - SYSTEM: "System" <= Default username - UNRETRIEVED: "Unretrieved" <= Default email state """ - + HELP_EMAIL = "help@get.gov" SYSTEM = "System" UNRETRIEVED = "Unretrieved"