diff --git a/.github/ISSUE_TEMPLATE/story.yml b/.github/ISSUE_TEMPLATE/story.yml deleted file mode 100644 index e7d81ad3a..000000000 --- a/.github/ISSUE_TEMPLATE/story.yml +++ /dev/null @@ -1,61 +0,0 @@ -name: Story -description: Capture actionable sprint work -labels: ["story"] - -body: - - type: markdown - id: help - attributes: - value: | - > **Note** - > GitHub Issues use [GitHub Flavored Markdown](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax) for formatting. - - type: textarea - id: story - attributes: - label: Story - description: | - Please add the "as a, I want, so that" details that describe the story. - If more than one "as a, I want, so that" describes the story, add multiple. - - Example: - As an analyst - I want the ability to approve a domain request - so that a request can be fulfilled and a new .gov domain can be provisioned - value: | - As a - I want - so that - validations: - required: true - - type: textarea - id: acceptance-criteria - attributes: - label: Acceptance Criteria - description: | - Please add the acceptance criteria that best describe the desired outcomes when this work is completed - - Example: - - Application sends an email when analysts approve domain requests - - Domain request status is "approved" - - Example ("given, when, then" format): - Given that I am an analyst who has finished reviewing a domain request - When I click to approve a domain request - Then the domain provisioning process should be initiated, and the applicant should receive an email update. - validations: - required: true - - type: textarea - id: additional-context - attributes: - label: Additional Context - description: "Please include additional references (screenshots, design links, documentation, etc.) that are relevant" - - type: textarea - id: issue-links - attributes: - label: Issue Links - description: | - What other issues does this story relate to and how? - - Example: - - šŸš§ Blocked by: #123 - - šŸ”„ Relates to: #234 diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml new file mode 100644 index 000000000..ecdf54bbc --- /dev/null +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -0,0 +1,90 @@ +# This workflow can be run from the CLI +# gh workflow run reset-db.yaml -f environment=ENVIRONMENT + +name: Reset database +run-name: Reset database for ${{ github.event.inputs.environment }} + +on: + workflow_dispatch: + inputs: + environment: + type: choice + description: Which environment should we flush and re-load data for? + options: + - el + - ad + - ms + - ag + - litterbox + - hotgov + - cb + - bob + - meoward + - backup + - ky + - es + - nl + - rh + - za + - gd + - rb + - ko + - ab + - rjm + - dk + +jobs: + reset-db: + runs-on: ubuntu-latest + env: + CF_USERNAME: CF_${{ github.event.inputs.environment }}_USERNAME + CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD + DESTINATION_ENVIRONMENT: ${{ github.event.inputs.environment}} + steps: + - name: Delete and Recreate Database + env: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + run: | + # install cf cli and other tools + wget -q -O - https://packages.cloudfoundry.org/debian/cli.cloudfoundry.org.key | sudo gpg --dearmor -o /usr/share/keyrings/cli.cloudfoundry.org.gpg + echo "deb [signed-by=/usr/share/keyrings/cli.cloudfoundry.org.gpg] https://packages.cloudfoundry.org/debian stable main" | sudo tee /etc/apt/sources.list.d/cloudfoundry-cli.list + + sudo apt-get update + sudo apt-get install cf8-cli + cf api api.fr.cloud.gov + cf auth "$CF_USERNAME" "$CF_PASSWORD" + cf target -o cisa-dotgov -s $DESTINATION_ENVIRONMENT + + + + # unbindĀ the service + cf unbind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database + #deleteĀ the service key + yes Y | cf delete-service-key getgov-$DESTINATION_ENVIRONMENT-database SERVICE_CONNECT + # deleteĀ the service + yes Y | cf delete-service getgov-$DESTINATION_ENVIRONMENT-database + # createĀ it again + cf create-service aws-rds micro-psql getgov-$DESTINATION_ENVIRONMENT-database + # waitĀ for it be created (up to 5 mins) + # this checks the creation cf service getgov-$DESTINATION_ENVIRONMENT-database + # the below command with check ā€œstatusā€ line using cf service command mentioned above. if it says ā€œcreate in progressā€ it will keep waiting otherwise the next steps fail + + timeout 480 bash -c "until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' + do + echo 'Database not up yet, waiting...' + sleep 30 + done" + + # rebindĀ the service + cf bind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database + #restageĀ the app or it will not connect to the database right for the next commands + cf restage getgov-$DESTINATION_ENVIRONMENT + # waitĀ for the above command to finish + # if it is taking way to long and the annoying ā€œinstance startingā€ line that keeps repeating, then run following two commands in a separate window. This will interrupt the death loop where it keeps hitting an error with it failing health checks + # create the cacheĀ table and run migrations + cf run-task getgov-$DESTINATION_ENVIRONMENT --command 'python manage.py createcachetable' --name createcachetable + cf run-task getgov-$DESTINATION_ENVIRONMENT --wait --command 'python manage.py migrate' --name migrate + + # load fixtures + cf run-task getgov-$DESTINATION_ENVIRONMENT --wait --command 'python manage.py load' --name loaddata diff --git a/docs/developer/workflows/README.md b/docs/developer/workflows/README.md new file mode 100644 index 000000000..6cff81add --- /dev/null +++ b/docs/developer/workflows/README.md @@ -0,0 +1,7 @@ +# Workflows Docs + +======================== + +This directory contains files related to workflows + +Delete And Recreate Database is in [docs/ops](../workflows/delete-and-recreate-db.md/). \ No newline at end of file diff --git a/docs/developer/workflows/delete-and-recreate-db.md b/docs/developer/workflows/delete-and-recreate-db.md new file mode 100644 index 000000000..7b378ce47 --- /dev/null +++ b/docs/developer/workflows/delete-and-recreate-db.md @@ -0,0 +1,13 @@ +## Delete And Recreate Database + +This script destroys and recreates a database. This is another troubleshooting tool for issues with the database. + +1. unbinds the database +2. deletes it +3. recreates it +4. binds it back to the sandbox +5. runs migrations + +Addition Info in this slack thread: + +- [Slack thread](https://cisa-corp.slack.com/archives/C05BGB4L5NF/p1725495150772119) diff --git a/ops/manifests/manifest-ab.yaml b/ops/manifests/manifest-ab.yaml index 3ca800392..f5a3e2c3c 100644 --- a/ops/manifests/manifest-ab.yaml +++ b/ops/manifests/manifest-ab.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-ab.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-ad.yaml b/ops/manifests/manifest-ad.yaml index 73d6f96ff..6975f9f50 100644 --- a/ops/manifests/manifest-ad.yaml +++ b/ops/manifests/manifest-ad.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-ad.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-ag.yaml b/ops/manifests/manifest-ag.yaml index 68d630f3e..192b58edb 100644 --- a/ops/manifests/manifest-ag.yaml +++ b/ops/manifests/manifest-ag.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-ag.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-backup.yaml b/ops/manifests/manifest-backup.yaml index ab9e36d68..194b6e91c 100644 --- a/ops/manifests/manifest-backup.yaml +++ b/ops/manifests/manifest-backup.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-backup.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-bob.yaml b/ops/manifests/manifest-bob.yaml index f39d9e145..7af7e1df5 100644 --- a/ops/manifests/manifest-bob.yaml +++ b/ops/manifests/manifest-bob.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-bob.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-cb.yaml b/ops/manifests/manifest-cb.yaml index b9be98d27..e08f800fa 100644 --- a/ops/manifests/manifest-cb.yaml +++ b/ops/manifests/manifest-cb.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-cb.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-development.yaml b/ops/manifests/manifest-development.yaml index 23558ba4c..957cb0227 100644 --- a/ops/manifests/manifest-development.yaml +++ b/ops/manifests/manifest-development.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-development.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-dk.yaml b/ops/manifests/manifest-dk.yaml index 071efb416..6afbe9321 100644 --- a/ops/manifests/manifest-dk.yaml +++ b/ops/manifests/manifest-dk.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-dk.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-el.yaml b/ops/manifests/manifest-el.yaml index 4c7d4d4e4..ee5673700 100644 --- a/ops/manifests/manifest-el.yaml +++ b/ops/manifests/manifest-el.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-el.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-es.yaml b/ops/manifests/manifest-es.yaml index 7fd19b7a0..f0fc73d7e 100644 --- a/ops/manifests/manifest-es.yaml +++ b/ops/manifests/manifest-es.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-es.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-gd.yaml b/ops/manifests/manifest-gd.yaml index 89a7c2169..5c4f83cc5 100644 --- a/ops/manifests/manifest-gd.yaml +++ b/ops/manifests/manifest-gd.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-gd.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-hotgov.yaml b/ops/manifests/manifest-hotgov.yaml index 70cc97ee7..2aa37817a 100644 --- a/ops/manifests/manifest-hotgov.yaml +++ b/ops/manifests/manifest-hotgov.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-hotgov.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-ko.yaml b/ops/manifests/manifest-ko.yaml index a69493f9b..adc3dcc89 100644 --- a/ops/manifests/manifest-ko.yaml +++ b/ops/manifests/manifest-ko.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-ko.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-ky.yaml b/ops/manifests/manifest-ky.yaml index f416d7385..292b0575c 100644 --- a/ops/manifests/manifest-ky.yaml +++ b/ops/manifests/manifest-ky.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-ky.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-litterbox.yaml b/ops/manifests/manifest-litterbox.yaml index ae899ef3a..e2ab5489c 100644 --- a/ops/manifests/manifest-litterbox.yaml +++ b/ops/manifests/manifest-litterbox.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-litterbox.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-meoward.yaml b/ops/manifests/manifest-meoward.yaml index c47d9529d..ba452684e 100644 --- a/ops/manifests/manifest-meoward.yaml +++ b/ops/manifests/manifest-meoward.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-meoward.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-ms.yaml b/ops/manifests/manifest-ms.yaml index ac46f5d92..0068dfa02 100644 --- a/ops/manifests/manifest-ms.yaml +++ b/ops/manifests/manifest-ms.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-ms.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: DEBUG + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-nl.yaml b/ops/manifests/manifest-nl.yaml index d74174e7d..fbf3b0f5f 100644 --- a/ops/manifests/manifest-nl.yaml +++ b/ops/manifests/manifest-nl.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-nl.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-rb.yaml b/ops/manifests/manifest-rb.yaml index 570b49dde..02b099bdd 100644 --- a/ops/manifests/manifest-rb.yaml +++ b/ops/manifests/manifest-rb.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-rb.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-rh.yaml b/ops/manifests/manifest-rh.yaml index f44894ce8..abce35140 100644 --- a/ops/manifests/manifest-rh.yaml +++ b/ops/manifests/manifest-rh.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-rh.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-rjm.yaml b/ops/manifests/manifest-rjm.yaml index 048b44e95..b51db1b95 100644 --- a/ops/manifests/manifest-rjm.yaml +++ b/ops/manifests/manifest-rjm.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-rjm.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/manifests/manifest-stable.yaml b/ops/manifests/manifest-stable.yaml index 80c97339f..438a012a9 100644 --- a/ops/manifests/manifest-stable.yaml +++ b/ops/manifests/manifest-stable.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://manage.get.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: json # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Which OIDC provider to use diff --git a/ops/manifests/manifest-staging.yaml b/ops/manifests/manifest-staging.yaml index 38099cf17..7679b7248 100644 --- a/ops/manifests/manifest-staging.yaml +++ b/ops/manifests/manifest-staging.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-staging.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: json # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/ops/scripts/manifest-sandbox-template.yaml b/ops/scripts/manifest-sandbox-template.yaml index f0aee9664..ddd1860e1 100644 --- a/ops/scripts/manifest-sandbox-template.yaml +++ b/ops/scripts/manifest-sandbox-template.yaml @@ -21,6 +21,8 @@ applications: DJANGO_BASE_URL: https://getgov-ENVIRONMENT.app.cloud.gov # Tell Django how much stuff to log DJANGO_LOG_LEVEL: INFO + # tell django what log format to use: console or json. See settings.py for more details. + DJANGO_LOG_FORMAT: console # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8ecf36f52..927af3621 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -28,7 +28,11 @@ from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField from registrar.models import DomainInformation, Portfolio, UserPortfolioPermission, DomainInvitation from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from registrar.utility.email_invitations import send_domain_invitation_email, send_portfolio_invitation_email +from registrar.utility.email_invitations import ( + send_domain_invitation_email, + send_portfolio_admin_addition_emails, + send_portfolio_invitation_email, +) from registrar.views.utility.invitation_helper import ( get_org_membership, get_requested_user, @@ -1329,6 +1333,14 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): get_roles.short_description = "Roles" # type: ignore + def delete_queryset(self, request, queryset): + """We override the delete method in the model. + When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action + the model delete does not get called. This method gets called instead. + This override makes sure our code in the model gets executed in these situations.""" + for obj in queryset: + obj.delete() # Calls the overridden delete method on each instance + class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom user domain role admin class.""" @@ -1369,9 +1381,13 @@ class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): change_form_template = "django/admin/user_domain_role_change_form.html" + # Override for the delete confirmation page on the domain table (bulk delete action) + delete_selected_confirmation_template = "django/admin/user_domain_role_delete_selected_confirmation.html" + # Fixes a bug where non-superusers are redirected to the main page def delete_view(self, request, object_id, extra_context=None): """Custom delete_view implementation that specifies redirect behaviour""" + self.delete_confirmation_template = "django/admin/user_domain_role_delete_confirmation.html" response = super().delete_view(request, object_id, extra_context) if isinstance(response, HttpResponseRedirect) and not request.user.has_perm("registrar.full_access_permission"): @@ -1407,10 +1423,13 @@ class BaseInvitationAdmin(ListHeaderAdmin): Normal flow on successful save_model on add is to redirect to changelist_view. If there are errors, flow is modified to instead render change form. """ - # store current messages from request so that they are preserved throughout the method + # store current messages from request in storage so that they are preserved throughout the + # method, as some flows remove and replace all messages, and so we store here to retrieve + # later storage = get_messages(request) - # Check if there are any error or warning messages in the `messages` framework - has_errors = any(message.level_tag in ["error", "warning"] for message in storage) + # Check if there are any error messages in the `messages` framework + # error messages stop the workflow; other message levels allow flow to continue as normal + has_errors = any(message.level_tag in ["error"] for message in storage) if has_errors: # Re-render the change form if there are errors or warnings @@ -1503,6 +1522,8 @@ class DomainInvitationAdmin(BaseInvitationAdmin): autocomplete_fields = ["domain"] change_form_template = "django/admin/domain_invitation_change_form.html" + # Override for the delete confirmation page on the domain table (bulk delete action) + delete_selected_confirmation_template = "django/admin/domain_invitation_delete_selected_confirmation.html" # Select domain invitations to change -> Domain invitations def changelist_view(self, request, extra_context=None): @@ -1512,6 +1533,16 @@ class DomainInvitationAdmin(BaseInvitationAdmin): # Get the filtered values return super().changelist_view(request, extra_context=extra_context) + def delete_view(self, request, object_id, extra_context=None): + """ + Custom delete_view to perform additional actions or customize the template. + """ + # Set the delete template to a custom one + self.delete_confirmation_template = "django/admin/domain_invitation_delete_confirmation.html" + response = super().delete_view(request, object_id, extra_context=extra_context) + + return response + def save_model(self, request, obj, form, change): """ Override the save_model method. @@ -1540,7 +1571,9 @@ class DomainInvitationAdmin(BaseInvitationAdmin): and not member_of_this_org and not member_of_a_different_org ): - send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=domain_org) + send_portfolio_invitation_email( + email=requested_email, requestor=requestor, portfolio=domain_org, is_admin_invitation=False + ) portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( email=requested_email, portfolio=domain_org, @@ -1552,13 +1585,14 @@ class DomainInvitationAdmin(BaseInvitationAdmin): portfolio_invitation.save() messages.success(request, f"{requested_email} has been invited to the organization: {domain_org}") - send_domain_invitation_email( + if not send_domain_invitation_email( email=requested_email, requestor=requestor, domains=domain, is_member_of_different_org=member_of_a_different_org, requested_user=requested_user, - ) + ): + messages.warning(request, "Could not send email confirmation to existing domain managers.") if requested_user is not None: # Domain Invitation creation for an existing User obj.retrieve() @@ -1630,33 +1664,68 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin): Emails sent to requested user / email. When exceptions are raised, return without saving model. """ - if not change: # Only send email if this is a new PortfolioInvitation (creation) + try: portfolio = obj.portfolio requested_email = obj.email requestor = request.user - # Look up a user with that email - requested_user = get_requested_user(requested_email) + is_admin_invitation = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in obj.roles + if not change: # Only send email if this is a new PortfolioInvitation (creation) + # Look up a user with that email + requested_user = get_requested_user(requested_email) - permission_exists = UserPortfolioPermission.objects.filter( - user__email=requested_email, portfolio=portfolio, user__email__isnull=False - ).exists() - try: + permission_exists = UserPortfolioPermission.objects.filter( + user__email=requested_email, portfolio=portfolio, user__email__isnull=False + ).exists() if not permission_exists: # if permission does not exist for a user with requested_email, send email - send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=portfolio) + if not send_portfolio_invitation_email( + email=requested_email, + requestor=requestor, + portfolio=portfolio, + is_admin_invitation=is_admin_invitation, + ): + messages.warning( + self.request, "Could not send email notification to existing organization admins." + ) # if user exists for email, immediately retrieve portfolio invitation upon creation if requested_user is not None: obj.retrieve() messages.success(request, f"{requested_email} has been invited.") else: messages.warning(request, "User is already a member of this portfolio.") - except Exception as e: - # when exception is raised, handle and do not save the model - handle_invitation_exceptions(request, e, requested_email) - return + else: # Handle the case when updating an existing PortfolioInvitation + # Retrieve the existing object from the database + existing_obj = PortfolioInvitation.objects.get(pk=obj.pk) + + # Check if the previous roles did NOT include ORGANIZATION_ADMIN + # and the new roles DO include ORGANIZATION_ADMIN + was_not_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in existing_obj.roles + # Check also if status is INVITED, ignore role changes for other statuses + is_invited = obj.status == PortfolioInvitation.PortfolioInvitationStatus.INVITED + + if was_not_admin and is_admin_invitation and is_invited: + # send email to existing portfolio admins if new admin + if not send_portfolio_admin_addition_emails( + email=requested_email, + requestor=requestor, + portfolio=portfolio, + ): + messages.warning(request, "Could not send email notification to existing organization admins.") + except Exception as e: + # when exception is raised, handle and do not save the model + handle_invitation_exceptions(request, e, requested_email) + return # Call the parent save method to save the object super().save_model(request, obj, form, change) + def delete_queryset(self, request, queryset): + """We override the delete method in the model. + When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action, + the model delete does not get called. This method gets called instead. + This override makes sure our code in the model gets executed in these situations.""" + for obj in queryset: + obj.delete() # Calls the overridden delete method on each instance + class DomainInformationResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the diff --git a/src/registrar/assets/src/js/getgov/table-base.js b/src/registrar/assets/src/js/getgov/table-base.js index 0abfee9b6..ce4397887 100644 --- a/src/registrar/assets/src/js/getgov/table-base.js +++ b/src/registrar/assets/src/js/getgov/table-base.js @@ -129,7 +129,7 @@ export class BaseTable { this.displayName = itemName; this.sectionSelector = itemName + 's'; this.tableWrapper = document.getElementById(`${this.sectionSelector}__table-wrapper`); - this.tableHeaders = document.querySelectorAll(`#${this.sectionSelector} th[data-sortable]`); + this.tableHeaderSortButtons = document.querySelectorAll(`#${this.sectionSelector} th[data-sortable] button`); this.currentSortBy = 'id'; this.currentOrder = 'asc'; this.currentStatus = []; @@ -303,13 +303,18 @@ export class BaseTable { * A helper that resets sortable table headers * */ - unsetHeader = (header) => { - header.removeAttribute('aria-sort'); - let headerName = header.innerText; - const headerLabel = `${headerName}, sortable column, currently unsorted"`; - const headerButtonLabel = `Click to sort by ascending order.`; - header.setAttribute("aria-label", headerLabel); - header.querySelector('.usa-table__header__button').setAttribute("title", headerButtonLabel); + unsetHeader = (headerSortButton) => { + let header = headerSortButton.closest('th'); + if (header) { + header.removeAttribute('aria-sort'); + let headerName = header.innerText; + const headerLabel = `${headerName}, sortable column, currently unsorted"`; + const headerButtonLabel = `Click to sort by ascending order.`; + header.setAttribute("aria-label", headerLabel); + header.querySelector('.usa-table__header__button').setAttribute("title", headerButtonLabel); + } else { + console.warn('Issue with DOM'); + } }; /** @@ -505,24 +510,21 @@ export class BaseTable { // Add event listeners to table headers for sorting initializeTableHeaders() { - this.tableHeaders.forEach(header => { - header.addEventListener('click', event => { - let button = header.querySelector('.usa-table__header__button') - const sortBy = header.getAttribute('data-sortable'); - let order = 'asc'; - // sort order will be ascending, unless the currently sorted column is ascending, and the user - // is selecting the same column to sort in descending order - if (sortBy === this.currentSortBy) { - order = this.currentOrder === 'asc' ? 'desc' : 'asc'; - } - // load the results with the updated sort - this.loadTable(1, sortBy, order); - // If the click occurs outside of the button, need to simulate a button click in order - // for USWDS listener on the button to execute. - // Check first to see if click occurs outside of the button - if (!button.contains(event.target)) { - // Simulate a button click - button.click(); + this.tableHeaderSortButtons.forEach(tableHeader => { + tableHeader.addEventListener('click', event => { + let header = tableHeader.closest('th'); + if (header) { + const sortBy = header.getAttribute('data-sortable'); + let order = 'asc'; + // sort order will be ascending, unless the currently sorted column is ascending, and the user + // is selecting the same column to sort in descending order + if (sortBy === this.currentSortBy) { + order = this.currentOrder === 'asc' ? 'desc' : 'asc'; + } + // load the results with the updated sort + this.loadTable(1, sortBy, order); + } else { + console.warn('Issue with DOM'); } }); }); @@ -587,9 +589,9 @@ export class BaseTable { // Reset UI and accessibility resetHeaders() { - this.tableHeaders.forEach(header => { + this.tableHeaderSortButtons.forEach(headerSortButton => { // Unset sort UI in headers - this.unsetHeader(header); + this.unsetHeader(headerSortButton); }); // Reset the announcement region this.tableAnnouncementRegion.innerHTML = ''; diff --git a/src/registrar/assets/src/js/getgov/table-member-domains.js b/src/registrar/assets/src/js/getgov/table-member-domains.js index f9b789e1f..d1455c4dc 100644 --- a/src/registrar/assets/src/js/getgov/table-member-domains.js +++ b/src/registrar/assets/src/js/getgov/table-member-domains.js @@ -35,16 +35,19 @@ export class MemberDomainsTable extends BaseTable { showElement(dataWrapper); hideElement(noSearchResultsWrapper); hideElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = ''; } else { hideElement(dataWrapper); showElement(noSearchResultsWrapper); hideElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = this.noSearchResultsWrapper.innerHTML; } } else { hideElement(searchSection); hideElement(dataWrapper); hideElement(noSearchResultsWrapper); showElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = this.noDataWrapper.innerHTML; } }; } diff --git a/src/registrar/assets/src/sass/_theme/_tooltips.scss b/src/registrar/assets/src/sass/_theme/_tooltips.scss index 22b5cf534..e1e31cbec 100644 --- a/src/registrar/assets/src/sass/_theme/_tooltips.scss +++ b/src/registrar/assets/src/sass/_theme/_tooltips.scss @@ -29,7 +29,7 @@ font-weight: 400 !important; } -.domains__table { +.domains__table, .usa-table { /* Trick tooltips in the domains table to do 2 things... 1 - Shrink itself to a padded viewport window diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index a58e3e2f9..78439188e 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -25,6 +25,7 @@ from typing import Final from botocore.config import Config import json import logging +import traceback from django.utils.log import ServerFormatter # # # ### @@ -60,6 +61,7 @@ env_db_url = env.dj_db_url("DATABASE_URL") env_debug = env.bool("DJANGO_DEBUG", default=False) env_is_production = env.bool("IS_PRODUCTION", default=False) env_log_level = env.str("DJANGO_LOG_LEVEL", "DEBUG") +env_log_format = env.str("DJANGO_LOG_FORMAT", "console") env_base_url: str = env.str("DJANGO_BASE_URL") env_getgov_public_site_url = env.str("GETGOV_PUBLIC_SITE_URL", "") env_oidc_active_provider = env.str("OIDC_ACTIVE_PROVIDER", "identity sandbox") @@ -471,7 +473,11 @@ class JsonFormatter(logging.Formatter): "lineno": record.lineno, "message": record.getMessage(), } - return json.dumps(log_record) + # Capture exception info if it exists + if record.exc_info: + log_record["exception"] = "".join(traceback.format_exception(*record.exc_info)) + + return json.dumps(log_record, ensure_ascii=False) class JsonServerFormatter(ServerFormatter): @@ -487,12 +493,18 @@ class JsonServerFormatter(ServerFormatter): return json.dumps(log_entry) -# default to json formatted logs -server_formatter, console_formatter = "json.server", "json" - -# don't use json format locally, it makes logs hard to read in console +# If we're running locally we don't want json formatting if "localhost" in env_base_url: - server_formatter, console_formatter = "django.server", "verbose" + django_handlers = ["console"] +elif env_log_format == "json": + # in production we need everything to be logged as json so that log levels are parsed correctly + django_handlers = ["json"] +else: + # for non-production non-local environments: + # - send ERROR and above to json handler + # - send below ERROR to console handler with verbose formatting + # yes this is janky but it's the best we can do for now + django_handlers = ["split_console", "split_json"] LOGGING = { "version": 1, @@ -526,29 +538,52 @@ LOGGING = { "console": { "level": env_log_level, "class": "logging.StreamHandler", - "formatter": console_formatter, + "formatter": "verbose", + }, + # Special handlers for split logging case + "split_console": { + "level": env_log_level, + "class": "logging.StreamHandler", + "formatter": "verbose", + "filters": ["below_error"], + }, + "split_json": { + "level": "ERROR", + "class": "logging.StreamHandler", + "formatter": "json", }, "django.server": { "level": "INFO", "class": "logging.StreamHandler", - "formatter": server_formatter, + "formatter": "django.server", + }, + "json": { + "level": env_log_level, + "class": "logging.StreamHandler", + "formatter": "json", }, # No file logger is configured, # because containerized apps # do not log to the file system. }, + "filters": { + "below_error": { + "()": "django.utils.log.CallbackFilter", + "callback": lambda record: record.levelno < logging.ERROR, + } + }, # define loggers: these are "sinks" into which # messages are sent for processing "loggers": { # Django's generic logger "django": { - "handlers": ["console"], + "handlers": django_handlers, "level": "INFO", "propagate": False, }, # Django's template processor "django.template": { - "handlers": ["console"], + "handlers": django_handlers, "level": "INFO", "propagate": False, }, @@ -566,19 +601,19 @@ LOGGING = { }, # OpenID Connect logger "oic": { - "handlers": ["console"], + "handlers": django_handlers, "level": "INFO", "propagate": False, }, # Django wrapper for OpenID Connect "djangooidc": { - "handlers": ["console"], + "handlers": django_handlers, "level": "INFO", "propagate": False, }, # Our app! "registrar": { - "handlers": ["console"], + "handlers": django_handlers, "level": "DEBUG", "propagate": False, }, @@ -586,7 +621,7 @@ LOGGING = { # root logger catches anything, unless # defined by a more specific logger "root": { - "handlers": ["console"], + "handlers": django_handlers, "level": "INFO", }, } diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index beb38e104..eb095c5ca 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -20,7 +20,6 @@ from registrar.views.report_views import ( AnalyticsView, ExportDomainRequestDataFull, ExportDataTypeUser, - ExportDataTypeRequests, ExportMembersPortfolio, ) @@ -260,11 +259,6 @@ urlpatterns = [ ExportDataTypeUser.as_view(), name="export_data_type_user", ), - path( - "reports/export_data_type_requests/", - ExportDataTypeRequests.as_view(), - name="export_data_type_requests", - ), path( "domain-request//edit/", views.DomainRequestWizard.as_view(), diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index b3d9c3727..a078c81ac 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -56,12 +56,11 @@ def add_path_to_context(request): def portfolio_permissions(request): """Make portfolio permissions for the request user available in global context""" portfolio_context = { - "has_base_portfolio_permission": False, + "has_view_portfolio_permission": False, + "has_edit_portfolio_permission": False, "has_any_domains_portfolio_permission": False, "has_any_requests_portfolio_permission": False, "has_edit_request_portfolio_permission": False, - "has_view_suborganization_portfolio_permission": False, - "has_edit_suborganization_portfolio_permission": False, "has_view_members_portfolio_permission": False, "has_edit_members_portfolio_permission": False, "portfolio": None, @@ -82,15 +81,11 @@ def portfolio_permissions(request): } ) - # Linting: line too long - view_suborg = request.user.has_view_suborganization_portfolio_permission(portfolio) - edit_suborg = request.user.has_edit_suborganization_portfolio_permission(portfolio) if portfolio: return { - "has_base_portfolio_permission": request.user.has_base_portfolio_permission(portfolio), + "has_view_portfolio_permission": request.user.has_view_portfolio_permission(portfolio), + "has_edit_portfolio_permission": request.user.has_edit_portfolio_permission(portfolio), "has_edit_request_portfolio_permission": request.user.has_edit_request_portfolio_permission(portfolio), - "has_view_suborganization_portfolio_permission": view_suborg, - "has_edit_suborganization_portfolio_permission": edit_suborg, "has_any_domains_portfolio_permission": request.user.has_any_domains_portfolio_permission(portfolio), "has_any_requests_portfolio_permission": request.user.has_any_requests_portfolio_permission(portfolio), "has_view_members_portfolio_permission": request.user.has_view_members_portfolio_permission(portfolio), diff --git a/src/registrar/fixtures/fixtures_domains.py b/src/registrar/fixtures/fixtures_domains.py index 4606024d0..8855194f8 100644 --- a/src/registrar/fixtures/fixtures_domains.py +++ b/src/registrar/fixtures/fixtures_domains.py @@ -3,7 +3,6 @@ from django.utils import timezone import logging import random from faker import Faker -from django.db import transaction from registrar.fixtures.fixtures_requests import DomainRequestFixture from registrar.fixtures.fixtures_users import UserFixture @@ -29,19 +28,18 @@ class DomainFixture(DomainRequestFixture): def load(cls): # Lumped under .atomic to ensure we don't make redundant DB calls. # This bundles them all together, and then saves it in a single call. - with transaction.atomic(): - try: - # Get the usernames of users created in the UserFixture - created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF] + try: + # Get the usernames of users created in the UserFixture + created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF] - # Filter users to only include those created by the fixture - users = list(User.objects.filter(username__in=created_usernames)) - except Exception as e: - logger.warning(e) - return + # Filter users to only include those created by the fixture + users = list(User.objects.filter(username__in=created_usernames)) + except Exception as e: + logger.warning(e) + return - # Approve each user associated with `in review` status domains - cls._approve_domain_requests(users) + # Approve each user associated with `in review` status domains + cls._approve_domain_requests(users) @staticmethod def _generate_fake_expiration_date(days_in_future=365): diff --git a/src/registrar/fixtures/fixtures_portfolios.py b/src/registrar/fixtures/fixtures_portfolios.py index 2a391fb16..b93b9efdd 100644 --- a/src/registrar/fixtures/fixtures_portfolios.py +++ b/src/registrar/fixtures/fixtures_portfolios.py @@ -1,7 +1,6 @@ import logging import random from faker import Faker -from django.db import transaction from registrar.models import User, DomainRequest, FederalAgency from registrar.models.portfolio import Portfolio @@ -84,42 +83,38 @@ class PortfolioFixture: def load(cls): """Creates portfolios.""" logger.info("Going to load %s portfolios" % len(cls.PORTFOLIOS)) + try: + user = User.objects.all().last() + except Exception as e: + logger.warning(e) + return - # Lumped under .atomic to ensure we don't make redundant DB calls. - # This bundles them all together, and then saves it in a single call. - with transaction.atomic(): + portfolios_to_create = [] + for portfolio_data in cls.PORTFOLIOS: + organization_name = portfolio_data["organization_name"] + + # Check if portfolio with the organization name already exists + if Portfolio.objects.filter(organization_name=organization_name).exists(): + logger.info( + f"Portfolio with organization name '{organization_name}' already exists, skipping creation." + ) + continue + + try: + portfolio = Portfolio( + creator=user, + organization_name=portfolio_data["organization_name"], + ) + cls._set_non_foreign_key_fields(portfolio, portfolio_data) + cls._set_foreign_key_fields(portfolio, portfolio_data, user) + portfolios_to_create.append(portfolio) + except Exception as e: + logger.warning(e) + + # Bulk create portfolios + if len(portfolios_to_create) > 0: try: - user = User.objects.all().last() + Portfolio.objects.bulk_create(portfolios_to_create) + logger.info(f"Successfully created {len(portfolios_to_create)} portfolios") except Exception as e: - logger.warning(e) - return - - portfolios_to_create = [] - for portfolio_data in cls.PORTFOLIOS: - organization_name = portfolio_data["organization_name"] - - # Check if portfolio with the organization name already exists - if Portfolio.objects.filter(organization_name=organization_name).exists(): - logger.info( - f"Portfolio with organization name '{organization_name}' already exists, skipping creation." - ) - continue - - try: - portfolio = Portfolio( - creator=user, - organization_name=portfolio_data["organization_name"], - ) - cls._set_non_foreign_key_fields(portfolio, portfolio_data) - cls._set_foreign_key_fields(portfolio, portfolio_data, user) - portfolios_to_create.append(portfolio) - except Exception as e: - logger.warning(e) - - # Bulk create domain requests - if len(portfolios_to_create) > 0: - try: - Portfolio.objects.bulk_create(portfolios_to_create) - logger.info(f"Successfully created {len(portfolios_to_create)} portfolios") - except Exception as e: - logger.warning(f"Error bulk creating portfolios: {e}") + logger.warning(f"Error bulk creating portfolios: {e}") diff --git a/src/registrar/fixtures/fixtures_requests.py b/src/registrar/fixtures/fixtures_requests.py index c4d824b37..6eee6438f 100644 --- a/src/registrar/fixtures/fixtures_requests.py +++ b/src/registrar/fixtures/fixtures_requests.py @@ -3,7 +3,6 @@ from django.utils import timezone import logging import random from faker import Faker -from django.db import transaction from registrar.fixtures.fixtures_portfolios import PortfolioFixture from registrar.fixtures.fixtures_suborganizations import SuborganizationFixture @@ -303,24 +302,17 @@ class DomainRequestFixture: def load(cls): """Creates domain requests for each user in the database.""" logger.info("Going to load %s domain requests" % len(cls.DOMAINREQUESTS)) + try: + # Get the usernames of users created in the UserFixture + created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF] - # Lumped under .atomic to ensure we don't make redundant DB calls. - # This bundles them all together, and then saves it in a single call. - # The atomic block will cause the code to stop executing if one instance in the - # nested iteration fails, which will cause an early exit and make it hard to debug. - # Comment out with transaction.atomic() when debugging. - with transaction.atomic(): - try: - # Get the usernames of users created in the UserFixture - created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF] + # Filter users to only include those created by the fixture + users = list(User.objects.filter(username__in=created_usernames)) + except Exception as e: + logger.warning(e) + return - # Filter users to only include those created by the fixture - users = list(User.objects.filter(username__in=created_usernames)) - except Exception as e: - logger.warning(e) - return - - cls._create_domain_requests(users) + cls._create_domain_requests(users) @classmethod def _create_domain_requests(cls, users): # noqa: C901 diff --git a/src/registrar/fixtures/fixtures_suborganizations.py b/src/registrar/fixtures/fixtures_suborganizations.py index af7e02804..787ce8f75 100644 --- a/src/registrar/fixtures/fixtures_suborganizations.py +++ b/src/registrar/fixtures/fixtures_suborganizations.py @@ -1,6 +1,5 @@ import logging from faker import Faker -from django.db import transaction from registrar.models.portfolio import Portfolio from registrar.models.suborganization import Suborganization @@ -34,14 +33,12 @@ class SuborganizationFixture: def load(cls): """Creates suborganizations.""" logger.info(f"Going to load {len(cls.SUBORGS)} suborgs") + portfolios = cls._get_portfolios() + if not portfolios: + return - with transaction.atomic(): - portfolios = cls._get_portfolios() - if not portfolios: - return - - suborgs_to_create = cls._prepare_suborgs_to_create(portfolios) - cls._bulk_create_suborgs(suborgs_to_create) + suborgs_to_create = cls._prepare_suborgs_to_create(portfolios) + cls._bulk_create_suborgs(suborgs_to_create) @classmethod def _get_portfolios(cls): diff --git a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py index 5f9fd64ef..e2c84f817 100644 --- a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py +++ b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py @@ -1,7 +1,6 @@ import logging import random from faker import Faker -from django.db import transaction from registrar.fixtures.fixtures_portfolios import PortfolioFixture from registrar.fixtures.fixtures_users import UserFixture @@ -26,56 +25,55 @@ class UserPortfolioPermissionFixture: # Lumped under .atomic to ensure we don't make redundant DB calls. # This bundles them all together, and then saves it in a single call. - with transaction.atomic(): - try: - # Get the usernames of users created in the UserFixture - created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF] + try: + # Get the usernames of users created in the UserFixture + created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF] - # Filter users to only include those created by the fixture - users = list(User.objects.filter(username__in=created_usernames)) + # Filter users to only include those created by the fixture + users = list(User.objects.filter(username__in=created_usernames)) - organization_names = [portfolio["organization_name"] for portfolio in PortfolioFixture.PORTFOLIOS] + organization_names = [portfolio["organization_name"] for portfolio in PortfolioFixture.PORTFOLIOS] - portfolios = list(Portfolio.objects.filter(organization_name__in=organization_names)) + portfolios = list(Portfolio.objects.filter(organization_name__in=organization_names)) - if not users: - logger.warning("User fixtures missing.") - return - - if not portfolios: - logger.warning("Portfolio fixtures missing.") - return - - except Exception as e: - logger.warning(e) + if not users: + logger.warning("User fixtures missing.") return - user_portfolio_permissions_to_create = [] - for user in users: - # Assign a random portfolio to a user - portfolio = random.choice(portfolios) # nosec - try: - if not UserPortfolioPermission.objects.filter(user=user, portfolio=portfolio).exists(): - user_portfolio_permission = UserPortfolioPermission( - user=user, - portfolio=portfolio, - roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], - additional_permissions=[ - UserPortfolioPermissionChoices.EDIT_MEMBERS, - UserPortfolioPermissionChoices.EDIT_REQUESTS, - ], - ) - user_portfolio_permissions_to_create.append(user_portfolio_permission) - else: - logger.info( - f"Permission exists for user '{user.username}' " - f"on portfolio '{portfolio.organization_name}'." - ) - except Exception as e: - logger.warning(e) + if not portfolios: + logger.warning("Portfolio fixtures missing.") + return - # Bulk create permissions - cls._bulk_create_permissions(user_portfolio_permissions_to_create) + except Exception as e: + logger.warning(e) + return + + user_portfolio_permissions_to_create = [] + for user in users: + # Assign a random portfolio to a user + portfolio = random.choice(portfolios) # nosec + try: + if not UserPortfolioPermission.objects.filter(user=user, portfolio=portfolio).exists(): + user_portfolio_permission = UserPortfolioPermission( + user=user, + portfolio=portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.EDIT_MEMBERS, + UserPortfolioPermissionChoices.EDIT_REQUESTS, + ], + ) + user_portfolio_permissions_to_create.append(user_portfolio_permission) + else: + logger.info( + f"Permission exists for user '{user.username}' " + f"on portfolio '{portfolio.organization_name}'." + ) + except Exception as e: + logger.warning(e) + + # Bulk create permissions + cls._bulk_create_permissions(user_portfolio_permissions_to_create) @classmethod def _bulk_create_permissions(cls, user_portfolio_permissions_to_create): diff --git a/src/registrar/fixtures/fixtures_users.py b/src/registrar/fixtures/fixtures_users.py index 977bf0858..876bc9fb5 100644 --- a/src/registrar/fixtures/fixtures_users.py +++ b/src/registrar/fixtures/fixtures_users.py @@ -1,6 +1,5 @@ import logging from faker import Faker -from django.db import transaction from registrar.models import ( User, @@ -455,10 +454,9 @@ class UserFixture: @classmethod def load(cls): - with transaction.atomic(): - cls.load_users(cls.ADMINS, "full_access_group", are_superusers=True) - cls.load_users(cls.STAFF, "cisa_analysts_group") + cls.load_users(cls.ADMINS, "full_access_group", are_superusers=True) + cls.load_users(cls.STAFF, "cisa_analysts_group") - # Combine ADMINS and STAFF lists - all_users = cls.ADMINS + cls.STAFF - cls.load_allowed_emails(cls, all_users, additional_emails=cls.ADDITIONAL_ALLOWED_EMAILS) + # Combine ADMINS and STAFF lists + all_users = cls.ADMINS + cls.STAFF + cls.load_allowed_emails(cls, all_users, additional_emails=cls.ADDITIONAL_ALLOWED_EMAILS) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 89f7522b3..d7a02b124 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -2,7 +2,8 @@ from __future__ import annotations # allows forward references in annotations import logging from api.views import DOMAIN_API_MESSAGES from phonenumber_field.formfields import PhoneNumberField # type: ignore - +from registrar.models.portfolio import Portfolio +from registrar.utility.waffle import flag_is_active_anywhere from django import forms from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe @@ -321,7 +322,8 @@ class OrganizationContactForm(RegistrarForm): # if it has been filled in when required. # uncomment to see if modelChoiceField can be an arg later required=False, - queryset=FederalAgency.objects.exclude(agency__in=excluded_agencies), + # We populate this queryset in init. + queryset=FederalAgency.objects.none(), widget=ComboboxWidget, ) organization_name = forms.CharField( @@ -363,6 +365,20 @@ class OrganizationContactForm(RegistrarForm): label="Urbanization (required for Puerto Rico only)", ) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + # Set the queryset for federal agency. + # If the organization_requests flag is active, We want to exclude agencies with a portfolio. + federal_agency_queryset = FederalAgency.objects.exclude(agency__in=self.excluded_agencies) + if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): + # Exclude both predefined agencies and those matching portfolio records in one query + federal_agency_queryset = federal_agency_queryset.exclude( + id__in=Portfolio.objects.values_list("federal_agency__id", flat=True) + ) + + self.fields["federal_agency"].queryset = federal_agency_queryset + def clean_federal_agency(self): """Require something to be selected when this is a federal agency.""" federal_agency = self.cleaned_data.get("federal_agency", None) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index c9ef280b0..2725224f1 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -312,6 +312,32 @@ class BasePortfolioMemberForm(forms.ModelForm): self.initial["domain_permissions"] = selected_domain_permission self.initial["member_permissions"] = selected_member_permission + def is_change_from_member_to_admin(self) -> bool: + """ + Checks if the roles have changed from not containing ORGANIZATION_ADMIN + to containing ORGANIZATION_ADMIN. + """ + previous_roles = set(self.initial.get("roles", [])) # Initial roles before change + new_roles = set(self.cleaned_data.get("roles", [])) # New roles after change + + return ( + UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in previous_roles + and UserPortfolioRoleChoices.ORGANIZATION_ADMIN in new_roles + ) + + def is_change_from_admin_to_member(self) -> bool: + """ + Checks if the roles have changed from containing ORGANIZATION_ADMIN + to not containing ORGANIZATION_ADMIN. + """ + previous_roles = set(self.initial.get("roles", [])) # Initial roles before change + new_roles = set(self.cleaned_data.get("roles", [])) # New roles after change + + return ( + UserPortfolioRoleChoices.ORGANIZATION_ADMIN in previous_roles + and UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in new_roles + ) + class PortfolioMemberForm(BasePortfolioMemberForm): """ diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 75c5d6acc..d753d0ce8 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -204,7 +204,9 @@ class Command(BaseCommand): # Fetch all users with manager roles for the domains # select_related means that a db query will not be occur when you do user_domain_role.user # Its similar to a set or dict in that it costs slightly more upfront in exchange for perf later - user_domain_roles = UserDomainRole.objects.select_related("user").filter(domain__in=domains, role=UserDomainRole.Roles.MANAGER) + user_domain_roles = UserDomainRole.objects.select_related("user").filter( + domain__in=domains, role=UserDomainRole.Roles.MANAGER + ) domain_managers.update(user_domain_roles) invited_managers: set[str] = set() @@ -243,9 +245,10 @@ class Command(BaseCommand): _, created = PortfolioInvitation.objects.get_or_create( portfolio=portfolio, email=email, - defaults={"status": PortfolioInvitation.PortfolioInvitationStatus.INVITED, - "roles": [UserPortfolioRoleChoices.ORGANIZATION_MEMBER] - }, + defaults={ + "status": PortfolioInvitation.PortfolioInvitationStatus.INVITED, + "roles": [UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + }, ) if created: self.added_invitations.add(email) diff --git a/src/registrar/management/commands/remove_unused_portfolios.py b/src/registrar/management/commands/remove_unused_portfolios.py index 4940cc16f..859318a45 100644 --- a/src/registrar/management/commands/remove_unused_portfolios.py +++ b/src/registrar/management/commands/remove_unused_portfolios.py @@ -149,9 +149,9 @@ class Command(BaseCommand): ) return - with transaction.atomic(): - # Try to delete the portfolios - try: + # Try to delete the portfolios + try: + with transaction.atomic(): summary = [] for portfolio in portfolios_to_delete: portfolio_summary = [f"---- CASCADE SUMMARY for {portfolio.organization_name} -----"] @@ -222,14 +222,14 @@ class Command(BaseCommand): """ ) - except IntegrityError as e: - logger.info( - f"""{TerminalColors.FAIL} - Could not delete some portfolios due to integrity constraints: - {e} - {TerminalColors.ENDC} - """ - ) + except IntegrityError as e: + logger.info( + f"""{TerminalColors.FAIL} + Could not delete some portfolios due to integrity constraints: + {e} + {TerminalColors.ENDC} + """ + ) def handle(self, *args, **options): # Get all Portfolio entries not in the allowed portfolios list diff --git a/src/registrar/migrations/0140_alter_portfolioinvitation_additional_permissions_and_more.py b/src/registrar/migrations/0140_alter_portfolioinvitation_additional_permissions_and_more.py new file mode 100644 index 000000000..8360d7a72 --- /dev/null +++ b/src/registrar/migrations/0140_alter_portfolioinvitation_additional_permissions_and_more.py @@ -0,0 +1,60 @@ +# Generated by Django 4.2.10 on 2025-02-04 11:18 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0139_alter_domainrequest_action_needed_reason"), + ] + + operations = [ + migrations.AlterField( + model_name="portfolioinvitation", + name="additional_permissions", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("view_all_domains", "View all domains and domain reports"), + ("view_managed_domains", "View managed domains"), + ("view_members", "View members"), + ("edit_members", "Create and edit members"), + ("view_all_requests", "View all requests"), + ("edit_requests", "Create and edit requests"), + ("view_portfolio", "View organization"), + ("edit_portfolio", "Edit organization"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + migrations.AlterField( + model_name="userportfoliopermission", + name="additional_permissions", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("view_all_domains", "View all domains and domain reports"), + ("view_managed_domains", "View managed domains"), + ("view_members", "View members"), + ("edit_members", "Create and edit members"), + ("view_all_requests", "View all requests"), + ("edit_requests", "Create and edit requests"), + ("view_portfolio", "View organization"), + ("edit_portfolio", "Edit organization"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + ] diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index cb481db7a..649b3f93d 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -9,6 +9,7 @@ from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignor from django.db import models, IntegrityError from django.utils import timezone from typing import Any +from registrar.models.domain_invitation import DomainInvitation from registrar.models.host import Host from registrar.models.host_ip import HostIP from registrar.utility.enums import DefaultEmail @@ -1177,6 +1178,10 @@ class Domain(TimeStampedModel, DomainHelper): return "DNS needed" return self.state.capitalize() + def active_invitations(self): + """Returns only the active invitations (those with status 'invited').""" + return self.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED) + def map_epp_contact_to_public_contact(self, contact: eppInfo.InfoContactResultData, contact_id, contact_type): """Maps the Epp contact representation to a PublicContact object. @@ -1582,11 +1587,9 @@ class Domain(TimeStampedModel, DomainHelper): if self.is_expired() and self.state != self.State.UNKNOWN: # Given expired is not a physical state, but it is displayed as such, # We need custom logic to determine this message. - help_text = ( - "This domain has expired, but it is still online. " "To renew this domain, contact help@get.gov." - ) + help_text = "This domain has expired. Complete the online renewal process to maintain access." elif flag_is_active(request, "domain_renewal") and self.is_expiring(): - help_text = "This domain will expire soon. Contact one of the listed domain managers to renew the domain." + help_text = "This domain is expiring soon. Complete the online renewal process to maintain access." else: help_text = Domain.State.get_help_text(self.state) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index c5a0926ad..1cca3742f 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -15,8 +15,7 @@ from registrar.utility.constants import BranchChoices from auditlog.models import LogEntry from django.core.exceptions import ValidationError -from registrar.utility.waffle import flag_is_active_for_user - +from registrar.utility.waffle import flag_is_active_for_user, flag_is_active_anywhere from .utility.time_stamped_model import TimeStampedModel from ..utility.email import send_templated_email, EmailSendingError from itertools import chain @@ -947,7 +946,7 @@ class DomainRequest(TimeStampedModel): try: if not context: has_organization_feature_flag = flag_is_active_for_user(recipient, "organization_feature") - is_org_user = has_organization_feature_flag and recipient.has_base_portfolio_permission(self.portfolio) + is_org_user = has_organization_feature_flag and recipient.has_view_portfolio_permission(self.portfolio) context = { "domain_request": self, # This is the user that we refer to in the email @@ -1299,6 +1298,40 @@ class DomainRequest(TimeStampedModel): return True return False + def unlock_organization_contact(self) -> bool: + """Unlocks the organization_contact step.""" + if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): + # Check if the current federal agency is an outlawed one + if self.organization_type == self.OrganizationChoices.FEDERAL and self.federal_agency: + Portfolio = apps.get_model("registrar.Portfolio") + return ( + FederalAgency.objects.exclude( + id__in=Portfolio.objects.values_list("federal_agency__id", flat=True), + ) + .filter(id=self.federal_agency.id) + .exists() + ) + return bool( + self.federal_agency is not None + or self.organization_name is not None + or self.address_line1 is not None + or self.city is not None + or self.state_territory is not None + or self.zipcode is not None + or self.urbanization is not None + ) + + def unlock_other_contacts(self) -> bool: + """Unlocks the other contacts step""" + other_contacts_filled_out = self.other_contacts.filter( + first_name__isnull=False, + last_name__isnull=False, + title__isnull=False, + email__isnull=False, + phone__isnull=False, + ).exists() + return (self.has_other_contacts() and other_contacts_filled_out) or self.no_other_contacts_rationale is not None + # ## Form policies ## # # # These methods control what questions need to be answered by applicants @@ -1396,140 +1429,6 @@ class DomainRequest(TimeStampedModel): names = [n for n in [self.cisa_representative_first_name, self.cisa_representative_last_name] if n] return " ".join(names) if names else "Unknown" - def _is_federal_complete(self): - # Federal -> "Federal government branch" page can't be empty + Federal Agency selection can't be None - return not (self.federal_type is None or self.federal_agency is None) - - def _is_interstate_complete(self): - # Interstate -> "About your organization" page can't be empty - return self.about_your_organization is not None - - def _is_state_or_territory_complete(self): - # State -> ""Election office" page can't be empty - return self.is_election_board is not None - - def _is_tribal_complete(self): - # Tribal -> "Tribal name" and "Election office" page can't be empty - return self.tribe_name is not None and self.is_election_board is not None - - def _is_county_complete(self): - # County -> "Election office" page can't be empty - return self.is_election_board is not None - - def _is_city_complete(self): - # City -> "Election office" page can't be empty - return self.is_election_board is not None - - def _is_special_district_complete(self): - # Special District -> "Election office" and "About your organization" page can't be empty - return self.is_election_board is not None and self.about_your_organization is not None - - # Do we still want to test this after creator is autogenerated? Currently it went back to being selectable - def _is_creator_complete(self): - return self.creator is not None - - def _is_organization_name_and_address_complete(self): - return not ( - self.organization_name is None - and self.address_line1 is None - and self.city is None - and self.state_territory is None - and self.zipcode is None - ) - - def _is_senior_official_complete(self): - return self.senior_official is not None - - def _is_requested_domain_complete(self): - return self.requested_domain is not None - - def _is_purpose_complete(self): - return self.purpose is not None - - def _has_other_contacts_and_filled(self): - # Other Contacts Radio button is Yes and if all required fields are filled - return ( - self.has_other_contacts() - and self.other_contacts.filter( - first_name__isnull=False, - last_name__isnull=False, - title__isnull=False, - email__isnull=False, - phone__isnull=False, - ).exists() - ) - - def _has_no_other_contacts_gives_rationale(self): - # Other Contacts Radio button is No and a rationale is provided - return self.has_other_contacts() is False and self.no_other_contacts_rationale is not None - - def _is_other_contacts_complete(self): - if self._has_other_contacts_and_filled() or self._has_no_other_contacts_gives_rationale(): - return True - return False - - def _cisa_rep_check(self): - # Either does not have a CISA rep, OR has a CISA rep + both first name - # and last name are NOT empty and are NOT an empty string - to_return = ( - self.has_cisa_representative is True - and self.cisa_representative_first_name is not None - and self.cisa_representative_first_name != "" - and self.cisa_representative_last_name is not None - and self.cisa_representative_last_name != "" - ) or self.has_cisa_representative is False - - return to_return - - def _anything_else_radio_button_and_text_field_check(self): - # Anything else boolean is True + filled text field and it's not an empty string OR the boolean is No - return ( - self.has_anything_else_text is True and self.anything_else is not None and self.anything_else != "" - ) or self.has_anything_else_text is False - - def _is_additional_details_complete(self): - return self._cisa_rep_check() and self._anything_else_radio_button_and_text_field_check() - - def _is_policy_acknowledgement_complete(self): - return self.is_policy_acknowledged is not None - - def _is_general_form_complete(self, request): - return ( - self._is_creator_complete() - and self._is_organization_name_and_address_complete() - and self._is_senior_official_complete() - and self._is_requested_domain_complete() - and self._is_purpose_complete() - and self._is_other_contacts_complete() - and self._is_additional_details_complete() - and self._is_policy_acknowledgement_complete() - ) - - def _form_complete(self, request): - match self.generic_org_type: - case DomainRequest.OrganizationChoices.FEDERAL: - is_complete = self._is_federal_complete() - case DomainRequest.OrganizationChoices.INTERSTATE: - is_complete = self._is_interstate_complete() - case DomainRequest.OrganizationChoices.STATE_OR_TERRITORY: - is_complete = self._is_state_or_territory_complete() - case DomainRequest.OrganizationChoices.TRIBAL: - is_complete = self._is_tribal_complete() - case DomainRequest.OrganizationChoices.COUNTY: - is_complete = self._is_county_complete() - case DomainRequest.OrganizationChoices.CITY: - is_complete = self._is_city_complete() - case DomainRequest.OrganizationChoices.SPECIAL_DISTRICT: - is_complete = self._is_special_district_complete() - case DomainRequest.OrganizationChoices.SCHOOL_DISTRICT: - is_complete = True - case _: - # NOTE: Shouldn't happen, this is only if somehow they didn't choose an org type - is_complete = False - if not is_complete or not self._is_general_form_complete(request): - return False - return True - """The following converted_ property methods get field data from this domain request's portfolio, if there is an associated portfolio. If not, they return data from the domain request model.""" diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 11c564c36..8feeb0794 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -8,6 +8,7 @@ from registrar.models import DomainInvitation, UserPortfolioPermission from .utility.portfolio_helper import ( UserPortfolioPermissionChoices, UserPortfolioRoleChoices, + cleanup_after_portfolio_member_deletion, validate_portfolio_invitation, ) # type: ignore from .utility.time_stamped_model import TimeStampedModel @@ -115,3 +116,27 @@ class PortfolioInvitation(TimeStampedModel): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() validate_portfolio_invitation(self) + + def delete(self, *args, **kwargs): + + User = get_user_model() + + email = self.email # Capture the email before the instance is deleted + portfolio = self.portfolio # Capture the portfolio before the instance is deleted + + # Call the superclass delete method to actually delete the instance + super().delete(*args, **kwargs) + + if self.status == self.PortfolioInvitationStatus.INVITED: + + # Query the user by email + users = User.objects.filter(email=email) + + if users.count() > 1: + # This should never happen, log an error if more than one object is returned + logger.error(f"Multiple users found with the same email: {email}") + + # Retrieve the first user, or None if no users are found + user = users.first() + + cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=email, user=user) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 2b5b56a78..6f8ee499b 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -171,11 +171,14 @@ class User(AbstractUser): now = timezone.now().date() expiration_window = 60 threshold_date = now + timedelta(days=expiration_window) + acceptable_statuses = [Domain.State.UNKNOWN, Domain.State.DNS_NEEDED, Domain.State.READY] + num_of_expiring_domains = Domain.objects.filter( id__in=domain_ids, expiration_date__isnull=False, expiration_date__lte=threshold_date, expiration_date__gt=now, + state__in=acceptable_statuses, ).count() return num_of_expiring_domains @@ -207,10 +210,10 @@ class User(AbstractUser): return portfolio_permission in user_portfolio_perms._get_portfolio_permissions() - def has_base_portfolio_permission(self, portfolio): + def has_view_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO) - def has_edit_org_portfolio_permission(self, portfolio): + def has_edit_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_PORTFOLIO) def has_any_domains_portfolio_permission(self, portfolio): @@ -265,13 +268,6 @@ class User(AbstractUser): def has_edit_request_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) - # Field specific permission checks - def has_view_suborganization_portfolio_permission(self, portfolio): - return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) - - def has_edit_suborganization_portfolio_permission(self, portfolio): - return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) - def is_portfolio_admin(self, portfolio): return "Admin" in self.portfolio_role_summary(portfolio) @@ -290,7 +286,7 @@ class User(AbstractUser): # Define the conditions and their corresponding roles conditions_roles = [ - (self.has_edit_suborganization_portfolio_permission(portfolio), ["Admin"]), + (self.has_edit_portfolio_permission(portfolio), ["Admin"]), ( self.has_view_all_domains_portfolio_permission(portfolio) and self.has_any_requests_portfolio_permission(portfolio) @@ -303,20 +299,20 @@ class User(AbstractUser): ["View-only admin"], ), ( - self.has_base_portfolio_permission(portfolio) + self.has_view_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio) and self.has_any_domains_portfolio_permission(portfolio), ["Domain requestor", "Domain manager"], ), ( - self.has_base_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), + self.has_view_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), ["Domain requestor"], ), ( - self.has_base_portfolio_permission(portfolio) and self.has_any_domains_portfolio_permission(portfolio), + self.has_view_portfolio_permission(portfolio) and self.has_any_domains_portfolio_permission(portfolio), ["Domain manager"], ), - (self.has_base_portfolio_permission(portfolio), ["Member"]), + (self.has_view_portfolio_permission(portfolio), ["Member"]), ] # Evaluate conditions and add roles @@ -474,7 +470,7 @@ class User(AbstractUser): def is_org_user(self, request): has_organization_feature_flag = flag_is_active(request, "organization_feature") portfolio = request.session.get("portfolio") - return has_organization_feature_flag and self.has_base_portfolio_permission(portfolio) + return has_organization_feature_flag and self.has_view_portfolio_permission(portfolio) def get_user_domain_ids(self, request): """Returns either the domains ids associated with this user on UserDomainRole or Portfolio""" diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index c4be90a9b..5378dc185 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -5,6 +5,7 @@ from registrar.models.utility.portfolio_helper import ( UserPortfolioRoleChoices, DomainRequestPermissionDisplay, MemberPermissionDisplay, + cleanup_after_portfolio_member_deletion, validate_user_portfolio_permission, ) from .utility.time_stamped_model import TimeStampedModel @@ -26,13 +27,10 @@ class UserPortfolioPermission(TimeStampedModel): UserPortfolioPermissionChoices.EDIT_MEMBERS, UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, ], # NOTE: Check FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS before adding roles here. UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, ], } @@ -42,7 +40,6 @@ class UserPortfolioPermission(TimeStampedModel): UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ UserPortfolioPermissionChoices.EDIT_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_MEMBERS, - UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, ], } @@ -188,3 +185,13 @@ class UserPortfolioPermission(TimeStampedModel): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() validate_user_portfolio_permission(self) + + def delete(self, *args, **kwargs): + + user = self.user # Capture the user before the instance is deleted + portfolio = self.portfolio # Capture the portfolio before the instance is deleted + + # Call the superclass delete method to actually delete the instance + super().delete(*args, **kwargs) + + cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=user.email, user=user) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index b3bb07c3d..5feae1cc1 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -1,5 +1,6 @@ from registrar.utility import StrEnum from django.db import models +from django.db.models import Q from django.apps import apps from django.forms import ValidationError from registrar.utility.waffle import flag_is_active_for_user @@ -41,10 +42,6 @@ class UserPortfolioPermissionChoices(models.TextChoices): VIEW_PORTFOLIO = "view_portfolio", "View organization" EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" - # Domain: field specific permissions - VIEW_SUBORGANIZATION = "view_suborganization", "View suborganization" - EDIT_SUBORGANIZATION = "edit_suborganization", "Edit suborganization" - @classmethod def get_user_portfolio_permission_label(cls, user_portfolio_permission): return cls(user_portfolio_permission).label if user_portfolio_permission else None @@ -136,9 +133,10 @@ def validate_user_portfolio_permission(user_portfolio_permission): "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) - existing_invitations = PortfolioInvitation.objects.exclude( - portfolio=user_portfolio_permission.portfolio - ).filter(email=user_portfolio_permission.user.email) + existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email).exclude( + Q(portfolio=user_portfolio_permission.portfolio) + | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) + ) if existing_invitations.exists(): raise ValidationError( "This user is already assigned to a portfolio invitation. " @@ -195,8 +193,8 @@ def validate_portfolio_invitation(portfolio_invitation): if not flag_is_active_for_user(user, "multiple_portfolios"): existing_permissions = UserPortfolioPermission.objects.filter(user=user) - existing_invitations = PortfolioInvitation.objects.exclude(id=portfolio_invitation.id).filter( - email=portfolio_invitation.email + existing_invitations = PortfolioInvitation.objects.filter(email=portfolio_invitation.email).exclude( + Q(id=portfolio_invitation.id) | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) ) if existing_permissions.exists(): @@ -210,3 +208,32 @@ def validate_portfolio_invitation(portfolio_invitation): "This user is already assigned to a portfolio invitation. " "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) + + +def cleanup_after_portfolio_member_deletion(portfolio, email, user=None): + """ + Cleans up after removing a portfolio member or a portfolio invitation. + + Args: + portfolio: portfolio + user: passed when removing a portfolio member. + email: passed when removing a portfolio invitation, or passed as user.email + when removing a portfolio member. + """ + + DomainInvitation = apps.get_model("registrar.DomainInvitation") + UserDomainRole = apps.get_model("registrar.UserDomainRole") + + # Fetch domain invitations matching the criteria + invitations = DomainInvitation.objects.filter( + email=email, domain__domain_info__portfolio=portfolio, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + + # Call `cancel_invitation` on each invitation + for invitation in invitations: + invitation.cancel_invitation() + invitation.save() + + if user: + # Remove user's domain roles for the current portfolio + UserDomainRole.objects.filter(user=user, domain__domain_info__portfolio=portfolio).delete() diff --git a/src/registrar/templates/django/admin/domain_invitation_delete_confirmation.html b/src/registrar/templates/django/admin/domain_invitation_delete_confirmation.html new file mode 100644 index 000000000..215bf5ada --- /dev/null +++ b/src/registrar/templates/django/admin/domain_invitation_delete_confirmation.html @@ -0,0 +1,16 @@ +{% extends 'admin/delete_confirmation.html' %} +{% load i18n static %} + +{% block content_subtitle %} + + {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/django/admin/domain_invitation_delete_selected_confirmation.html b/src/registrar/templates/django/admin/domain_invitation_delete_selected_confirmation.html new file mode 100644 index 000000000..2e15347c1 --- /dev/null +++ b/src/registrar/templates/django/admin/domain_invitation_delete_selected_confirmation.html @@ -0,0 +1,16 @@ +{% extends 'admin/delete_selected_confirmation.html' %} +{% load i18n static %} + +{% block content_subtitle %} + + {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html new file mode 100644 index 000000000..171f4c3ea --- /dev/null +++ b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html @@ -0,0 +1,13 @@ +{% extends 'admin/delete_confirmation.html' %} +{% load i18n static %} + +{% block content_subtitle %} + + {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/django/admin/user_domain_role_delete_selected_confirmation.html b/src/registrar/templates/django/admin/user_domain_role_delete_selected_confirmation.html new file mode 100644 index 000000000..392d1aebc --- /dev/null +++ b/src/registrar/templates/django/admin/user_domain_role_delete_selected_confirmation.html @@ -0,0 +1,13 @@ +{% extends 'admin/delete_selected_confirmation.html' %} +{% load i18n static %} + +{% block content_subtitle %} + + {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index 1d34ef4e4..758c43366 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -49,11 +49,11 @@ {% if has_domain_renewal_flag and domain.is_expired and is_domain_manager %} This domain has expired, but it is still online. {% url 'domain-renewal' pk=domain.id as url %} - Renew to maintain access. + Renew to maintain access. {% elif has_domain_renewal_flag and domain.is_expiring and is_domain_manager %} This domain will expire soon. {% url 'domain-renewal' pk=domain.id as url %} - Renew to maintain access. + Renew to maintain access. {% elif has_domain_renewal_flag and domain.is_expiring and is_portfolio_user %} This domain will expire soon. Contact one of the listed domain managers to renew the domain. {% elif has_domain_renewal_flag and domain.is_expired and is_portfolio_user %} @@ -103,12 +103,12 @@ {% endif %} {% if portfolio %} - {% if has_any_domains_portfolio_permission and has_edit_suborganization_portfolio_permission %} + {% if has_any_domains_portfolio_permission and has_edit_portfolio_permission %} {% url 'domain-suborganization' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_edit_suborganization_portfolio_permission %} - {% elif has_any_domains_portfolio_permission and has_view_suborganization_portfolio_permission %} + {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_edit_portfolio_permission %} + {% elif has_any_domains_portfolio_permission and has_view_portfolio_permission %} {% url 'domain-suborganization' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_view_suborganization_portfolio_permission view_button=True %} + {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_view_portfolio_permission view_button=True %} {% endif %} {% else %} {% url 'domain-org-name-address' pk=domain.id as url %} diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html index 30e1be0e4..703c2358f 100644 --- a/src/registrar/templates/domain_renewal.html +++ b/src/registrar/templates/domain_renewal.html @@ -38,11 +38,11 @@ {{ block.super }}

Confirm the following information for accuracy

-

Review these details below. We +

Review the details below. We require that you maintain accurate information for the domain. The details you provide will only be used to support the administration of .gov and won't be made public.

-

If you would like to retire your domain instead, please +

If you would like to retire your domain instead, please contact us.

Required fields are marked with an asterisk (*).

@@ -98,7 +98,7 @@ {% if form.is_policy_acknowledged.errors %} {% for error in form.is_policy_acknowledged.errors %} @@ -131,7 +129,7 @@ name="submit_button" value="next" class="usa-button margin-top-3" - > Submit + > Submit and renew diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index ca3802720..5946b6859 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -61,7 +61,7 @@ {% if portfolio %} {% comment %} Only show this menu option if the user has the perms to do so {% endcomment %} - {% if has_any_domains_portfolio_permission and has_view_suborganization_portfolio_permission %} + {% if has_any_domains_portfolio_permission and has_view_portfolio_permission %} {% with url_name="domain-suborganization" %} {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %} {% endwith %} diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index e050690c8..1c3b8e588 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -39,7 +39,7 @@ please contact help@get.gov.

- {% if has_any_domains_portfolio_permission and has_edit_suborganization_portfolio_permission %} + {% if has_any_domains_portfolio_permission and has_edit_portfolio_permission %}
{% csrf_token %} {% input_with_errors form.sub_organization %} diff --git a/src/registrar/templates/emails/domain_manager_deleted_notification.txt b/src/registrar/templates/emails/domain_manager_deleted_notification.txt new file mode 100644 index 000000000..fbb1e47cc --- /dev/null +++ b/src/registrar/templates/emails/domain_manager_deleted_notification.txt @@ -0,0 +1,27 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +Hi,{% if domain_manager and domain_manager.first_name %} {{ domain_manager.first_name }}.{% endif %} + +A domain manager was removed from {{ domain.name }}. + +REMOVED BY: {{ removed_by.email }} +REMOVED ON: {{ date }} +MANAGER REMOVED: {{ manager_removed.email }} + +---------------------------------------------------------------- + +WHY DID YOU RECEIVE THIS EMAIL? +Youā€™re listed as a domain manager for {{ domain.name }}, so youā€™ll receive a notification whenever a domain manager is removed from that domain. +If you have questions or concerns, reach out to the person who removed the domain manager or reply to this email. + +THANK YOU +.Gov helps the public identify official, trusted information. Thank you for using a .gov domain. + +---------------------------------------------------------------- + +The .gov team +Contact us: +Learn about .gov + +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency +(CISA) +{% endautoescape %} diff --git a/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt b/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt new file mode 100644 index 000000000..c84a20f18 --- /dev/null +++ b/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt @@ -0,0 +1 @@ +A domain manager was removed from {{ domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/portfolio_admin_addition_notification.txt b/src/registrar/templates/emails/portfolio_admin_addition_notification.txt new file mode 100644 index 000000000..b8953aa67 --- /dev/null +++ b/src/registrar/templates/emails/portfolio_admin_addition_notification.txt @@ -0,0 +1,40 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +Hi,{% if portfolio_admin and portfolio_admin.first_name %} {{ portfolio_admin.first_name }}.{% endif %} + +An admin was invited to your .gov organization. + +ORGANIZATION: {{ portfolio.organization_name }} +INVITED BY: {{ requestor_email }} +INVITED ON: {{date}} +ADMIN INVITED: {{ invited_email_address }} + +---------------------------------------------------------------- + +NEXT STEPS +The person who received the invitation will become an admin once they log in to the +.gov registrar. They'll need to access the registrar using a Login.gov account that's +associated with the invited email address. + +If you need to cancel this invitation or remove the admin, you can do that by going to +the Members section for your organization . + + +WHY DID YOU RECEIVE THIS EMAIL? +Youā€™re listed as an admin for {{ portfolio.organization_name }}. That means you'll receive a notification +whenever a new admin is invited to that organization. + +If you have questions or concerns, reach out to the person who sent the invitation or reply to this email. + + +THANK YOU +.Gov helps the public identify official, trusted information. Thank you for using a .gov domain. + +---------------------------------------------------------------- + +The .gov team +Contact us: +Learn about .gov + +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency +(CISA) +{% endautoescape %} diff --git a/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt b/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt new file mode 100644 index 000000000..3d6b2a140 --- /dev/null +++ b/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt @@ -0,0 +1 @@ +An admin was invited to your .gov organization \ No newline at end of file diff --git a/src/registrar/templates/emails/portfolio_admin_removal_notification.txt b/src/registrar/templates/emails/portfolio_admin_removal_notification.txt new file mode 100644 index 000000000..6a536aa49 --- /dev/null +++ b/src/registrar/templates/emails/portfolio_admin_removal_notification.txt @@ -0,0 +1,33 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +Hi,{% if portfolio_admin and portfolio_admin.first_name %} {{ portfolio_admin.first_name }}.{% endif %} + +An admin was removed from your .gov organization. + +ORGANIZATION: {{ portfolio.organization_name }} +REMOVED BY: {{ requestor_email }} +REMOVED ON: {{date}} +ADMIN REMOVED: {{ removed_email_address }} + +You can view this update by going to the Members section for your .gov organization . + +---------------------------------------------------------------- + +WHY DID YOU RECEIVE THIS EMAIL? +Youā€™re listed as an admin for {{ portfolio.organization_name }}. That means you'll receive a notification +whenever an admin is removed from that organization. + +If you have questions or concerns, reach out to the person who removed the admin or reply to this email. + + +THANK YOU +.Gov helps the public identify official, trusted information. Thank you for using a .gov domain. + +---------------------------------------------------------------- + +The .gov team +Contact us: +Learn about .gov + +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency +(CISA) +{% endautoescape %} diff --git a/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt b/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt new file mode 100644 index 000000000..e250b17f8 --- /dev/null +++ b/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt @@ -0,0 +1 @@ +An admin was removed from your .gov organization \ No newline at end of file diff --git a/src/registrar/templates/includes/domain_dates.html b/src/registrar/templates/includes/domain_dates.html index b14c091d0..339d75c44 100644 --- a/src/registrar/templates/includes/domain_dates.html +++ b/src/registrar/templates/includes/domain_dates.html @@ -1,7 +1,7 @@ {% if domain.expiration_date or domain.created_at %}

{% if domain.expiration_date %} - Expires: + Date of expiration: {{ domain.expiration_date|date }} {% if domain.is_expired %} (expired){% endif %}
diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index b026a7a6b..8adc0929a 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -51,20 +51,7 @@

- {% if portfolio %} -
-
- - -
-
- {% endif %} + {% if portfolio %} @@ -78,6 +65,7 @@ id="domain-requests__usa-button--filter" aria-expanded="false" aria-controls="filter-status" + aria-label="Status, list 7 items" > Status
-
+

{% if num_expiring_domains == 1%} - One domain will expire soon. Go to "Manage" to renew the domain. Show expiring domain. + One domain will expire soon. Go to "Manage" to renew the domain. Show expiring domain. {% else%} - Multiple domains will expire soon. Go to "Manage" to renew the domains. Show expiring domains. + Multiple domains will expire soon. Go to "Manage" to renew the domains. Show expiring domains. {% endif %}

@@ -64,7 +64,7 @@ {% if user_domain_count and user_domain_count > 0 %}
- + Export as CSV @@ -76,14 +76,14 @@ {% if has_domain_renewal_flag and num_expiring_domains > 0 and not portfolio %} -
+
-
+

{% if num_expiring_domains == 1%} - One domain will expire soon. Go to "Manage" to renew the domain. Show expiring domain. + One domain will expire soon. Go to "Manage" to renew the domain. Show expiring domain. {% else%} - Multiple domains will expire soon. Go to "Manage" to renew the domains. Show expiring domains. + Multiple domains will expire soon. Go to "Manage" to renew the domains. Show expiring domains. {% endif %}

@@ -101,6 +101,7 @@ id="domains__usa-button--filter" aria-expanded="false" aria-controls="filter-status" + aria-label="Status, list 5 items" > Status
  • diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index cc308619a..6c1e6fd44 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -38,7 +38,7 @@
    • - + Export as CSV diff --git a/src/registrar/templates/includes/request_review_steps.html b/src/registrar/templates/includes/request_review_steps.html index 6151d01a8..f1b13f890 100644 --- a/src/registrar/templates/includes/request_review_steps.html +++ b/src/registrar/templates/includes/request_review_steps.html @@ -41,7 +41,7 @@ {% endif %} {% if step == Step.ORGANIZATION_CONTACT %} - {% if domain_request.organization_name %} + {% if domain_request.unlock_organization_contact %} {% with title=form_titles|get_item:step value=domain_request %} {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url address='true' %} {% endwith %} @@ -116,7 +116,7 @@ {% endif %} {% if step == Step.OTHER_CONTACTS %} - {% if domain_request.other_contacts.all %} + {% if domain_request.unlock_other_contacts %} {% with title=form_titles|get_item:step value=domain_request.other_contacts.all %} {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url contact='true' list='true' %} {% endwith %} diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 26e56fea7..d062a7b4e 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -113,10 +113,10 @@ {% endif %} {% endif %} - {% if value.invitations.all %} + {% if value.active_invitations.all %}

    Invited domain managers

      - {% for item in value.invitations.all %} + {% for item in value.active_invitations.all %}
    • {{ item.email }}
      • {% endfor %}
    diff --git a/src/registrar/templates/portfolio_organization.html b/src/registrar/templates/portfolio_organization.html index e6bd19ec2..2cc7e3f0f 100644 --- a/src/registrar/templates/portfolio_organization.html +++ b/src/registrar/templates/portfolio_organization.html @@ -28,7 +28,7 @@

    The name of your organization will be publicly listed as the domain registrant.

    - {% if has_edit_org_portfolio_permission %} + {% if has_edit_portfolio_permission %}

    Your organization name canā€™t be updated here. To suggest an update, email help@get.gov. diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 036e35a50..9447d211f 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -120,7 +120,7 @@ class TestFsmModelResource(TestCase): fsm_field_mock.save.assert_not_called() -class TestDomainInvitationAdmin(TestCase): +class TestDomainInvitationAdmin(WebTest): """Tests for the DomainInvitationAdmin class as super user Notes: @@ -128,15 +128,27 @@ class TestDomainInvitationAdmin(TestCase): tests have available superuser, client, and admin """ - def setUp(self): + # csrf checks do not work with WebTest. + # We disable them here. TODO for another ticket. + csrf_checks = False + + @classmethod + def setUpClass(self): + super().setUpClass() + self.site = AdminSite() self.factory = RequestFactory() - self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite()) self.superuser = create_superuser() + + def setUp(self): + super().setUp() + self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite()) self.domain = Domain.objects.create(name="example.com") self.portfolio = Portfolio.objects.create(organization_name="new portfolio", creator=self.superuser) DomainInformation.objects.create(domain=self.domain, portfolio=self.portfolio, creator=self.superuser) """Create a client object""" self.client = Client(HTTP_HOST="localhost:8080") + self.client.force_login(self.superuser) + self.app.set_user(self.superuser.username) def tearDown(self): """Delete all DomainInvitation objects""" @@ -254,6 +266,7 @@ class TestDomainInvitationAdmin(TestCase): email="test@example.com", requestor=self.superuser, portfolio=self.portfolio, + is_admin_invitation=False, ) # Assert success message @@ -504,6 +517,7 @@ class TestDomainInvitationAdmin(TestCase): email="test@example.com", requestor=self.superuser, portfolio=self.portfolio, + is_admin_invitation=False, ) # Assert retrieve on domain invite only was called @@ -567,6 +581,7 @@ class TestDomainInvitationAdmin(TestCase): email="test@example.com", requestor=self.superuser, portfolio=self.portfolio, + is_admin_invitation=False, ) # Assert retrieve on domain invite only was called @@ -693,6 +708,7 @@ class TestDomainInvitationAdmin(TestCase): email="nonexistent@example.com", requestor=self.superuser, portfolio=self.portfolio, + is_admin_invitation=False, ) # Assert retrieve was not called @@ -918,6 +934,7 @@ class TestDomainInvitationAdmin(TestCase): email="nonexistent@example.com", requestor=self.superuser, portfolio=self.portfolio, + is_admin_invitation=False, ) # Assert retrieve on domain invite only was called @@ -979,6 +996,7 @@ class TestDomainInvitationAdmin(TestCase): email="nonexistent@example.com", requestor=self.superuser, portfolio=self.portfolio, + is_admin_invitation=False, ) # Assert retrieve on domain invite only was called @@ -1065,6 +1083,50 @@ class TestDomainInvitationAdmin(TestCase): self.assertEqual(DomainInvitation.objects.count(), 0) self.assertEqual(PortfolioInvitation.objects.count(), 1) + @less_console_noise_decorator + def test_custom_delete_confirmation_page(self): + """Tests if custom alerts display on Domain Invitation delete page""" + self.client.force_login(self.superuser) + self.app.set_user(self.superuser.username) + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_invitation, _ = DomainInvitation.objects.get_or_create(domain=domain) + + domain_invitation_change_page = self.app.get( + reverse("admin:registrar_domaininvitation_change", args=[domain_invitation.pk]) + ) + + self.assertContains(domain_invitation_change_page, "domain-invitation-test.gov") + # click the "Delete" link + confirmation_page = domain_invitation_change_page.click("Delete", index=0) + + custom_alert_content = "If you cancel the domain invitation here" + self.assertContains(confirmation_page, custom_alert_content) + + @less_console_noise_decorator + def test_custom_selected_delete_confirmation_page(self): + """Tests if custom alerts display on Domain Invitation selected delete page from Domain Invitation table""" + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_invitation, _ = DomainInvitation.objects.get_or_create(domain=domain) + + # Get the index. The post expects the index to be encoded as a string + index = f"{domain_invitation.id}" + + test_helper = GenericTestHelper( + factory=self.factory, + user=self.superuser, + admin=self.admin, + url=reverse("admin:registrar_domaininvitation_changelist"), + model=Domain, + client=self.client, + ) + + # Simulate selecting a single record, then clicking "Delete selected domains" + response = test_helper.get_table_delete_confirmation_page("0", index) + + # Check for custom alert message + custom_alert_content = "If you cancel the domain invitation here" + self.assertContains(response, custom_alert_content) + class TestUserPortfolioPermissionAdmin(TestCase): """Tests for the PortfolioInivtationAdmin class""" @@ -1204,7 +1266,7 @@ class TestPortfolioInvitationAdmin(TestCase): @less_console_noise_decorator @patch("registrar.admin.send_portfolio_invitation_email") - @patch("django.contrib.messages.success") # Mock the `messages.warning` call + @patch("django.contrib.messages.success") # Mock the `messages.success` call def test_save_sends_email(self, mock_messages_success, mock_send_email): """On save_model, an email is sent if an invitation already exists.""" @@ -1427,7 +1489,7 @@ class TestPortfolioInvitationAdmin(TestCase): @less_console_noise_decorator @patch("registrar.admin.send_portfolio_invitation_email") - @patch("django.contrib.messages.warning") # Mock the `messages.error` call + @patch("django.contrib.messages.error") # Mock the `messages.error` call def test_save_exception_generic_error(self, mock_messages_error, mock_send_email): """Handle generic exceptions correctly during portfolio invitation.""" self.client.force_login(self.superuser) @@ -1455,6 +1517,94 @@ class TestPortfolioInvitationAdmin(TestCase): # Assert that messages.error was called with the correct message mock_messages_error.assert_called_once_with(request, "Could not send email invitation.") + @less_console_noise_decorator + @patch("registrar.admin.send_portfolio_admin_addition_emails") + def test_save_existing_sends_email_notification(self, mock_send_email): + """On save_model to an existing invitation, an email is set to notify existing + admins, if the invitation changes from member to admin.""" + + # Create an instance of the admin class + admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) + + # Mock the response value of the email send + mock_send_email.return_value = True + + # Create and save a PortfolioInvitation instance + portfolio_invitation = PortfolioInvitation.objects.create( + email="james.gordon@gotham.gov", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], # Initially NOT an admin + status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, # Must be "INVITED" + ) + + # Create a request object + request = self.factory.post(f"/admin/registrar/PortfolioInvitation/{portfolio_invitation.pk}/change/") + request.user = self.superuser + + # Change roles from MEMBER to ADMIN + portfolio_invitation.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + + # Call the save_model method + admin_instance.save_model(request, portfolio_invitation, None, True) + + # Assert that send_portfolio_admin_addition_emails is called + mock_send_email.assert_called_once() + + # Get the arguments passed to send_portfolio_admin_addition_emails + _, called_kwargs = mock_send_email.call_args + + # Assert the email content + self.assertEqual(called_kwargs["email"], "james.gordon@gotham.gov") + self.assertEqual(called_kwargs["requestor"], self.superuser) + self.assertEqual(called_kwargs["portfolio"], self.portfolio) + + @less_console_noise_decorator + @patch("registrar.admin.send_portfolio_admin_addition_emails") + @patch("django.contrib.messages.warning") # Mock the `messages.warning` call + def test_save_existing_email_notification_warning(self, mock_messages_warning, mock_send_email): + """On save_model for an existing invitation, a warning is displayed if method to + send email to notify admins returns False.""" + + # Create an instance of the admin class + admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) + + # Mock the response value of the email send + mock_send_email.return_value = False + + # Create and save a PortfolioInvitation instance + portfolio_invitation = PortfolioInvitation.objects.create( + email="james.gordon@gotham.gov", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], # Initially NOT an admin + status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, # Must be "INVITED" + ) + + # Create a request object + request = self.factory.post(f"/admin/registrar/PortfolioInvitation/{portfolio_invitation.pk}/change/") + request.user = self.superuser + + # Change roles from MEMBER to ADMIN + portfolio_invitation.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + + # Call the save_model method + admin_instance.save_model(request, portfolio_invitation, None, True) + + # Assert that send_portfolio_admin_addition_emails is called + mock_send_email.assert_called_once() + + # Get the arguments passed to send_portfolio_admin_addition_emails + _, called_kwargs = mock_send_email.call_args + + # Assert the email content + self.assertEqual(called_kwargs["email"], "james.gordon@gotham.gov") + self.assertEqual(called_kwargs["requestor"], self.superuser) + self.assertEqual(called_kwargs["portfolio"], self.portfolio) + + # Assert that messages.error was called with the correct message + mock_messages_warning.assert_called_once_with( + request, "Could not send email notification to existing organization admins." + ) + class TestHostAdmin(TestCase): """Tests for the HostAdmin class as super user @@ -1922,7 +2072,7 @@ class TestDomainInformationAdmin(TestCase): self.test_helper.assert_table_sorted("-4", ("-creator__first_name", "-creator__last_name")) -class TestUserDomainRoleAdmin(TestCase): +class TestUserDomainRoleAdmin(WebTest): """Tests for the UserDomainRoleAdmin class as super user Notes: @@ -1949,6 +2099,8 @@ class TestUserDomainRoleAdmin(TestCase): """Setup environment for a mock admin user""" super().setUp() self.client = Client(HTTP_HOST="localhost:8080") + self.client.force_login(self.superuser) + self.app.set_user(self.superuser.username) def tearDown(self): """Delete all Users, Domains, and UserDomainRoles""" @@ -2111,6 +2263,48 @@ class TestUserDomainRoleAdmin(TestCase): # We only need to check for the end of the HTML string self.assertContains(response, "Joe Jones AntarcticPolarBears@example.com", count=1) + @less_console_noise_decorator + def test_custom_delete_confirmation_page(self): + """Tests if custom alerts display on User Domain Role delete page""" + domain, _ = Domain.objects.get_or_create(name="user-domain-role-test.gov", state=Domain.State.READY) + domain_role, _ = UserDomainRole.objects.get_or_create(domain=domain, user=self.superuser) + + domain_invitation_change_page = self.app.get( + reverse("admin:registrar_userdomainrole_change", args=[domain_role.pk]) + ) + + self.assertContains(domain_invitation_change_page, "user-domain-role-test.gov") + # click the "Delete" link + confirmation_page = domain_invitation_change_page.click("Delete", index=0) + + custom_alert_content = "If you remove someone from a domain here" + self.assertContains(confirmation_page, custom_alert_content) + + @less_console_noise_decorator + def test_custom_selected_delete_confirmation_page(self): + """Tests if custom alerts display on selected delete page from User Domain Roles table""" + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_role, _ = UserDomainRole.objects.get_or_create(domain=domain, user=self.superuser) + + # Get the index. The post expects the index to be encoded as a string + index = f"{domain_role.id}" + + test_helper = GenericTestHelper( + factory=self.factory, + user=self.superuser, + admin=self.admin, + url=reverse("admin:registrar_userdomainrole_changelist"), + model=Domain, + client=self.client, + ) + + # Simulate selecting a single record, then clicking "Delete selected domains" + response = test_helper.get_table_delete_confirmation_page("0", index) + + # Check for custom alert message + custom_alert_content = "If you remove someone from a domain here" + self.assertContains(response, custom_alert_content) + class TestListHeaderAdmin(TestCase): """Tests for the ListHeaderAdmin class as super user diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py index b9fef1bf8..77a8c402f 100644 --- a/src/registrar/tests/test_email_invitations.py +++ b/src/registrar/tests/test_email_invitations.py @@ -1,10 +1,25 @@ import unittest from unittest.mock import patch, MagicMock from datetime import date +from registrar.models.domain import Domain +from registrar.models.portfolio import Portfolio +from registrar.models.user import User +from registrar.models.user_domain_role import UserDomainRole +from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.utility.email import EmailSendingError -from registrar.utility.email_invitations import send_domain_invitation_email +from registrar.utility.email_invitations import ( + _send_portfolio_admin_addition_emails_to_portfolio_admins, + _send_portfolio_admin_removal_emails_to_portfolio_admins, + send_domain_invitation_email, + send_emails_to_domain_managers, + send_portfolio_admin_addition_emails, + send_portfolio_admin_removal_emails, + send_portfolio_invitation_email, +) from api.tests.common import less_console_noise_decorator +from registrar.utility.errors import MissingEmailError class DomainInvitationEmail(unittest.TestCase): @@ -13,9 +28,9 @@ class DomainInvitationEmail(unittest.TestCase): @patch("registrar.utility.email_invitations.send_templated_email") @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") @patch("registrar.utility.email_invitations._validate_invitation") - @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations._get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") - @patch("registrar.utility.email_invitations.normalize_domains") + @patch("registrar.utility.email_invitations._normalize_domains") def test_send_domain_invitation_email( self, mock_normalize_domains, @@ -55,7 +70,7 @@ class DomainInvitationEmail(unittest.TestCase): # Assertions mock_normalize_domains.assert_called_once_with(mock_domain) - mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_get_requestor_email.assert_called_once_with(mock_requestor, domains=[mock_domain]) mock_validate_invitation.assert_called_once_with( email, None, [mock_domain], mock_requestor, is_member_of_different_org ) @@ -78,9 +93,9 @@ class DomainInvitationEmail(unittest.TestCase): @patch("registrar.utility.email_invitations.send_templated_email") @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") @patch("registrar.utility.email_invitations._validate_invitation") - @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations._get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") - @patch("registrar.utility.email_invitations.normalize_domains") + @patch("registrar.utility.email_invitations._normalize_domains") def test_send_domain_invitation_email_multiple_domains( self, mock_normalize_domains, @@ -134,7 +149,7 @@ class DomainInvitationEmail(unittest.TestCase): # Assertions mock_normalize_domains.assert_called_once_with([mock_domain1, mock_domain2]) - mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain1, mock_domain2]) + mock_get_requestor_email.assert_called_once_with(mock_requestor, domains=[mock_domain1, mock_domain2]) mock_validate_invitation.assert_called_once_with( email, None, [mock_domain1, mock_domain2], mock_requestor, is_member_of_different_org ) @@ -194,7 +209,7 @@ class DomainInvitationEmail(unittest.TestCase): mock_validate_invitation.assert_called_once() @less_console_noise_decorator - @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations._get_requestor_email") def test_send_domain_invitation_email_raises_get_requestor_email_exception(self, mock_get_requestor_email): """Test sending domain invitation email for one domain and assert exception when get_requestor_email fails. @@ -214,9 +229,9 @@ class DomainInvitationEmail(unittest.TestCase): @less_console_noise_decorator @patch("registrar.utility.email_invitations._validate_invitation") - @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations._get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") - @patch("registrar.utility.email_invitations.normalize_domains") + @patch("registrar.utility.email_invitations._normalize_domains") def test_send_domain_invitation_email_raises_sending_email_exception( self, mock_normalize_domains, @@ -255,7 +270,7 @@ class DomainInvitationEmail(unittest.TestCase): # Assertions mock_normalize_domains.assert_called_once_with(mock_domain) - mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_get_requestor_email.assert_called_once_with(mock_requestor, domains=[mock_domain]) mock_validate_invitation.assert_called_once_with( email, None, [mock_domain], mock_requestor, is_member_of_different_org ) @@ -264,9 +279,9 @@ class DomainInvitationEmail(unittest.TestCase): @less_console_noise_decorator @patch("registrar.utility.email_invitations.send_emails_to_domain_managers") @patch("registrar.utility.email_invitations._validate_invitation") - @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations._get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") - @patch("registrar.utility.email_invitations.normalize_domains") + @patch("registrar.utility.email_invitations._normalize_domains") def test_send_domain_invitation_email_manager_emails_send_mail_exception( self, mock_normalize_domains, @@ -290,22 +305,586 @@ class DomainInvitationEmail(unittest.TestCase): email = "invitee@example.com" is_member_of_different_org = False - mock_send_domain_manager_emails.side_effect = EmailSendingError("Error sending email") + # Change the return value to False for mock_send_domain_manager_emails + mock_send_domain_manager_emails.return_value = False - # Call and assert exception - with self.assertRaises(EmailSendingError) as context: - send_domain_invitation_email( - email=email, - requestor=mock_requestor, - domains=mock_domain, - is_member_of_different_org=is_member_of_different_org, - ) + # Call and assert that send_domain_invitation_email returns False + result = send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=mock_domain, + is_member_of_different_org=is_member_of_different_org, + ) # Assertions mock_normalize_domains.assert_called_once_with(mock_domain) - mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_get_requestor_email.assert_called_once_with(mock_requestor, domains=[mock_domain]) mock_validate_invitation.assert_called_once_with( email, None, [mock_domain], mock_requestor, is_member_of_different_org ) mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None) - self.assertEqual(str(context.exception), "Error sending email") + + # Assert that the result is False + self.assertFalse(result) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.models.UserDomainRole.objects.filter") + def test_send_emails_to_domain_managers_all_emails_sent_successfully(self, mock_filter, mock_send_templated_email): + """Test when all emails are sent successfully.""" + + # Setup mocks + mock_domain = MagicMock(spec=Domain) + mock_requestor_email = "requestor@example.com" + mock_email = "invitee@example.com" + + # Create mock user and UserDomainRole + mock_user = MagicMock(spec=User) + mock_user.email = "manager@example.com" + mock_user_domain_role = MagicMock(spec=UserDomainRole, user=mock_user) + + # Mock the filter method to return a list of mock UserDomainRole objects + mock_filter.return_value = [mock_user_domain_role] + + # Mock successful email sending + mock_send_templated_email.return_value = None # No exception means success + + # Call function + result = send_emails_to_domain_managers(mock_email, mock_requestor_email, mock_domain) + + # Assertions + self.assertTrue(result) # All emails should be successfully sent + mock_send_templated_email.assert_called_once_with( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address="manager@example.com", + context={ + "domain": mock_domain, + "requestor_email": mock_requestor_email, + "invited_email_address": mock_email, + "domain_manager": mock_user, + "date": date.today(), + }, + ) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.models.UserDomainRole.objects.filter") + def test_send_emails_to_domain_managers_email_send_fails(self, mock_filter, mock_send_templated_email): + """Test when sending an email fails (raises EmailSendingError).""" + + # Setup mocks + mock_domain = MagicMock(spec=Domain) + mock_requestor_email = "requestor@example.com" + mock_email = "invitee@example.com" + + # Create mock user and UserDomainRole + mock_user = MagicMock(spec=User) + mock_user.email = "manager@example.com" + mock_user_domain_role = MagicMock(spec=UserDomainRole, user=mock_user) + + # Mock the filter method to return a list of mock UserDomainRole objects + mock_filter.return_value = [mock_user_domain_role] + + # Mock sending email to raise an EmailSendingError + mock_send_templated_email.side_effect = EmailSendingError("Email sending failed") + + # Call function + result = send_emails_to_domain_managers(mock_email, mock_requestor_email, mock_domain) + + # Assertions + self.assertFalse(result) # The result should be False as email sending failed + mock_send_templated_email.assert_called_once_with( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address="manager@example.com", + context={ + "domain": mock_domain, + "requestor_email": mock_requestor_email, + "invited_email_address": mock_email, + "domain_manager": mock_user, + "date": date.today(), + }, + ) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.models.UserDomainRole.objects.filter") + def test_send_emails_to_domain_managers_no_domain_managers(self, mock_filter, mock_send_templated_email): + """Test when there are no domain managers.""" + + # Setup mocks + mock_domain = MagicMock(spec=Domain) + mock_requestor_email = "requestor@example.com" + mock_email = "invitee@example.com" + + # Mock no domain managers (empty UserDomainRole queryset) + mock_filter.return_value = [] + + # Call function + result = send_emails_to_domain_managers(mock_email, mock_requestor_email, mock_domain) + + # Assertions + self.assertTrue(result) # No emails to send, so it should return True + mock_send_templated_email.assert_not_called() # No emails should be sent + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.models.UserDomainRole.objects.filter") + def test_send_emails_to_domain_managers_some_emails_fail(self, mock_filter, mock_send_templated_email): + """Test when some emails fail to send.""" + + # Setup mocks + mock_domain = MagicMock(spec=Domain) + mock_requestor_email = "requestor@example.com" + mock_email = "invitee@example.com" + + # Create mock users and UserDomainRoles + mock_user_1 = MagicMock(spec=User) + mock_user_1.email = "manager1@example.com" + mock_user_2 = MagicMock(spec=User) + mock_user_2.email = "manager2@example.com" + + mock_user_domain_role_1 = MagicMock(spec=UserDomainRole, user=mock_user_1) + mock_user_domain_role_2 = MagicMock(spec=UserDomainRole, user=mock_user_2) + mock_filter.return_value = [mock_user_domain_role_1, mock_user_domain_role_2] + + # Mock first email success and second email failure + mock_send_templated_email.side_effect = [None, EmailSendingError("Failed to send email")] + + # Call function + result = send_emails_to_domain_managers(mock_email, mock_requestor_email, mock_domain) + + # Assertions + self.assertFalse(result) # One email failed, so result should be False + mock_send_templated_email.assert_any_call( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address="manager1@example.com", + context={ + "domain": mock_domain, + "requestor_email": mock_requestor_email, + "invited_email_address": mock_email, + "domain_manager": mock_user_1, + "date": date.today(), + }, + ) + mock_send_templated_email.assert_any_call( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address="manager2@example.com", + context={ + "domain": mock_domain, + "requestor_email": mock_requestor_email, + "invited_email_address": mock_email, + "domain_manager": mock_user_2, + "date": date.today(), + }, + ) + + +class PortfolioInvitationEmailTests(unittest.TestCase): + + def setUp(self): + """Setup common test data for all test cases""" + self.email = "invitee@example.com" + self.requestor = MagicMock(name="User") + self.requestor.email = "requestor@example.com" + self.portfolio = MagicMock(name="Portfolio") + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + def test_send_portfolio_invitation_email_success(self, mock_send_templated_email): + """Test successful email sending""" + is_admin_invitation = False + + result = send_portfolio_invitation_email(self.email, self.requestor, self.portfolio, is_admin_invitation) + + self.assertTrue(result) + mock_send_templated_email.assert_called_once() + + @less_console_noise_decorator + @patch( + "registrar.utility.email_invitations.send_templated_email", + side_effect=EmailSendingError("Failed to send email"), + ) + def test_send_portfolio_invitation_email_failure(self, mock_send_templated_email): + """Test failure when sending email""" + is_admin_invitation = False + + with self.assertRaises(EmailSendingError) as context: + send_portfolio_invitation_email(self.email, self.requestor, self.portfolio, is_admin_invitation) + + self.assertIn("Could not sent email invitation to", str(context.exception)) + + @less_console_noise_decorator + @patch( + "registrar.utility.email_invitations._get_requestor_email", + side_effect=MissingEmailError("Requestor has no email"), + ) + @less_console_noise_decorator + def test_send_portfolio_invitation_email_missing_requestor_email(self, mock_get_email): + """Test when requestor has no email""" + is_admin_invitation = False + + with self.assertRaises(MissingEmailError) as context: + send_portfolio_invitation_email(self.email, self.requestor, self.portfolio, is_admin_invitation) + + self.assertIn( + "Can't send invitation email. No email is associated with your user account.", str(context.exception) + ) + + @less_console_noise_decorator + @patch( + "registrar.utility.email_invitations._send_portfolio_admin_addition_emails_to_portfolio_admins", + return_value=False, + ) + @patch("registrar.utility.email_invitations.send_templated_email") + def test_send_portfolio_invitation_email_admin_invitation(self, mock_send_templated_email, mock_admin_email): + """Test admin invitation email logic""" + is_admin_invitation = True + + result = send_portfolio_invitation_email(self.email, self.requestor, self.portfolio, is_admin_invitation) + + self.assertFalse(result) # Admin email sending failed + mock_send_templated_email.assert_called_once() + mock_admin_email.assert_called_once() + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations._get_requestor_email") + @patch("registrar.utility.email_invitations._send_portfolio_admin_addition_emails_to_portfolio_admins") + def test_send_email_success(self, mock_send_admin_emails, mock_get_requestor_email): + """Test successful sending of admin addition emails.""" + mock_get_requestor_email.return_value = "requestor@example.com" + mock_send_admin_emails.return_value = True + + result = send_portfolio_admin_addition_emails(self.email, self.requestor, self.portfolio) + + mock_get_requestor_email.assert_called_once_with(self.requestor, portfolio=self.portfolio) + mock_send_admin_emails.assert_called_once_with(self.email, "requestor@example.com", self.portfolio) + self.assertTrue(result) + + @less_console_noise_decorator + @patch( + "registrar.utility.email_invitations._get_requestor_email", + side_effect=MissingEmailError("Requestor email missing"), + ) + def test_missing_requestor_email_raises_exception(self, mock_get_requestor_email): + """Test exception raised if requestor email is missing.""" + with self.assertRaises(MissingEmailError): + send_portfolio_admin_addition_emails(self.email, self.requestor, self.portfolio) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations._get_requestor_email") + @patch("registrar.utility.email_invitations._send_portfolio_admin_addition_emails_to_portfolio_admins") + def test_send_email_failure(self, mock_send_admin_emails, mock_get_requestor_email): + """Test handling of failure in sending admin addition emails.""" + mock_get_requestor_email.return_value = "requestor@example.com" + mock_send_admin_emails.return_value = False # Simulate failure + + result = send_portfolio_admin_addition_emails(self.email, self.requestor, self.portfolio) + + self.assertFalse(result) + mock_get_requestor_email.assert_called_once_with(self.requestor, portfolio=self.portfolio) + mock_send_admin_emails.assert_called_once_with(self.email, "requestor@example.com", self.portfolio) + + +class SendPortfolioAdminAdditionEmailsTests(unittest.TestCase): + """Unit tests for _send_portfolio_admin_addition_emails_to_portfolio_admins function.""" + + def setUp(self): + """Set up test data.""" + self.email = "new.admin@example.com" + self.requestor_email = "requestor@example.com" + self.portfolio = MagicMock(spec=Portfolio) + self.portfolio.organization_name = "Test Organization" + + # Mock portfolio admin users + self.admin_user1 = MagicMock(spec=User) + self.admin_user1.email = "admin1@example.com" + + self.admin_user2 = MagicMock(spec=User) + self.admin_user2.email = "admin2@example.com" + + self.portfolio_admin1 = MagicMock(spec=UserPortfolioPermission) + self.portfolio_admin1.user = self.admin_user1 + self.portfolio_admin1.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + + self.portfolio_admin2 = MagicMock(spec=UserPortfolioPermission) + self.portfolio_admin2.user = self.admin_user2 + self.portfolio_admin2.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.utility.email_invitations.UserPortfolioPermission.objects.filter") + def test_send_email_success(self, mock_filter, mock_send_templated_email): + """Test successful sending of admin addition emails.""" + mock_filter.return_value.exclude.return_value = [self.portfolio_admin1, self.portfolio_admin2] + mock_send_templated_email.return_value = None # No exception means success + + result = _send_portfolio_admin_addition_emails_to_portfolio_admins( + self.email, self.requestor_email, self.portfolio + ) + + mock_filter.assert_called_once_with( + portfolio=self.portfolio, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + mock_send_templated_email.assert_any_call( + "emails/portfolio_admin_addition_notification.txt", + "emails/portfolio_admin_addition_notification_subject.txt", + to_address=self.admin_user1.email, + context={ + "portfolio": self.portfolio, + "requestor_email": self.requestor_email, + "invited_email_address": self.email, + "portfolio_admin": self.admin_user1, + "date": date.today(), + }, + ) + mock_send_templated_email.assert_any_call( + "emails/portfolio_admin_addition_notification.txt", + "emails/portfolio_admin_addition_notification_subject.txt", + to_address=self.admin_user2.email, + context={ + "portfolio": self.portfolio, + "requestor_email": self.requestor_email, + "invited_email_address": self.email, + "portfolio_admin": self.admin_user2, + "date": date.today(), + }, + ) + self.assertTrue(result) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email", side_effect=EmailSendingError) + @patch("registrar.utility.email_invitations.UserPortfolioPermission.objects.filter") + def test_send_email_failure(self, mock_filter, mock_send_templated_email): + """Test handling of failure in sending admin addition emails.""" + mock_filter.return_value.exclude.return_value = [self.portfolio_admin1, self.portfolio_admin2] + + result = _send_portfolio_admin_addition_emails_to_portfolio_admins( + self.email, self.requestor_email, self.portfolio + ) + + self.assertFalse(result) + mock_filter.assert_called_once_with( + portfolio=self.portfolio, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + mock_send_templated_email.assert_any_call( + "emails/portfolio_admin_addition_notification.txt", + "emails/portfolio_admin_addition_notification_subject.txt", + to_address=self.admin_user1.email, + context={ + "portfolio": self.portfolio, + "requestor_email": self.requestor_email, + "invited_email_address": self.email, + "portfolio_admin": self.admin_user1, + "date": date.today(), + }, + ) + mock_send_templated_email.assert_any_call( + "emails/portfolio_admin_addition_notification.txt", + "emails/portfolio_admin_addition_notification_subject.txt", + to_address=self.admin_user2.email, + context={ + "portfolio": self.portfolio, + "requestor_email": self.requestor_email, + "invited_email_address": self.email, + "portfolio_admin": self.admin_user2, + "date": date.today(), + }, + ) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.UserPortfolioPermission.objects.filter") + def test_no_admins_to_notify(self, mock_filter): + """Test case where there are no portfolio admins to notify.""" + mock_filter.return_value.exclude.return_value = [] # No admins + + result = _send_portfolio_admin_addition_emails_to_portfolio_admins( + self.email, self.requestor_email, self.portfolio + ) + + self.assertTrue(result) # No emails sent, but also no failures + mock_filter.assert_called_once_with( + portfolio=self.portfolio, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + +class SendPortfolioAdminRemovalEmailsToAdminsTests(unittest.TestCase): + """Unit tests for _send_portfolio_admin_removal_emails_to_portfolio_admins function.""" + + def setUp(self): + """Set up test data.""" + self.email = "removed.admin@example.com" + self.requestor_email = "requestor@example.com" + self.portfolio = MagicMock(spec=Portfolio) + self.portfolio.organization_name = "Test Organization" + + # Mock portfolio admin users + self.admin_user1 = MagicMock(spec=User) + self.admin_user1.email = "admin1@example.com" + + self.admin_user2 = MagicMock(spec=User) + self.admin_user2.email = "admin2@example.com" + + self.portfolio_admin1 = MagicMock(spec=UserPortfolioPermission) + self.portfolio_admin1.user = self.admin_user1 + self.portfolio_admin1.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + + self.portfolio_admin2 = MagicMock(spec=UserPortfolioPermission) + self.portfolio_admin2.user = self.admin_user2 + self.portfolio_admin2.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.utility.email_invitations.UserPortfolioPermission.objects.filter") + def test_send_email_success(self, mock_filter, mock_send_templated_email): + """Test successful sending of admin removal emails.""" + mock_filter.return_value.exclude.return_value = [self.portfolio_admin1, self.portfolio_admin2] + mock_send_templated_email.return_value = None # No exception means success + + result = _send_portfolio_admin_removal_emails_to_portfolio_admins( + self.email, self.requestor_email, self.portfolio + ) + + mock_filter.assert_called_once_with( + portfolio=self.portfolio, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + mock_send_templated_email.assert_any_call( + "emails/portfolio_admin_removal_notification.txt", + "emails/portfolio_admin_removal_notification_subject.txt", + to_address=self.admin_user1.email, + context={ + "portfolio": self.portfolio, + "requestor_email": self.requestor_email, + "removed_email_address": self.email, + "portfolio_admin": self.admin_user1, + "date": date.today(), + }, + ) + mock_send_templated_email.assert_any_call( + "emails/portfolio_admin_removal_notification.txt", + "emails/portfolio_admin_removal_notification_subject.txt", + to_address=self.admin_user2.email, + context={ + "portfolio": self.portfolio, + "requestor_email": self.requestor_email, + "removed_email_address": self.email, + "portfolio_admin": self.admin_user2, + "date": date.today(), + }, + ) + self.assertTrue(result) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email", side_effect=EmailSendingError) + @patch("registrar.utility.email_invitations.UserPortfolioPermission.objects.filter") + def test_send_email_failure(self, mock_filter, mock_send_templated_email): + """Test handling of failure in sending admin removal emails.""" + mock_filter.return_value.exclude.return_value = [self.portfolio_admin1, self.portfolio_admin2] + + result = _send_portfolio_admin_removal_emails_to_portfolio_admins( + self.email, self.requestor_email, self.portfolio + ) + + self.assertFalse(result) + mock_filter.assert_called_once_with( + portfolio=self.portfolio, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + mock_send_templated_email.assert_any_call( + "emails/portfolio_admin_removal_notification.txt", + "emails/portfolio_admin_removal_notification_subject.txt", + to_address=self.admin_user1.email, + context={ + "portfolio": self.portfolio, + "requestor_email": self.requestor_email, + "removed_email_address": self.email, + "portfolio_admin": self.admin_user1, + "date": date.today(), + }, + ) + mock_send_templated_email.assert_any_call( + "emails/portfolio_admin_removal_notification.txt", + "emails/portfolio_admin_removal_notification_subject.txt", + to_address=self.admin_user2.email, + context={ + "portfolio": self.portfolio, + "requestor_email": self.requestor_email, + "removed_email_address": self.email, + "portfolio_admin": self.admin_user2, + "date": date.today(), + }, + ) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.UserPortfolioPermission.objects.filter") + def test_no_admins_to_notify(self, mock_filter): + """Test case where there are no portfolio admins to notify.""" + mock_filter.return_value.exclude.return_value = [] # No admins + + result = _send_portfolio_admin_removal_emails_to_portfolio_admins( + self.email, self.requestor_email, self.portfolio + ) + + self.assertTrue(result) # No emails sent, but also no failures + mock_filter.assert_called_once_with( + portfolio=self.portfolio, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + +class SendPortfolioAdminRemovalEmailsTests(unittest.TestCase): + """Unit tests for send_portfolio_admin_removal_emails function.""" + + def setUp(self): + """Set up test data.""" + self.email = "removed.admin@example.com" + self.requestor = MagicMock(spec=User) + self.requestor.email = "requestor@example.com" + self.portfolio = MagicMock(spec=Portfolio) + self.portfolio.organization_name = "Test Organization" + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations._get_requestor_email") + @patch("registrar.utility.email_invitations._send_portfolio_admin_removal_emails_to_portfolio_admins") + def test_send_email_success(self, mock_send_removal_emails, mock_get_requestor_email): + """Test successful execution of send_portfolio_admin_removal_emails.""" + mock_get_requestor_email.return_value = self.requestor.email + mock_send_removal_emails.return_value = True # Simulating success + + result = send_portfolio_admin_removal_emails(self.email, self.requestor, self.portfolio) + + mock_get_requestor_email.assert_called_once_with(self.requestor, portfolio=self.portfolio) + mock_send_removal_emails.assert_called_once_with(self.email, self.requestor.email, self.portfolio) + self.assertTrue(result) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations._get_requestor_email", side_effect=MissingEmailError("No email found")) + @patch("registrar.utility.email_invitations._send_portfolio_admin_removal_emails_to_portfolio_admins") + def test_missing_email_error(self, mock_send_removal_emails, mock_get_requestor_email): + """Test handling of MissingEmailError when requestor has no email.""" + with self.assertRaises(MissingEmailError) as context: + send_portfolio_admin_removal_emails(self.email, self.requestor, self.portfolio) + + mock_get_requestor_email.assert_called_once_with(self.requestor, portfolio=self.portfolio) + mock_send_removal_emails.assert_not_called() # Should not proceed if email retrieval fails + self.assertEqual( + str(context.exception), "Can't send invitation email. No email is associated with your user account." + ) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations._get_requestor_email") + @patch( + "registrar.utility.email_invitations._send_portfolio_admin_removal_emails_to_portfolio_admins", + return_value=False, + ) + def test_send_email_failure(self, mock_send_removal_emails, mock_get_requestor_email): + """Test handling of failure when admin removal emails fail to send.""" + mock_get_requestor_email.return_value = self.requestor.email + mock_send_removal_emails.return_value = False # Simulating failure + + result = send_portfolio_admin_removal_emails(self.email, self.requestor, self.portfolio) + + mock_get_requestor_email.assert_called_once_with(self.requestor, portfolio=self.portfolio) + mock_send_removal_emails.assert_called_once_with(self.email, self.requestor.email, self.portfolio) + self.assertFalse(result) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 82e3b40bb..35a7d76ac 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -24,6 +24,7 @@ from registrar.forms.portfolio import ( PortfolioMemberForm, PortfolioNewMemberForm, ) +from waffle.models import get_waffle_flag_model from registrar.models.portfolio import Portfolio from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user import User @@ -39,6 +40,10 @@ class TestFormValidation(MockEppLib): self.API_BASE_PATH = "/api/v1/available/?domain=" self.user = get_user_model().objects.create(username="username") self.factory = RequestFactory() + # We use both of these flags in the test. In the normal app these are generated normally. + # The alternative syntax is adding the decorator to each test. + get_waffle_flag_model().objects.get_or_create(name="organization_feature") + get_waffle_flag_model().objects.get_or_create(name="organization_requests") @less_console_noise_decorator def test_org_contact_zip_invalid(self): diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index ef811e083..4401b73e8 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1,9 +1,10 @@ from django.forms import ValidationError from django.test import TestCase from unittest.mock import patch - +from unittest.mock import Mock from django.test import RequestFactory - +from waffle.models import get_waffle_flag_model +from registrar.views.domain_request import DomainRequestWizard from registrar.models import ( Contact, DomainRequest, @@ -164,6 +165,7 @@ class TestPortfolioInvitations(TestCase): DomainInformation.objects.all().delete() Domain.objects.all().delete() UserPortfolioPermission.objects.all().delete() + UserDomainRole.objects.all().delete() Portfolio.objects.all().delete() PortfolioInvitation.objects.all().delete() User.objects.all().delete() @@ -442,6 +444,294 @@ class TestPortfolioInvitations(TestCase): pass + @less_console_noise_decorator + def test_delete_portfolio_invitation_deletes_portfolio_domain_invitations(self): + """Deleting a portfolio invitation causes domain invitations for the same email on the same + portfolio to be canceled.""" + + email_with_no_user = "email-with-no-user@email.gov" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_in_portfolio_2) + + domain_not_in_portfolio, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio) + invite_3, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_not_in_portfolio) + + invitation_of_email_with_no_user, _ = PortfolioInvitation.objects.get_or_create( + email=email_with_no_user, + portfolio=self.portfolio, + roles=[self.portfolio_role_base, self.portfolio_role_admin], + additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # Delete member (invite) + invitation_of_email_with_no_user.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + @less_console_noise_decorator + def test_deleting_a_retrieved_invitation_has_no_side_effects(self): + """Deleting a retrieved portfolio invitation causes no side effects.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # retrieve the invitation + self.invitation.retrieve() + self.invitation.save() + + # Delete member (invite) + self.invitation.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # Test that no side effects have been triggered + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + @less_console_noise_decorator + def test_delete_portfolio_invitation_deletes_user_domain_roles(self): + """Deleting a portfolio invitation causes domain invitations for the same email on the same + portfolio to be canceled, also deletes any exiting user domain roles on the portfolio for the + user if the user exists.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # Delete member (invite) + self.invitation.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles have been deleted for the domains in portfolio + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + + # The user domain role on the domain not in portfolio still exists + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + class TestUserPortfolioPermission(TestCase): @less_console_noise_decorator @@ -457,6 +747,7 @@ class TestUserPortfolioPermission(TestCase): Domain.objects.all().delete() DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() + DomainInvitation.objects.all().delete() UserPortfolioPermission.objects.all().delete() Portfolio.objects.all().delete() User.objects.all().delete() @@ -750,6 +1041,129 @@ class TestUserPortfolioPermission(TestCase): # Should return the forbidden permissions for member role self.assertEqual(member_only_permissions, set(member_forbidden)) + @less_console_noise_decorator + def test_delete_portfolio_permission_deletes_user_domain_roles(self): + """Deleting a user portfolio permission causes domain invitations for the same email on the same + portfolio to be canceled, also deletes any exiting user domain roles on the portfolio for the + user if the user exists.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # Create portfolio permission + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + portfolio=self.portfolio, user=self.user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # Delete member (user portfolio permission) + portfolio_permission.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles have been deleted for the domains in portfolio + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + + # The user domain role on the domain not in portfolio still exists + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + class TestUser(TestCase): """Test actions that occur on user login, @@ -777,8 +1191,8 @@ class TestUser(TestCase): User.objects.all().delete() UserDomainRole.objects.all().delete() - @patch.object(User, "has_edit_suborganization_portfolio_permission", return_value=True) - def test_portfolio_role_summary_admin(self, mock_edit_suborganization): + @patch.object(User, "has_edit_portfolio_permission", return_value=True) + def test_portfolio_role_summary_admin(self, mock_edit_org): # Test if the user is recognized as an Admin self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Admin"]) @@ -803,7 +1217,7 @@ class TestUser(TestCase): @patch.multiple( User, - has_base_portfolio_permission=lambda self, portfolio: True, + has_view_portfolio_permission=lambda self, portfolio: True, has_edit_request_portfolio_permission=lambda self, portfolio: True, has_any_domains_portfolio_permission=lambda self, portfolio: True, ) @@ -813,7 +1227,7 @@ class TestUser(TestCase): @patch.multiple( User, - has_base_portfolio_permission=lambda self, portfolio: True, + has_view_portfolio_permission=lambda self, portfolio: True, has_edit_request_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_requestor(self): @@ -822,14 +1236,14 @@ class TestUser(TestCase): @patch.multiple( User, - has_base_portfolio_permission=lambda self, portfolio: True, + has_view_portfolio_permission=lambda self, portfolio: True, has_any_domains_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_manager(self): # Test if the user has 'Member' and 'Domain manager' roles self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain manager"]) - @patch.multiple(User, has_base_portfolio_permission=lambda self, portfolio: True) + @patch.multiple(User, has_view_portfolio_permission=lambda self, portfolio: True) def test_portfolio_role_summary_member(self): # Test if the user is recognized as a Member self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Member"]) @@ -839,17 +1253,17 @@ class TestUser(TestCase): self.assertEqual(self.user.portfolio_role_summary(self.portfolio), []) @patch("registrar.models.User._has_portfolio_permission") - def test_has_base_portfolio_permission(self, mock_has_permission): + def test_has_view_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True - self.assertTrue(self.user.has_base_portfolio_permission(self.portfolio)) + self.assertTrue(self.user.has_view_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO) @patch("registrar.models.User._has_portfolio_permission") - def test_has_edit_org_portfolio_permission(self, mock_has_permission): + def test_has_edit_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True - self.assertTrue(self.user.has_edit_org_portfolio_permission(self.portfolio)) + self.assertTrue(self.user.has_edit_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_PORTFOLIO) @patch("registrar.models.User._has_portfolio_permission") @@ -892,20 +1306,6 @@ class TestUser(TestCase): self.assertTrue(self.user.has_edit_request_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) - @patch("registrar.models.User._has_portfolio_permission") - def test_has_view_suborganization_portfolio_permission(self, mock_has_permission): - mock_has_permission.return_value = True - - self.assertTrue(self.user.has_view_suborganization_portfolio_permission(self.portfolio)) - mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) - - @patch("registrar.models.User._has_portfolio_permission") - def test_has_edit_suborganization_portfolio_permission(self, mock_has_permission): - mock_has_permission.return_value = True - - self.assertTrue(self.user.has_edit_suborganization_portfolio_permission(self.portfolio)) - mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) - @less_console_noise_decorator def test_check_transition_domains_without_domains_on_login(self): """A user's on_each_login callback does not check transition domains. @@ -1692,11 +2092,20 @@ class TestDomainRequestIncomplete(TestCase): anything_else="Anything else", is_policy_acknowledged=True, creator=self.user, + city="fake", ) - self.domain_request.other_contacts.add(other) self.domain_request.current_websites.add(current) self.domain_request.alternative_domains.add(alt) + self.wizard = DomainRequestWizard() + self.wizard._domain_request = self.domain_request + self.wizard.request = Mock(user=self.user, session={}) + self.wizard.kwargs = {"id": self.domain_request.id} + + # We use both of these flags in the test. In the normal app these are generated normally. + # The alternative syntax is adding the decorator to each test. + get_waffle_flag_model().objects.get_or_create(name="organization_feature") + get_waffle_flag_model().objects.get_or_create(name="organization_requests") def tearDown(self): super().tearDown() @@ -1711,30 +2120,31 @@ class TestDomainRequestIncomplete(TestCase): @less_console_noise_decorator def test_is_federal_complete(self): - self.assertTrue(self.domain_request._is_federal_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.federal_type = None self.domain_request.save() - self.assertFalse(self.domain_request._is_federal_complete()) + self.domain_request.refresh_from_db() + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_interstate_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.INTERSTATE self.domain_request.about_your_organization = "Something something about your organization" self.domain_request.save() - self.assertTrue(self.domain_request._is_interstate_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.about_your_organization = None self.domain_request.save() - self.assertFalse(self.domain_request._is_interstate_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_state_or_territory_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.STATE_OR_TERRITORY self.domain_request.is_election_board = True self.domain_request.save() - self.assertTrue(self.domain_request._is_state_or_territory_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_state_or_territory_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_tribal_complete(self): @@ -1742,33 +2152,33 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.tribe_name = "Tribe Name" self.domain_request.is_election_board = False self.domain_request.save() - self.assertTrue(self.domain_request._is_tribal_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_tribal_complete()) + self.assertFalse(self.wizard.form_is_complete()) self.domain_request.tribe_name = None self.domain_request.save() - self.assertFalse(self.domain_request._is_tribal_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_county_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.COUNTY self.domain_request.is_election_board = False self.domain_request.save() - self.assertTrue(self.domain_request._is_county_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_county_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_city_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.CITY self.domain_request.is_election_board = False self.domain_request.save() - self.assertTrue(self.domain_request._is_city_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_city_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_special_district_complete(self): @@ -1776,55 +2186,55 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.about_your_organization = "Something something about your organization" self.domain_request.is_election_board = False self.domain_request.save() - self.assertTrue(self.domain_request._is_special_district_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_special_district_complete()) + self.assertFalse(self.wizard.form_is_complete()) self.domain_request.about_your_organization = None self.domain_request.save() - self.assertFalse(self.domain_request._is_special_district_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_organization_name_and_address_complete(self): - self.assertTrue(self.domain_request._is_organization_name_and_address_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.organization_name = None self.domain_request.address_line1 = None self.domain_request.save() - self.assertTrue(self.domain_request._is_organization_name_and_address_complete()) + self.assertTrue(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_senior_official_complete(self): - self.assertTrue(self.domain_request._is_senior_official_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.senior_official = None self.domain_request.save() - self.assertFalse(self.domain_request._is_senior_official_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_requested_domain_complete(self): - self.assertTrue(self.domain_request._is_requested_domain_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.requested_domain = None self.domain_request.save() - self.assertFalse(self.domain_request._is_requested_domain_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_purpose_complete(self): - self.assertTrue(self.domain_request._is_purpose_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.purpose = None self.domain_request.save() - self.assertFalse(self.domain_request._is_purpose_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_other_contacts_complete_missing_one_field(self): - self.assertTrue(self.domain_request._is_other_contacts_complete()) + self.assertTrue(self.wizard.form_is_complete()) contact = self.domain_request.other_contacts.first() contact.first_name = None contact.save() - self.assertFalse(self.domain_request._is_other_contacts_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_other_contacts_complete_all_none(self): self.domain_request.other_contacts.clear() - self.assertFalse(self.domain_request._is_other_contacts_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_other_contacts_False_and_has_rationale(self): @@ -1832,7 +2242,7 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.other_contacts.clear() self.domain_request.other_contacts.exists = False self.domain_request.no_other_contacts_rationale = "Some rationale" - self.assertTrue(self.domain_request._is_other_contacts_complete()) + self.assertTrue(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_other_contacts_False_and_NO_rationale(self): @@ -1840,7 +2250,7 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.other_contacts.clear() self.domain_request.other_contacts.exists = False self.domain_request.no_other_contacts_rationale = None - self.assertFalse(self.domain_request._is_other_contacts_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_additional_details_complete(self): @@ -2044,28 +2454,28 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.save() self.domain_request.refresh_from_db() self.assertEqual( - self.domain_request._is_additional_details_complete(), + self.wizard.form_is_complete(), case["expected"], msg=f"Failed for case: {case}", ) @less_console_noise_decorator def test_is_policy_acknowledgement_complete(self): - self.assertTrue(self.domain_request._is_policy_acknowledgement_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_policy_acknowledged = False - self.assertTrue(self.domain_request._is_policy_acknowledgement_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_policy_acknowledged = None - self.assertFalse(self.domain_request._is_policy_acknowledgement_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_form_complete(self): request = self.factory.get("/") request.user = self.user - self.assertTrue(self.domain_request._form_complete(request)) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.generic_org_type = None self.domain_request.save() - self.assertFalse(self.domain_request._form_complete(request)) + self.assertFalse(self.wizard.form_is_complete()) class TestPortfolio(TestCase): diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index c3528311d..b19b245e5 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -1106,7 +1106,7 @@ class TestDomainRequest(TestCase): federal_agency=fed_agency, organization_type=DomainRequest.OrganizationChoices.FEDERAL, ) - user_portfolio_permission = UserPortfolioPermission.objects.create( # noqa: F841 + UserPortfolioPermission.objects.create( user=self.dummy_user_3, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) # Adds cc'ed email in this test's allow list diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 4ba5b5bc9..bab4f327b 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -725,7 +725,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() self.assertEqual(csv_content, expected_content) - # @less_console_noise_decorator + @less_console_noise_decorator def test_domain_request_data_full(self): """Tests the full domain request report.""" # Remove "Submitted at" because we can't guess this immutable, dynamically generated test data diff --git a/src/registrar/tests/test_utilities.py b/src/registrar/tests/test_utilities.py index 5a2234d66..d882fdedd 100644 --- a/src/registrar/tests/test_utilities.py +++ b/src/registrar/tests/test_utilities.py @@ -1,7 +1,8 @@ from django.test import TestCase from registrar.models import User from waffle.testutils import override_flag -from registrar.utility.waffle import flag_is_active_for_user +from waffle.models import get_waffle_flag_model +from registrar.utility.waffle import flag_is_active_for_user, flag_is_active_anywhere class FlagIsActiveForUserTest(TestCase): @@ -21,3 +22,40 @@ class FlagIsActiveForUserTest(TestCase): # Test that the flag is inactive for the user is_active = flag_is_active_for_user(self.user, "test_flag") self.assertFalse(is_active) + + +class TestFlagIsActiveAnywhere(TestCase): + def setUp(self): + self.user = User.objects.create_user(username="testuser") + self.flag_name = "test_flag" + + @override_flag("test_flag", active=True) + def test_flag_active_for_everyone(self): + """Test when flag is active for everyone""" + is_active = flag_is_active_anywhere("test_flag") + self.assertTrue(is_active) + + @override_flag("test_flag", active=False) + def test_flag_inactive_for_everyone(self): + """Test when flag is inactive for everyone""" + is_active = flag_is_active_anywhere("test_flag") + self.assertFalse(is_active) + + def test_flag_active_for_some_users(self): + """Test when flag is active for specific users""" + flag, _ = get_waffle_flag_model().objects.get_or_create(name="test_flag") + flag.everyone = None + flag.save() + flag.users.add(self.user) + + is_active = flag_is_active_anywhere("test_flag") + self.assertTrue(is_active) + + def test_flag_inactive_with_no_users(self): + """Test when flag has no users and everyone is None""" + flag, _ = get_waffle_flag_model().objects.get_or_create(name="test_flag") + flag.everyone = None + flag.save() + + is_active = flag_is_active_anywhere("test_flag") + self.assertFalse(is_active) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index f46e417be..2dfead13f 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -214,7 +214,7 @@ class HomeTests(TestWithUser): @less_console_noise_decorator def test_state_help_text_expired(self): """Tests if each domain state has help text when expired""" - expired_text = "This domain has expired, but it is still online. " + expired_text = "This domain has expired. " test_domain, _ = Domain.objects.get_or_create(name="expired.gov", state=Domain.State.READY) test_domain.expiration_date = date(2011, 10, 10) test_domain.save() @@ -240,7 +240,7 @@ class HomeTests(TestWithUser): """Tests if each domain state has help text when expiration date is None""" # == Test a expiration of None for state ready. This should be expired. == # - expired_text = "This domain has expired, but it is still online. " + expired_text = "This domain has expired. " test_domain, _ = Domain.objects.get_or_create(name="imexpired.gov", state=Domain.State.READY) test_domain.expiration_date = None test_domain.save() diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index d4766b474..b84d284d8 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -439,15 +439,21 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): username="usertest", ) - self.domaintorenew, _ = Domain.objects.get_or_create( + self.domain_to_renew, _ = Domain.objects.get_or_create( name="domainrenewal.gov", ) - UserDomainRole.objects.get_or_create( - user=self.user, domain=self.domaintorenew, role=UserDomainRole.Roles.MANAGER + self.domain_not_expiring, _ = Domain.objects.get_or_create( + name="domainnotexpiring.gov", expiration_date=timezone.now().date() + timedelta(days=65) ) - DomainInformation.objects.get_or_create(creator=self.user, domain=self.domaintorenew) + self.domain_no_domain_manager, _ = Domain.objects.get_or_create(name="domainnodomainmanager.gov") + + UserDomainRole.objects.get_or_create( + user=self.user, domain=self.domain_to_renew, role=UserDomainRole.Roles.MANAGER + ) + + DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_to_renew) self.portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org", creator=self.user) @@ -473,13 +479,15 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) def test_expiring_domain_on_detail_page_as_domain_manager(self): + """If a user is a domain manager and their domain is expiring soon, + user should be able to see the "Renew to maintain access" link domain overview detail box.""" self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( Domain, "is_expired", self.custom_is_expired_false ): - self.assertEquals(self.domaintorenew.state, Domain.State.UNKNOWN) + self.assertEquals(self.domain_to_renew.state, Domain.State.UNKNOWN) detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.domaintorenew.id}), + reverse("domain", kwargs={"pk": self.domain_to_renew.id}), ) self.assertContains(detail_page, "Expiring soon") @@ -491,6 +499,8 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) def test_expiring_domain_on_detail_page_in_org_model_as_a_non_domain_manager(self): + """In org model: If a user is NOT a domain manager and their domain is expiring soon, + user be notified to contact a domain manager in the domain overview detail box.""" portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org", creator=self.user) non_dom_manage_user = get_user_model().objects.create( first_name="Non Domain", @@ -510,9 +520,9 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ], ) - domaintorenew2, _ = Domain.objects.get_or_create(name="bogusdomain2.gov") + domain_to_renew2, _ = Domain.objects.get_or_create(name="bogusdomain2.gov") DomainInformation.objects.get_or_create( - creator=non_dom_manage_user, domain=domaintorenew2, portfolio=self.portfolio + creator=non_dom_manage_user, domain=domain_to_renew2, portfolio=self.portfolio ) non_dom_manage_user.refresh_from_db() self.client.force_login(non_dom_manage_user) @@ -520,38 +530,42 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): Domain, "is_expired", self.custom_is_expired_false ): detail_page = self.client.get( - reverse("domain", kwargs={"pk": domaintorenew2.id}), + reverse("domain", kwargs={"pk": domain_to_renew2.id}), ) self.assertContains(detail_page, "Contact one of the listed domain managers to renew the domain.") @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) def test_expiring_domain_on_detail_page_in_org_model_as_a_domain_manager(self): + """Inorg model: If a user is a domain manager and their domain is expiring soon, + user should be able to see the "Renew to maintain access" link domain overview detail box.""" portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org2", creator=self.user) - domaintorenew3, _ = Domain.objects.get_or_create(name="bogusdomain3.gov") + domain_to_renew3, _ = Domain.objects.get_or_create(name="bogusdomain3.gov") - UserDomainRole.objects.get_or_create(user=self.user, domain=domaintorenew3, role=UserDomainRole.Roles.MANAGER) - DomainInformation.objects.get_or_create(creator=self.user, domain=domaintorenew3, portfolio=portfolio) + UserDomainRole.objects.get_or_create(user=self.user, domain=domain_to_renew3, role=UserDomainRole.Roles.MANAGER) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_to_renew3, portfolio=portfolio) self.user.refresh_from_db() self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( Domain, "is_expired", self.custom_is_expired_false ): detail_page = self.client.get( - reverse("domain", kwargs={"pk": domaintorenew3.id}), + reverse("domain", kwargs={"pk": domain_to_renew3.id}), ) self.assertContains(detail_page, "Renew to maintain access") @override_flag("domain_renewal", active=True) def test_domain_renewal_form_and_sidebar_expiring(self): + """If a user is a domain manager and their domain is expiring soon, + user should be able to see Renewal Form on the sidebar.""" self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( Domain, "is_expiring", self.custom_is_expiring ): # Grab the detail page detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.domaintorenew.id}), + reverse("domain", kwargs={"pk": self.domain_to_renew.id}), ) # Make sure we see the link as a domain manager @@ -561,18 +575,19 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(detail_page, "Renewal form") # Grab link to the renewal page - renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domaintorenew.id}) + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domain_to_renew.id}) self.assertContains(detail_page, f'href="{renewal_form_url}"') # Simulate clicking the link response = self.client.get(renewal_form_url) self.assertEqual(response.status_code, 200) - self.assertContains(response, f"Renew {self.domaintorenew.name}") + self.assertContains(response, f"Renew {self.domain_to_renew.name}") @override_flag("domain_renewal", active=True) def test_domain_renewal_form_and_sidebar_expired(self): - + """If a user is a domain manager and their domain is expired, + user should be able to see Renewal Form on the sidebar.""" self.client.force_login(self.user) with patch.object(Domain, "is_expired", self.custom_is_expired_true), patch.object( @@ -580,10 +595,9 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): ): # Grab the detail page detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.domaintorenew.id}), + reverse("domain", kwargs={"pk": self.domain_to_renew.id}), ) - print("puglesss", self.domaintorenew.is_expired) # Make sure we see the link as a domain manager self.assertContains(detail_page, "Renew to maintain access") @@ -591,17 +605,19 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(detail_page, "Renewal form") # Grab link to the renewal page - renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domaintorenew.id}) + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domain_to_renew.id}) self.assertContains(detail_page, f'href="{renewal_form_url}"') # Simulate clicking the link response = self.client.get(renewal_form_url) self.assertEqual(response.status_code, 200) - self.assertContains(response, f"Renew {self.domaintorenew.name}") + self.assertContains(response, f"Renew {self.domain_to_renew.name}") @override_flag("domain_renewal", active=True) def test_domain_renewal_form_your_contact_info_edit(self): + """Checking that if a user is a domain manager they can edit the + Your Profile portion of the Renewal Form.""" with less_console_noise(): # Start on the Renewal page for the domain renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) @@ -620,6 +636,8 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) def test_domain_renewal_form_security_email_edit(self): + """Checking that if a user is a domain manager they can edit the + Security Email portion of the Renewal Form.""" with less_console_noise(): # Start on the Renewal page for the domain renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) @@ -641,6 +659,8 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) def test_domain_renewal_form_domain_manager_edit(self): + """Checking that if a user is a domain manager they can edit the + Domain Manager portion of the Renewal Form.""" with less_console_noise(): # Start on the Renewal page for the domain renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) @@ -658,8 +678,26 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(edit_page, "Domain managers can update all information related to a domain") @override_flag("domain_renewal", active=True) - def test_ack_checkbox_not_checked(self): + def test_domain_renewal_form_not_expired_or_expiring(self): + """Checking that if the user's domain is not expired or expiring that user should not be able + to access /renewal and that it should receive a 403.""" + with less_console_noise(): + # Start on the Renewal page for the domain + renewal_page = self.client.get(reverse("domain-renewal", kwargs={"pk": self.domain_not_expiring.id})) + self.assertEqual(renewal_page.status_code, 403) + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_does_not_appear_if_not_domain_manager(self): + """If user is not a domain manager and tries to access /renewal, user should receive a 403.""" + with patch.object(Domain, "is_expired", self.custom_is_expired_true), patch.object( + Domain, "is_expired", self.custom_is_expired_true + ): + renewal_page = self.client.get(reverse("domain-renewal", kwargs={"pk": self.domain_no_domain_manager.id})) + self.assertEqual(renewal_page.status_code, 403) + + @override_flag("domain_renewal", active=True) + def test_ack_checkbox_not_checked(self): + """If user don't check the checkbox, user should receive an error message.""" # Grab the renewal URL renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) @@ -671,7 +709,8 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) def test_ack_checkbox_checked(self): - + """If user check the checkbox and submits the form, + user should be redirected Domain Over page with an updated by 1 year expiration date""" # Grab the renewal URL with patch.object(Domain, "renew_domain", self.custom_renew_domain): renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) @@ -810,7 +849,10 @@ class TestDomainManagers(TestDomainOverview): # Verify that the invitation emails were sent mock_send_portfolio_email.assert_called_once_with( - email="mayor@igorville.gov", requestor=self.user, portfolio=self.portfolio + email="mayor@igorville.gov", + requestor=self.user, + portfolio=self.portfolio, + is_admin_invitation=False, ) mock_send_domain_email.assert_called_once() call_args = mock_send_domain_email.call_args.kwargs @@ -864,7 +906,10 @@ class TestDomainManagers(TestDomainOverview): # Verify that the invitation emails were sent mock_send_portfolio_email.assert_called_once_with( - email="notauser@igorville.gov", requestor=self.user, portfolio=self.portfolio + email="notauser@igorville.gov", + requestor=self.user, + portfolio=self.portfolio, + is_admin_invitation=False, ) mock_send_domain_email.assert_called_once() call_args = mock_send_domain_email.call_args.kwargs @@ -886,6 +931,40 @@ class TestDomainManagers(TestDomainOverview): success_page = success_result.follow() self.assertContains(success_page, "notauser@igorville.gov") + @override_flag("organization_feature", active=True) + @less_console_noise_decorator + @patch("registrar.views.domain.send_portfolio_invitation_email") + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_user_add_form_fails_to_send_to_some_managers( + self, mock_send_domain_email, mock_send_portfolio_email + ): + """Adding an email not associated with a user works and sends portfolio invitation, + and when domain managers email(s) fail to send, assert proper warning displayed.""" + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + + add_page.form["email"] = "notauser@igorville.gov" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + mock_send_domain_email.return_value = False + + success_result = add_page.form.submit() + + self.assertEqual(success_result.status_code, 302) + self.assertEqual( + success_result["Location"], + reverse("domain-users", kwargs={"pk": self.domain.id}), + ) + + # Verify that the invitation emails were sent + mock_send_portfolio_email.assert_called_once() + mock_send_domain_email.assert_called_once() + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + success_page = success_result.follow() + self.assertContains(success_page, "Could not send email confirmation to existing domain managers.") + @boto3_mocking.patching @override_flag("organization_feature", active=True) @less_console_noise_decorator @@ -965,7 +1044,10 @@ class TestDomainManagers(TestDomainOverview): # Verify that the invitation emails were sent mock_send_portfolio_email.assert_called_once_with( - email="mayor@igorville.gov", requestor=self.user, portfolio=self.portfolio + email="mayor@igorville.gov", + requestor=self.user, + portfolio=self.portfolio, + is_admin_invitation=False, ) mock_send_domain_email.assert_not_called() @@ -981,6 +1063,23 @@ class TestDomainManagers(TestDomainOverview): success_page = success_result.follow() self.assertContains(success_page, "Failed to send email.") + @boto3_mocking.patching + @less_console_noise_decorator + @patch("registrar.views.domain.send_templated_email") + def test_domain_remove_manager(self, mock_send_templated_email): + """Removing a domain manager sends notification email to other domain managers.""" + self.manager, _ = User.objects.get_or_create(email="mayor@igorville.com", first_name="Hello", last_name="World") + self.manager_domain_permission, _ = UserDomainRole.objects.get_or_create(user=self.manager, domain=self.domain) + self.client.post(reverse("domain-user-delete", kwargs={"pk": self.domain.id, "user_pk": self.manager.id})) + + # Verify that the notification emails were sent to domain manager + mock_send_templated_email.assert_called_once_with( + "emails/domain_manager_deleted_notification.txt", + "emails/domain_manager_deleted_notification_subject.txt", + to_address="info@example.com", + context=ANY, + ) + @less_console_noise_decorator @patch("registrar.views.domain.send_domain_invitation_email") def test_domain_invitation_created(self, mock_send_domain_email): @@ -2108,7 +2207,7 @@ class TestDomainSuborganization(TestDomainOverview): self.domain_information.refresh_from_db() # Add portfolio perms to the user object - portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + UserPortfolioPermission.objects.get_or_create( user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) @@ -2866,11 +2965,11 @@ class TestDomainRenewal(TestWithUser): name="igorville.gov", expiration_date=expiring_date ) self.domain_with_expired_date, _ = Domain.objects.get_or_create( - name="domainwithexpireddate.com", expiration_date=expired_date + name="domainwithexpireddate.gov", expiration_date=expired_date ) self.domain_with_current_date, _ = Domain.objects.get_or_create( - name="domainwithfarexpireddate.com", expiration_date=expiring_date_current + name="domainwithfarexpireddate.gov", expiration_date=expiring_date_current ) UserDomainRole.objects.get_or_create( @@ -2916,7 +3015,7 @@ class TestDomainRenewal(TestWithUser): today = datetime.now() expiring_date = (today + timedelta(days=30)).strftime("%Y-%m-%d") self.domain_with_another_expiring, _ = Domain.objects.get_or_create( - name="domainwithanotherexpiringdate.com", expiration_date=expiring_date + name="domainwithanotherexpiringdate.gov", expiration_date=expiring_date ) UserDomainRole.objects.get_or_create( @@ -2952,7 +3051,7 @@ class TestDomainRenewal(TestWithUser): today = datetime.now() expiring_date = (today + timedelta(days=31)).strftime("%Y-%m-%d") self.domain_with_another_expiring_org_model, _ = Domain.objects.get_or_create( - name="domainwithanotherexpiringdate_orgmodel.com", expiration_date=expiring_date + name="domainwithanotherexpiringdate_orgmodel.gov", expiration_date=expiring_date ) UserDomainRole.objects.get_or_create( diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py index ceae1e35f..c505421ec 100644 --- a/src/registrar/tests/test_views_members_json.py +++ b/src/registrar/tests/test_views_members_json.py @@ -372,6 +372,21 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): domain=domain3, ) + # create another domain in the portfolio + # but make sure the domain invitation is canceled + domain4 = Domain.objects.create( + name="somedomain4.com", + ) + DomainInformation.objects.create( + creator=self.user, + domain=domain4, + ) + DomainInvitation.objects.create( + email=self.email6, + domain=domain4, + status=DomainInvitation.DomainInvitationStatus.CANCELED, + ) + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) self.assertEqual(response.status_code, 200) data = response.json @@ -381,6 +396,7 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): self.assertIn("somedomain1.com", domain_names) self.assertIn("thissecondinvitetestsasubqueryinjson@lets.notbreak", domain_names) self.assertNotIn("somedomain3.com", domain_names) + self.assertNotIn("somedomain4.com", domain_names) @less_console_noise_decorator @override_flag("organization_feature", active=True) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 3afbb2a7b..097aa1879 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -22,7 +22,7 @@ from registrar.models.utility.portfolio_helper import UserPortfolioPermissionCho from registrar.tests.test_views import TestWithUser from registrar.utility.email import EmailSendingError from registrar.utility.errors import MissingEmailError -from .common import MockSESClient, completed_domain_request, create_test_user, create_user +from .common import MockEppLib, MockSESClient, completed_domain_request, create_test_user, create_user from waffle.testutils import override_flag from django.contrib.sessions.middleware import SessionMiddleware import boto3_mocking # type: ignore @@ -1097,8 +1097,10 @@ class TestPortfolio(WebTest): @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_requests", active=True) + @override_flag("organization_members", active=True) def test_main_nav_when_user_has_no_permissions(self): - """Test the nav contains a link to the no requests page""" + """Test the nav contains a link to the no requests page + Also test that members link not present""" UserPortfolioPermission.objects.get_or_create( user=self.user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] ) @@ -1118,20 +1120,23 @@ class TestPortfolio(WebTest): self.assertNotContains(portfolio_landing_page, "basic-nav-section-two") # link to requests self.assertNotContains(portfolio_landing_page, 'href="/requests/') - # link to create + # link to create request self.assertNotContains(portfolio_landing_page, 'href="/request/') + # link to members + self.assertNotContains(portfolio_landing_page, 'href="/members/') @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_requests", active=True) + @override_flag("organization_members", active=True) def test_main_nav_when_user_has_all_permissions(self): """Test the nav contains a dropdown with a link to create and another link to view requests - Also test for the existence of the Create a new request btn on the requests page""" + Also test for the existence of the Create a new request btn on the requests page + Also test for the existence of the members link""" UserPortfolioPermission.objects.get_or_create( user=self.user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], - additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS], ) self.client.force_login(self.user) # create and submit a domain request @@ -1151,6 +1156,8 @@ class TestPortfolio(WebTest): self.assertContains(portfolio_landing_page, 'href="/requests/') # link to create self.assertContains(portfolio_landing_page, 'href="/request/') + # link to members + self.assertContains(portfolio_landing_page, 'href="/members/') requests_page = self.client.get(reverse("domain-requests")) @@ -1160,15 +1167,18 @@ class TestPortfolio(WebTest): @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_requests", active=True) + @override_flag("organization_members", active=True) def test_main_nav_when_user_has_view_but_not_edit_permissions(self): """Test the nav contains a simple link to view requests - Also test for the existence of the Create a new request btn on the requests page""" + Also test for the existence of the Create a new request btn on the requests page + Also test for the existence of members link""" UserPortfolioPermission.objects.get_or_create( user=self.user, portfolio=self.portfolio, additional_permissions=[ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.VIEW_MEMBERS, ], ) self.client.force_login(self.user) @@ -1189,6 +1199,8 @@ class TestPortfolio(WebTest): self.assertContains(portfolio_landing_page, 'href="/requests/') # link to create self.assertNotContains(portfolio_landing_page, 'href="/request/') + # link to members + self.assertContains(portfolio_landing_page, 'href="/members/') requests_page = self.client.get(reverse("domain-requests")) @@ -1631,10 +1643,33 @@ class TestPortfolio(WebTest): # Assert that the toggleable alert ID exists self.assertContains(response, '

    list[Domain]: +def _normalize_domains(domains: Domain | list[Domain]) -> list[Domain]: """Ensures domains is always a list.""" return [domains] if isinstance(domains, Domain) else domains -def get_requestor_email(requestor, domains): +def _get_requestor_email(requestor, domains=None, portfolio=None): """Get the requestor's email or raise an error if it's missing. If the requestor is staff, default email is returned. + + Raises: + MissingEmailError """ if requestor.is_staff: return settings.DEFAULT_FROM_EMAIL if not requestor.email or requestor.email.strip() == "": - domain_names = ", ".join([domain.name for domain in domains]) - raise MissingEmailError(email=requestor.email, domain=domain_names) + domain_names = None + if domains: + domain_names = ", ".join([domain.name for domain in domains]) + raise MissingEmailError(email=requestor.email, domain=domain_names, portfolio=portfolio) return requestor.email @@ -158,7 +177,7 @@ def send_invitation_email(email, requestor_email, domains, requested_user): raise EmailSendingError(f"Could not send email invitation to {email} for domains: {domain_names}") from err -def send_portfolio_invitation_email(email: str, requestor, portfolio): +def send_portfolio_invitation_email(email: str, requestor, portfolio, is_admin_invitation): """ Sends a portfolio member invitation email to the specified address. @@ -168,21 +187,17 @@ def send_portfolio_invitation_email(email: str, requestor, portfolio): email (str): Email address of the recipient requestor (User): The user initiating the invitation. portfolio (Portfolio): The portfolio object for which the invitation is being sent. + is_admin_invitation (boolean): boolean indicating if the invitation is an admin invitation + + Returns: + Boolean indicating if all messages were sent successfully. Raises: MissingEmailError: If the requestor has no email associated with their account. EmailSendingError: If there is an error while sending the email. """ - # Default email address for staff - requestor_email = settings.DEFAULT_FROM_EMAIL - - # Check if the requestor is staff and has an email - if not requestor.is_staff: - if not requestor.email or requestor.email.strip() == "": - raise MissingEmailError(email=email, portfolio=portfolio) - else: - requestor_email = requestor.email + requestor_email = _get_requestor_email(requestor, portfolio=portfolio) try: send_templated_email( @@ -199,3 +214,119 @@ def send_portfolio_invitation_email(email: str, requestor, portfolio): raise EmailSendingError( f"Could not sent email invitation to {email} for portfolio {portfolio}. Portfolio invitation not saved." ) from err + + all_admin_emails_sent = True + # send emails to portfolio admins + if is_admin_invitation: + all_admin_emails_sent = _send_portfolio_admin_addition_emails_to_portfolio_admins( + email=email, + requestor_email=requestor_email, + portfolio=portfolio, + ) + return all_admin_emails_sent + + +def send_portfolio_admin_addition_emails(email: str, requestor, portfolio: Portfolio): + """ + Notifies all portfolio admins of the provided portfolio of a newly invited portfolio admin + + Returns: + Boolean indicating if all messages were sent successfully. + + Raises: + MissingEmailError + """ + requestor_email = _get_requestor_email(requestor, portfolio=portfolio) + return _send_portfolio_admin_addition_emails_to_portfolio_admins(email, requestor_email, portfolio) + + +def _send_portfolio_admin_addition_emails_to_portfolio_admins(email: str, requestor_email, portfolio: Portfolio): + """ + Notifies all portfolio admins of the provided portfolio of a newly invited portfolio admin + + Returns: + Boolean indicating if all messages were sent successfully. + """ + all_emails_sent = True + # Get each portfolio admin from list + user_portfolio_permissions = UserPortfolioPermission.objects.filter( + portfolio=portfolio, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ).exclude(user__email=email) + for user_portfolio_permission in user_portfolio_permissions: + # Send email to each portfolio_admin + user = user_portfolio_permission.user + try: + send_templated_email( + "emails/portfolio_admin_addition_notification.txt", + "emails/portfolio_admin_addition_notification_subject.txt", + to_address=user.email, + context={ + "portfolio": portfolio, + "requestor_email": requestor_email, + "invited_email_address": email, + "portfolio_admin": user, + "date": date.today(), + }, + ) + except EmailSendingError: + logger.warning( + "Could not send email organization admin notification to %s " "for portfolio: %s", + user.email, + portfolio.organization_name, + exc_info=True, + ) + all_emails_sent = False + return all_emails_sent + + +def send_portfolio_admin_removal_emails(email: str, requestor, portfolio: Portfolio): + """ + Notifies all portfolio admins of the provided portfolio of a removed portfolio admin + + Returns: + Boolean indicating if all messages were sent successfully. + + Raises: + MissingEmailError + """ + requestor_email = _get_requestor_email(requestor, portfolio=portfolio) + return _send_portfolio_admin_removal_emails_to_portfolio_admins(email, requestor_email, portfolio) + + +def _send_portfolio_admin_removal_emails_to_portfolio_admins(email: str, requestor_email, portfolio: Portfolio): + """ + Notifies all portfolio admins of the provided portfolio of a removed portfolio admin + + Returns: + Boolean indicating if all messages were sent successfully. + """ + all_emails_sent = True + # Get each portfolio admin from list + user_portfolio_permissions = UserPortfolioPermission.objects.filter( + portfolio=portfolio, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ).exclude(user__email=email) + for user_portfolio_permission in user_portfolio_permissions: + # Send email to each portfolio_admin + user = user_portfolio_permission.user + try: + send_templated_email( + "emails/portfolio_admin_removal_notification.txt", + "emails/portfolio_admin_removal_notification_subject.txt", + to_address=user.email, + context={ + "portfolio": portfolio, + "requestor_email": requestor_email, + "removed_email_address": email, + "portfolio_admin": user, + "date": date.today(), + }, + ) + except EmailSendingError: + logger.warning( + "Could not send email organization admin notification to %s " "for portfolio: %s", + user.email, + portfolio.organization_name, + exc_info=True, + ) + all_emails_sent = False + return all_emails_sent diff --git a/src/registrar/utility/waffle.py b/src/registrar/utility/waffle.py index a78799e4c..3071fbed9 100644 --- a/src/registrar/utility/waffle.py +++ b/src/registrar/utility/waffle.py @@ -1,5 +1,6 @@ from django.http import HttpRequest from waffle.decorators import flag_is_active +from waffle.models import get_waffle_flag_model def flag_is_active_for_user(user, flag_name): @@ -10,3 +11,21 @@ def flag_is_active_for_user(user, flag_name): request = HttpRequest() request.user = user return flag_is_active(request, flag_name) + + +def flag_is_active_anywhere(flag_name): + """Checks if the given flag name is active for anyone, anywhere. + More specifically, it checks on flag.everyone or flag.users.exists(). + Does not check self.superuser, self.staff or self.group. + + This function effectively behaves like a switch: + If said flag is enabled for someone, somewhere - return true. + Otherwise - return false. + """ + try: + flag = get_waffle_flag_model().get(flag_name) + if flag.everyone is None: + return flag.users.exists() + return flag.everyone + except get_waffle_flag_model().DoesNotExist: + return False diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 83898934c..089bbe1a9 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -311,11 +311,39 @@ class DomainView(DomainBaseView): self._update_session_with_domain() -class DomainRenewalView(DomainView): +class DomainRenewalView(DomainBaseView): """Domain detail overview page.""" template_name = "domain_renewal.html" + def get_context_data(self, **kwargs): + """Grabs the security email information and adds security_email to the renewal form context + sets it to None if it uses a default email""" + + context = super().get_context_data(**kwargs) + + default_emails = [DefaultEmail.PUBLIC_CONTACT_DEFAULT.value, DefaultEmail.LEGACY_DEFAULT.value] + + context["hidden_security_emails"] = default_emails + + security_email = self.object.get_security_email() + context["security_email"] = security_email + return context + + def in_editable_state(self, pk): + """Override in_editable_state from DomainPermission + Allow renewal form to be accessed + returns boolean""" + requested_domain = None + if Domain.objects.filter(id=pk).exists(): + requested_domain = Domain.objects.get(id=pk) + + return ( + requested_domain + and requested_domain.is_editable() + and (requested_domain.is_expiring() or requested_domain.is_expired()) + ) + def post(self, request, pk): domain = get_object_or_404(Domain, id=pk) @@ -1206,7 +1234,9 @@ class DomainAddUserView(DomainFormBaseView): and requestor_can_update_portfolio and not member_of_this_org ): - send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=domain_org) + send_portfolio_invitation_email( + email=requested_email, requestor=requestor, portfolio=domain_org, is_admin_invitation=False + ) portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( email=requested_email, portfolio=domain_org, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] ) @@ -1227,24 +1257,26 @@ class DomainAddUserView(DomainFormBaseView): def _handle_new_user_invitation(self, email, requestor, member_of_different_org): """Handle invitation for a new user who does not exist in the system.""" - send_domain_invitation_email( + if not send_domain_invitation_email( email=email, requestor=requestor, domains=self.object, is_member_of_different_org=member_of_different_org, - ) + ): + messages.warning(self.request, "Could not send email confirmation to existing domain managers.") DomainInvitation.objects.get_or_create(email=email, domain=self.object) messages.success(self.request, f"{email} has been invited to the domain: {self.object}") def _handle_existing_user(self, email, requestor, requested_user, member_of_different_org): """Handle adding an existing user to the domain.""" - send_domain_invitation_email( + if not send_domain_invitation_email( email=email, requestor=requestor, domains=self.object, is_member_of_different_org=member_of_different_org, requested_user=requested_user, - ) + ): + messages.warning(self.request, "Could not send email confirmation to existing domain managers.") UserDomainRole.objects.create( user=requested_user, domain=self.object, @@ -1316,10 +1348,49 @@ class DomainDeleteUserView(UserDomainRolePermissionDeleteView): # Delete the object super().form_valid(form) + # Email all domain managers that domain manager has been removed + domain = self.object.domain + + context = { + "domain": domain, + "removed_by": self.request.user, + "manager_removed": self.object.user, + "date": date.today(), + "changes": "Domain Manager", + } + self.email_domain_managers( + domain, + "emails/domain_manager_deleted_notification.txt", + "emails/domain_manager_deleted_notification_subject.txt", + context, + ) + # Add a success message messages.success(self.request, self.get_success_message()) return redirect(self.get_success_url()) + def email_domain_managers(self, domain: Domain, template: str, subject_template: str, context={}): + manager_pks = UserDomainRole.objects.filter(domain=domain.pk, role=UserDomainRole.Roles.MANAGER).values_list( + "user", flat=True + ) + emails = list(User.objects.filter(pk__in=manager_pks).values_list("email", flat=True)) + + for email in emails: + try: + send_templated_email( + template, + subject_template, + to_address=email, + context=context, + ) + except EmailSendingError: + logger.warning( + "Could not send notification email to %s for domain %s", + email, + domain.name, + exc_info=True, + ) + def post(self, request, *args, **kwargs): """Custom post implementation to ensure last userdomainrole is not removed and to redirect to home in the event that the user deletes themselves""" diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 9754b0d0c..3248c1368 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -107,15 +107,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): Step.TRIBAL_GOVERNMENT: lambda self: self.domain_request.tribe_name is not None, Step.ORGANIZATION_FEDERAL: lambda self: self.domain_request.federal_type is not None, Step.ORGANIZATION_ELECTION: lambda self: self.domain_request.is_election_board is not None, - Step.ORGANIZATION_CONTACT: lambda self: ( - self.domain_request.federal_agency is not None - or self.domain_request.organization_name is not None - or self.domain_request.address_line1 is not None - or self.domain_request.city is not None - or self.domain_request.state_territory is not None - or self.domain_request.zipcode is not None - or self.domain_request.urbanization is not None - ), + Step.ORGANIZATION_CONTACT: lambda self: self.from_model("unlock_organization_contact", False), Step.ABOUT_YOUR_ORGANIZATION: lambda self: self.domain_request.about_your_organization is not None, Step.SENIOR_OFFICIAL: lambda self: self.domain_request.senior_official is not None, Step.CURRENT_SITES: lambda self: ( @@ -123,9 +115,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): ), Step.DOTGOV_DOMAIN: lambda self: self.domain_request.requested_domain is not None, Step.PURPOSE: lambda self: self.domain_request.purpose is not None, - Step.OTHER_CONTACTS: lambda self: ( - self.domain_request.other_contacts.exists() or self.domain_request.no_other_contacts_rationale is not None - ), + Step.OTHER_CONTACTS: lambda self: self.from_model("unlock_other_contacts", False), Step.ADDITIONAL_DETAILS: lambda self: ( # Additional details is complete as long as "has anything else" and "has cisa rep" are not None ( @@ -434,20 +424,28 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): Queries the DB for a domain request and returns a list of unlocked steps.""" return [key for key, is_unlocked_checker in self.unlocking_steps.items() if is_unlocked_checker(self)] + def form_is_complete(self): + """Determines if all required steps in the domain request form are complete. + Returns: + bool: True if all required steps are complete, False otherwise + """ + # 1. Get all steps visibly present to the user (required steps) + # 2. Return every possible step that is "unlocked" (even hidden, conditional ones) + # 3. Narrows down the list to remove hidden conditional steps + required_steps = set(self.steps.all) + unlockable_steps = {step.value for step in self.db_check_for_unlocking_steps()} + unlocked_steps = {step for step in required_steps if step in unlockable_steps} + return required_steps == unlocked_steps + def get_context_data(self): """Define context for access on all wizard pages.""" - requested_domain_name = None if self.domain_request.requested_domain is not None: requested_domain_name = self.domain_request.requested_domain.name context = {} - - # Note: we will want to consolidate the non_org_steps_complete check into the same check that - # org_steps_complete is using at some point. - non_org_steps_complete = DomainRequest._form_complete(self.domain_request, self.request) - org_steps_complete = len(self.db_check_for_unlocking_steps()) == len(self.steps) - if (not self.is_portfolio and non_org_steps_complete) or (self.is_portfolio and org_steps_complete): + org_steps_complete = self.form_is_complete() + if org_steps_complete: context = { "form_titles": self.titles, "steps": self.steps, @@ -782,7 +780,8 @@ class Review(DomainRequestWizard): forms = [] # type: ignore def get_context_data(self): - if DomainRequest._form_complete(self.domain_request, self.request) is False: + form_complete = self.form_is_complete() + if form_complete is False: logger.warning("User arrived at review page with an incomplete form.") context = super().get_context_data() context["Step"] = self.get_step_enum().__members__ diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index a45ad66e9..29dc6a71c 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -123,7 +123,11 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): # Subquery to get concatenated domain information for each email domain_invitations = ( - DomainInvitation.objects.filter(email=OuterRef("email"), domain__domain_info__portfolio=portfolio) + DomainInvitation.objects.filter( + email=OuterRef("email"), + domain__domain_info__portfolio=portfolio, + status=DomainInvitation.DomainInvitationStatus.INVITED, + ) .annotate( concatenated_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField()) ) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index f5db165d1..0f93ec8e1 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -15,7 +15,12 @@ from registrar.models.user_domain_role import UserDomainRole from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.utility.email import EmailSendingError -from registrar.utility.email_invitations import send_domain_invitation_email, send_portfolio_invitation_email +from registrar.utility.email_invitations import ( + send_domain_invitation_email, + send_portfolio_admin_addition_emails, + send_portfolio_admin_removal_emails, + send_portfolio_invitation_email, +) from registrar.utility.errors import MissingEmailError from registrar.utility.enums import DefaultUserValues from registrar.views.utility.mixins import PortfolioMemberPermission @@ -143,6 +148,19 @@ class PortfolioMemberDeleteView(PortfolioMemberPermission, View): messages.error(request, error_message) return redirect(reverse("member", kwargs={"pk": pk})) + # if member being removed is an admin + if UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_member_permission.roles: + try: + # attempt to send notification emails of the removal to other portfolio admins + if not send_portfolio_admin_removal_emails( + email=portfolio_member_permission.user.email, + requestor=request.user, + portfolio=portfolio_member_permission.portfolio, + ): + messages.warning(self.request, "Could not send email notification to existing organization admins.") + except Exception as e: + self._handle_exceptions(e) + # passed all error conditions portfolio_member_permission.delete() @@ -154,6 +172,18 @@ class PortfolioMemberDeleteView(PortfolioMemberPermission, View): messages.success(request, success_message) return redirect(reverse("members")) + def _handle_exceptions(self, exception): + """Handle exceptions raised during the process.""" + if isinstance(exception, MissingEmailError): + messages.warning(self.request, "Could not send email notification to existing organization admins.") + logger.warning( + "Could not send email notification to existing organization admins.", + exc_info=True, + ) + else: + logger.warning("Could not send email notification to existing organization admins.", exc_info=True) + messages.warning(self.request, "Could not send email notification to existing organization admins.") + class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): @@ -177,16 +207,33 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): def post(self, request, pk): portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) - user_initially_is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_permission.roles user = portfolio_permission.user form = self.form_class(request.POST, instance=portfolio_permission) + removing_admin_role_on_self = False if form.is_valid(): - # Check if user is removing their own admin or edit role - removing_admin_role_on_self = ( - request.user == user - and user_initially_is_admin - and UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in form.cleaned_data.get("role", []) - ) + try: + if form.is_change_from_member_to_admin(): + if not send_portfolio_admin_addition_emails( + email=portfolio_permission.user.email, + requestor=request.user, + portfolio=portfolio_permission.portfolio, + ): + messages.warning( + self.request, "Could not send email notification to existing organization admins." + ) + elif form.is_change_from_admin_to_member(): + if not send_portfolio_admin_removal_emails( + email=portfolio_permission.user.email, + requestor=request.user, + portfolio=portfolio_permission.portfolio, + ): + messages.warning( + self.request, "Could not send email notification to existing organization admins." + ) + # Check if user is removing their own admin or edit role + removing_admin_role_on_self = request.user == user + except Exception as e: + self._handle_exceptions(e) form.save() messages.success(self.request, "The member access and permission changes have been saved.") return redirect("member", pk=pk) if not removing_admin_role_on_self else redirect("home") @@ -200,6 +247,18 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): }, ) + def _handle_exceptions(self, exception): + """Handle exceptions raised during the process.""" + if isinstance(exception, MissingEmailError): + messages.warning(self.request, "Could not send email notification to existing organization admins.") + logger.warning( + "Could not send email notification to existing organization admins.", + exc_info=True, + ) + else: + logger.warning("Could not send email notification to existing organization admins.", exc_info=True) + messages.warning(self.request, "Could not send email notification to existing organization admins.") + class PortfolioMemberDomainsView(PortfolioMemberDomainsPermissionView, View): @@ -303,13 +362,14 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V # get added_domains from ids to pass to send email method and bulk create added_domains = Domain.objects.filter(id__in=added_domain_ids) member_of_a_different_org, _ = get_org_membership(portfolio, member.email, member) - send_domain_invitation_email( + if not send_domain_invitation_email( email=member.email, requestor=requestor, domains=added_domains, is_member_of_different_org=member_of_a_different_org, requested_user=member, - ) + ): + messages.warning(self.request, "Could not send email confirmation to existing domain managers.") # Bulk create UserDomainRole instances for added domains UserDomainRole.objects.bulk_create( [ @@ -379,6 +439,17 @@ class PortfolioInvitedMemberDeleteView(PortfolioMemberPermission, View): """ portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + # if invitation being removed is an admin + if UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_invitation.roles: + try: + # attempt to send notification emails of the removal to portfolio admins + if not send_portfolio_admin_removal_emails( + email=portfolio_invitation.email, requestor=request.user, portfolio=portfolio_invitation.portfolio + ): + messages.warning(self.request, "Could not send email notification to existing organization admins.") + except Exception as e: + self._handle_exceptions(e) + portfolio_invitation.delete() success_message = f"You've removed {portfolio_invitation.email} from the organization." @@ -389,6 +460,18 @@ class PortfolioInvitedMemberDeleteView(PortfolioMemberPermission, View): messages.success(request, success_message) return redirect(reverse("members")) + def _handle_exceptions(self, exception): + """Handle exceptions raised during the process.""" + if isinstance(exception, MissingEmailError): + messages.warning(self.request, "Could not send email notification to existing organization admins.") + logger.warning( + "Could not send email notification to existing organization admins.", + exc_info=True, + ) + else: + logger.warning("Could not send email notification to existing organization admins.", exc_info=True) + messages.warning(self.request, "Could not send email notification to existing organization admins.") + class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): @@ -412,6 +495,27 @@ class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) form = self.form_class(request.POST, instance=portfolio_invitation) if form.is_valid(): + try: + if form.is_change_from_member_to_admin(): + if not send_portfolio_admin_addition_emails( + email=portfolio_invitation.email, + requestor=request.user, + portfolio=portfolio_invitation.portfolio, + ): + messages.warning( + self.request, "Could not send email notification to existing organization admins." + ) + elif form.is_change_from_admin_to_member(): + if not send_portfolio_admin_removal_emails( + email=portfolio_invitation.email, + requestor=request.user, + portfolio=portfolio_invitation.portfolio, + ): + messages.warning( + self.request, "Could not send email notification to existing organization admins." + ) + except Exception as e: + self._handle_exceptions(e) form.save() messages.success(self.request, "The member access and permission changes have been saved.") return redirect("invitedmember", pk=pk) @@ -425,6 +529,18 @@ class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): }, ) + def _handle_exceptions(self, exception): + """Handle exceptions raised during the process.""" + if isinstance(exception, MissingEmailError): + messages.warning(self.request, "Could not send email notification to existing organization admins.") + logger.warning( + "Could not send email notification to existing organization admins.", + exc_info=True, + ) + else: + logger.warning("Could not send email notification to existing organization admins.", exc_info=True) + messages.warning(self.request, "Could not send email notification to existing organization admins.") + class PortfolioInvitedMemberDomainsView(PortfolioMemberDomainsPermissionView, View): @@ -525,12 +641,13 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission # get added_domains from ids to pass to send email method and bulk create added_domains = Domain.objects.filter(id__in=added_domain_ids) member_of_a_different_org, _ = get_org_membership(portfolio, email, None) - send_domain_invitation_email( + if not send_domain_invitation_email( email=email, requestor=requestor, domains=added_domains, is_member_of_different_org=member_of_a_different_org, - ) + ): + messages.warning(self.request, "Could not send email confirmation to existing domain managers.") # Update existing invitations from CANCELED to INVITED existing_invitations = DomainInvitation.objects.filter(domain__in=added_domains, email=email) @@ -639,7 +756,7 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): """Add additional context data to the template.""" context = super().get_context_data(**kwargs) portfolio = self.request.session.get("portfolio") - context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission(portfolio) + context["has_edit_portfolio_permission"] = self.request.user.has_edit_portfolio_permission(portfolio) return context def get_object(self, queryset=None): @@ -779,12 +896,19 @@ class PortfolioAddMemberView(PortfolioMembersPermissionView, FormMixin): requested_email = form.cleaned_data["email"] requestor = self.request.user portfolio = form.cleaned_data["portfolio"] + is_admin_invitation = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in form.cleaned_data["roles"] requested_user = User.objects.filter(email=requested_email).first() permission_exists = UserPortfolioPermission.objects.filter(user=requested_user, portfolio=portfolio).exists() try: if not requested_user or not permission_exists: - send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=portfolio) + if not send_portfolio_invitation_email( + email=requested_email, + requestor=requestor, + portfolio=portfolio, + is_admin_invitation=is_admin_invitation, + ): + messages.warning(self.request, "Could not send email notification to existing organization admins.") portfolio_invitation = form.save() # if user exists for email, immediately retrieve portfolio invitation upon creation if requested_user is not None: @@ -807,7 +931,7 @@ class PortfolioAddMemberView(PortfolioMembersPermissionView, FormMixin): portfolio, exc_info=True, ) - messages.warning(self.request, "Could not send email invitation.") + messages.error(self.request, "Could not send organization invitation email.") elif isinstance(exception, MissingEmailError): messages.error(self.request, str(exception)) logger.error( @@ -816,4 +940,4 @@ class PortfolioAddMemberView(PortfolioMembersPermissionView, FormMixin): ) else: logger.warning("Could not send email invitation (Other Exception)", exc_info=True) - messages.warning(self.request, "Could not send email invitation.") + messages.warning(self.request, "Could not send portfolio email invitation.") diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index 694d1e205..1b9198c79 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -5,17 +5,20 @@ from django.views import View from django.shortcuts import render from django.contrib import admin from django.db.models import Avg, F + +from registrar.views.utility.mixins import DomainAndRequestsReportsPermission, PortfolioReportsPermission from .. import models import datetime from django.utils import timezone - +from django.contrib.admin.views.decorators import staff_member_required +from django.utils.decorators import method_decorator from registrar.utility import csv_export - import logging logger = logging.getLogger(__name__) +@method_decorator(staff_member_required, name="dispatch") class AnalyticsView(View): def get(self, request): thirty_days_ago = datetime.datetime.today() - datetime.timedelta(days=30) @@ -149,6 +152,7 @@ class AnalyticsView(View): return render(request, "admin/analytics.html", context) +@method_decorator(staff_member_required, name="dispatch") class ExportDataType(View): def get(self, request, *args, **kwargs): # match the CSV example with all the fields @@ -158,7 +162,7 @@ class ExportDataType(View): return response -class ExportDataTypeUser(View): +class ExportDataTypeUser(DomainAndRequestsReportsPermission, View): """Returns a domain report for a given user on the request""" def get(self, request, *args, **kwargs): @@ -169,7 +173,7 @@ class ExportDataTypeUser(View): return response -class ExportMembersPortfolio(View): +class ExportMembersPortfolio(PortfolioReportsPermission, View): """Returns a members report for a given portfolio""" def get(self, request, *args, **kwargs): @@ -197,17 +201,7 @@ class ExportMembersPortfolio(View): return response -class ExportDataTypeRequests(View): - """Returns a domain requests report for a given user on the request""" - - def get(self, request, *args, **kwargs): - response = HttpResponse(content_type="text/csv") - response["Content-Disposition"] = 'attachment; filename="domain-requests.csv"' - csv_export.DomainRequestDataType.export_data_to_csv(response, request=request) - - return response - - +@method_decorator(staff_member_required, name="dispatch") class ExportDataFull(View): def get(self, request, *args, **kwargs): # Smaller export based on 1 @@ -217,6 +211,7 @@ class ExportDataFull(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataFederal(View): def get(self, request, *args, **kwargs): # Federal only @@ -226,6 +221,7 @@ class ExportDataFederal(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDomainRequestDataFull(View): """Generates a downloaded report containing all Domain Requests (except started)""" @@ -237,6 +233,7 @@ class ExportDomainRequestDataFull(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataDomainsGrowth(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -249,6 +246,7 @@ class ExportDataDomainsGrowth(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataRequestsGrowth(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -261,6 +259,7 @@ class ExportDataRequestsGrowth(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataManagedDomains(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -272,6 +271,7 @@ class ExportDataManagedDomains(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataUnmanagedDomains(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") diff --git a/src/registrar/views/utility/invitation_helper.py b/src/registrar/views/utility/invitation_helper.py index 98c36b308..18c427940 100644 --- a/src/registrar/views/utility/invitation_helper.py +++ b/src/registrar/views/utility/invitation_helper.py @@ -3,7 +3,6 @@ from django.db import IntegrityError from registrar.models import PortfolioInvitation, User, UserPortfolioPermission from registrar.utility.email import EmailSendingError import logging - from registrar.utility.errors import ( AlreadyDomainInvitedError, AlreadyDomainManagerError, @@ -61,20 +60,19 @@ def get_requested_user(email): def handle_invitation_exceptions(request, exception, email): """Handle exceptions raised during the process.""" if isinstance(exception, EmailSendingError): - logger.warning(str(exception), exc_info=True) + logger.warning(exception, exc_info=True) messages.error(request, str(exception)) elif isinstance(exception, MissingEmailError): messages.error(request, str(exception)) - logger.error(str(exception), exc_info=True) + logger.error(exception, exc_info=True) elif isinstance(exception, OutsideOrgMemberError): messages.error(request, str(exception)) - logger.warning(str(exception), exc_info=True) elif isinstance(exception, AlreadyDomainManagerError): - messages.warning(request, str(exception)) + messages.error(request, str(exception)) elif isinstance(exception, AlreadyDomainInvitedError): - messages.warning(request, str(exception)) + messages.error(request, str(exception)) elif isinstance(exception, IntegrityError): - messages.warning(request, f"{email} is already a manager for this domain") + messages.error(request, f"{email} is already a manager for this domain") else: logger.warning("Could not send email invitation (Other Exception)", exc_info=True) - messages.warning(request, "Could not send email invitation.") + messages.error(request, "Could not send email invitation.") diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 236ef8696..8a4666372 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -153,6 +153,48 @@ class PermissionsLoginMixin(PermissionRequiredMixin): return super().handle_no_permission() +class DomainAndRequestsReportsPermission(PermissionsLoginMixin): + """Permission mixin for domain and requests csv downloads""" + + def has_permission(self): + """Check if this user has access to this domain. + + The user is in self.request.user and the domain needs to be looked + up from the domain's primary key in self.kwargs["pk"] + """ + + if not self.request.user.is_authenticated: + return False + + if self.request.user.is_restricted(): + return False + + return True + + +class PortfolioReportsPermission(PermissionsLoginMixin): + """Permission mixin for portfolio csv downloads""" + + def has_permission(self): + """Check if this user has access to this domain. + + The user is in self.request.user and the domain needs to be looked + up from the domain's primary key in self.kwargs["pk"] + """ + + if not self.request.user.is_authenticated: + return False + + if self.request.user.is_restricted(): + return False + + portfolio = self.request.session.get("portfolio") + if not self.request.user.has_view_members_portfolio_permission(portfolio): + return False + + return self.request.user.is_org_user(self.request) + + class DomainPermission(PermissionsLoginMixin): """Permission mixin that redirects to domain if user has access, otherwise 403""" @@ -192,7 +234,8 @@ class DomainPermission(PermissionsLoginMixin): def can_access_domain_via_portfolio(self, pk): """Most views should not allow permission to portfolio users. If particular views allow access to the domain pages, they will need to override - this function.""" + this function. + """ return False def in_editable_state(self, pk):