From 4607245acf37b0dbd98e0bac4124620089c43da6 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 2 Jul 2024 08:58:37 -0600 Subject: [PATCH 01/44] Rebuild DB workflow DRAFT --- .github/workflows/rebuild-db.yaml | 113 ++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 .github/workflows/rebuild-db.yaml diff --git a/.github/workflows/rebuild-db.yaml b/.github/workflows/rebuild-db.yaml new file mode 100644 index 000000000..3c75a5209 --- /dev/null +++ b/.github/workflows/rebuild-db.yaml @@ -0,0 +1,113 @@ +# This workflow can be run from the CLI +# gh workflow run rebuild-db.yaml -f environment=ENVIRONMENT +# OR +# cf run-task getgov-ENVIRONMENT --command 'python manage.py flush' --name flush +# cf run-task getgov-ENVIRONMENT --command 'python manage.py load' --name loaddata + +name: Rebuild database +run-name: Rebuild 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: + - staging + - development + - ag + - litterbox + - hotgov + - cb + - bob + - meoward + - backup + - ky + - es + - nl + - rh + - za + - gd + - rb + - ko + - ab + - rjm + - dk + +jobs: + connect-to-service: + runs-on: ubuntu-latest + env: + CF_USERNAME: CF_${{ github.event.inputs.environment }}_USERNAME + CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + - name: Set up Cloud Foundry CLI + run: | + sudo apt-get update + sudo apt-get install -y wget + wget -q -O cf-cli.tgz "https://packages.cloudfoundry.org/stable?release=linux64-binary&source=github" + tar -xzf cf-cli.tgz + sudo mv cf /usr/local/bin + + - name: Authenticate to Cloud Foundry + run: | + cf api # Replace with your CF API endpoint + cf auth ${{ secrets.CF_USERNAME }} ${{ secrets.CF_PASSWORD }} + + - name: Target organization and space + run: | + cf target -o cisa-dotgov -s nl + + - name: Connect to service + id: connect + run: | + cf connect-to-service -no-client getgov-nl getgov-nl-database > connection_info.txt + cat connection_info.txt + + - name: Extract connection details + id: extract + run: | + port=$(grep -oP 'port:\s*\K\d+' connection_info.txt) + username=$(grep -oP 'user:\s*\K\w+' connection_info.txt) + broker_name=$(grep -oP 'dbname:\s*\K\w+' connection_info.txt) + echo "::set-output name=port::$port" + echo "::set-output name=username::$username" + echo "::set-output name=broker_name::$broker_name" + + - name: Connect to PostgreSQL + run: | + psql -h localhost -p ${{ steps.extract.outputs.port }} -U ${{ steps.extract.outputs.username }} -d ${{ steps.extract.outputs.broker_name }} + env: + PGPASSWORD: ${{ secrets.PG_PASSWORD }} + + - name: Get table names + id: get_tables + run: | + tables=$(psql -h localhost -p ${{ steps.extract.outputs.port }} -U ${{ steps.extract.outputs.username }} -d ${{ steps.extract.outputs.broker_name }} -c "\dt" -t | awk '{print $3}') + echo "::set-output name=tables::$tables" + + - name: Drop all tables + run: | + for table in ${{ steps.get_tables.outputs.tables }} + do + psql -h localhost -p ${{ steps.extract.outputs.port }} -U ${{ steps.extract.outputs.username }} -d ${{ steps.extract.outputs.broker_name }} -c "DROP TABLE IF EXISTS $table CASCADE;" + done + env: + PGPASSWORD: ${{ secrets.PG_PASSWORD }} + + - name: Migrate + run: | + cf ssh getgov-nl -c "/tmp/lifecycle/shell ./manage.py migrate" + + - name: Run fixtures + run: | + cf ssh getgov-nl -c "/tmp/lifecycle/shell ./manage.py load" + + - name: Create cache table + run: | + cf ssh getgov-nl -c "/tmp/lifecycle/shell ./manage.py createcachetable" From b1e89a651bc65c5316672b9324436e492f44c991 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 2 Jul 2024 13:10:04 -0600 Subject: [PATCH 02/44] incremental update to test --- .github/workflows/rebuild-db.yaml | 53 ++++++++++++------------------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/.github/workflows/rebuild-db.yaml b/.github/workflows/rebuild-db.yaml index 3c75a5209..f84a6e0a7 100644 --- a/.github/workflows/rebuild-db.yaml +++ b/.github/workflows/rebuild-db.yaml @@ -1,8 +1,5 @@ # This workflow can be run from the CLI # gh workflow run rebuild-db.yaml -f environment=ENVIRONMENT -# OR -# cf run-task getgov-ENVIRONMENT --command 'python manage.py flush' --name flush -# cf run-task getgov-ENVIRONMENT --command 'python manage.py load' --name loaddata name: Rebuild database run-name: Rebuild database for ${{ github.event.inputs.environment }} @@ -14,7 +11,6 @@ on: type: choice description: Which environment should we flush and re-load data for? options: - - staging - development - ag - litterbox @@ -43,30 +39,23 @@ jobs: CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD steps: - - name: Checkout repository - uses: actions/checkout@v2 + # - name: Delete existing data for ${{ github.event.inputs.environment }} + # uses: cloud-gov/cg-cli-tools@main + # with: + # cf_username: ${{ secrets[env.CF_USERNAME] }} + # cf_password: ${{ secrets[env.CF_PASSWORD] }} + # cf_org: cisa-dotgov + # cf_space: ${{ github.event.inputs.environment }} + # cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py flush --no-input' --name flush" - - name: Set up Cloud Foundry CLI - run: | - sudo apt-get update - sudo apt-get install -y wget - wget -q -O cf-cli.tgz "https://packages.cloudfoundry.org/stable?release=linux64-binary&source=github" - tar -xzf cf-cli.tgz - sudo mv cf /usr/local/bin - - - name: Authenticate to Cloud Foundry - run: | - cf api # Replace with your CF API endpoint - cf auth ${{ secrets.CF_USERNAME }} ${{ secrets.CF_PASSWORD }} - - - name: Target organization and space - run: | - cf target -o cisa-dotgov -s nl + # - name: Target organization and space + # run: | + # cf target -o cisa-dotgov -s nl - name: Connect to service id: connect run: | - cf connect-to-service -no-client getgov-nl getgov-nl-database > connection_info.txt + cf connect-to-service -no-client getgov-${{ github.event.inputs.environment }} getgov-${{ github.event.inputs.environment }}-database > connection_info.txt cat connection_info.txt - name: Extract connection details @@ -100,14 +89,14 @@ jobs: env: PGPASSWORD: ${{ secrets.PG_PASSWORD }} - - name: Migrate - run: | - cf ssh getgov-nl -c "/tmp/lifecycle/shell ./manage.py migrate" + # - name: Migrate + # run: | + # cf ssh getgov-${{ github.event.inputs.environment }} -c "/tmp/lifecycle/shell ./manage.py migrate" - - name: Run fixtures - run: | - cf ssh getgov-nl -c "/tmp/lifecycle/shell ./manage.py load" + # - name: Run fixtures + # run: | + # cf ssh getgov-${{ github.event.inputs.environment }} -c "/tmp/lifecycle/shell ./manage.py load" - - name: Create cache table - run: | - cf ssh getgov-nl -c "/tmp/lifecycle/shell ./manage.py createcachetable" + # - name: Create cache table + # run: | + # cf ssh getgov-${{ github.event.inputs.environment }} -c "/tmp/lifecycle/shell ./manage.py createcachetable" From a4c06b9cdf8a14b1e0e18e02d3a180192c65705f Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 19 Jul 2024 18:26:00 -0600 Subject: [PATCH 03/44] added drop table script --- .../management/commands/drop_tables.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 src/registrar/management/commands/drop_tables.py diff --git a/src/registrar/management/commands/drop_tables.py b/src/registrar/management/commands/drop_tables.py new file mode 100644 index 000000000..8ef1e9584 --- /dev/null +++ b/src/registrar/management/commands/drop_tables.py @@ -0,0 +1,39 @@ +import logging +from django.conf import settings +from django.core.management import BaseCommand +from django.apps import apps +from django.db import connection, transaction + +from registrar.management.commands.utility.terminal_helper import TerminalHelper + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = 'Drops all tables in the database' + + def handle(self, **options): + """Delete all rows from a list of tables""" + + if settings.IS_PRODUCTION: + logger.error("drop_tables cannot be run in production") + return + + self.print_tables() + logger.info(self.style.WARNING('Dropping all tables...')) + with connection.cursor() as cursor: + cursor.execute("DROP SCHEMA public CASCADE;") + cursor.execute("CREATE SCHEMA public;") + logger.info(self.style.SUCCESS('All tables dropped.')) + + def print_tables(self): + logger.info(self.style.WARNING('Fetching table names...')) + with connection.cursor() as cursor: + cursor.execute("SELECT table_name FROM information_schema.tables WHERE table_schema='public'") + table_names = cursor.fetchall() + if table_names: + logger.info(self.style.NOTICE('Tables in the database:')) + for name in table_names: + logger.info(name[0]) + else: + logger.info(self.style.WARNING('No tables found.')) \ No newline at end of file From 9b079811c8a489c6f74f990742c75fefe11cde91 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Jul 2024 14:31:39 -0600 Subject: [PATCH 04/44] Update drop_tables.py --- .../management/commands/drop_tables.py | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/registrar/management/commands/drop_tables.py b/src/registrar/management/commands/drop_tables.py index 8ef1e9584..1a5a40ee0 100644 --- a/src/registrar/management/commands/drop_tables.py +++ b/src/registrar/management/commands/drop_tables.py @@ -19,21 +19,20 @@ class Command(BaseCommand): logger.error("drop_tables cannot be run in production") return - self.print_tables() logger.info(self.style.WARNING('Dropping all tables...')) - with connection.cursor() as cursor: - cursor.execute("DROP SCHEMA public CASCADE;") - cursor.execute("CREATE SCHEMA public;") - logger.info(self.style.SUCCESS('All tables dropped.')) - - def print_tables(self): - logger.info(self.style.WARNING('Fetching table names...')) with connection.cursor() as cursor: cursor.execute("SELECT table_name FROM information_schema.tables WHERE table_schema='public'") table_names = cursor.fetchall() if table_names: - logger.info(self.style.NOTICE('Tables in the database:')) - for name in table_names: - logger.info(name[0]) + try: + logger.info(self.style.NOTICE('Dropping tables in the database:')) + for name in table_names: + name_as_str = name[0] + logger.info(f"Dropping {name_as_str}") + cursor.execute(f"DROP TABLE {name_as_str} CASCADE;") + except Exception as err: + logger.error(f"Could not drop tables from DB: {err}") + else: + logger.info(self.style.SUCCESS('All tables dropped.')) else: - logger.info(self.style.WARNING('No tables found.')) \ No newline at end of file + logger.info(self.style.WARNING('No tables found.')) From 9b65879a4f6da779d707dacda55f28d6440673b1 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 22 Jul 2024 16:38:25 -0600 Subject: [PATCH 05/44] updated workflow --- .github/workflows/rebuild-db.yaml | 95 ++++++++++++------------------- 1 file changed, 35 insertions(+), 60 deletions(-) diff --git a/.github/workflows/rebuild-db.yaml b/.github/workflows/rebuild-db.yaml index f84a6e0a7..fe47b43a8 100644 --- a/.github/workflows/rebuild-db.yaml +++ b/.github/workflows/rebuild-db.yaml @@ -31,72 +31,47 @@ on: - rjm - dk + jobs: - connect-to-service: + reset-db: runs-on: ubuntu-latest env: CF_USERNAME: CF_${{ github.event.inputs.environment }}_USERNAME CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD - steps: - # - name: Delete existing data for ${{ github.event.inputs.environment }} - # uses: cloud-gov/cg-cli-tools@main - # with: - # cf_username: ${{ secrets[env.CF_USERNAME] }} - # cf_password: ${{ secrets[env.CF_PASSWORD] }} - # cf_org: cisa-dotgov - # cf_space: ${{ github.event.inputs.environment }} - # cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py flush --no-input' --name flush" + - name: Drop Tables for ${{ github.event.inputs.environment }} + uses: cloud-gov/cg-cli-tools@main + with: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + cf_org: cisa-dotgov + cf_space: ${{ github.event.inputs.environment }} + cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py drop_tables --no-input' --name flush" + + - name: Run Django migrations for ${{ github.event.inputs.environment }} + uses: cloud-gov/cg-cli-tools@main + with: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + cf_org: cisa-dotgov + cf_space: ${{ github.event.inputs.environment }} + cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py migrate' --name migrate" - # - name: Target organization and space - # run: | - # cf target -o cisa-dotgov -s nl + - name: Run fixtures for ${{ github.event.inputs.environment }} + uses: cloud-gov/cg-cli-tools@main + with: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + cf_org: cisa-dotgov + cf_space: ${{ github.event.inputs.environment }} + cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py load' --name load" - - name: Connect to service - id: connect - run: | - cf connect-to-service -no-client getgov-${{ github.event.inputs.environment }} getgov-${{ github.event.inputs.environment }}-database > connection_info.txt - cat connection_info.txt + - name: Create cache table for ${{ github.event.inputs.environment }} + uses: cloud-gov/cg-cli-tools@main + with: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + cf_org: cisa-dotgov + cf_space: ${{ github.event.inputs.environment }} + cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py createcachetable' --name createcachetable" - - name: Extract connection details - id: extract - run: | - port=$(grep -oP 'port:\s*\K\d+' connection_info.txt) - username=$(grep -oP 'user:\s*\K\w+' connection_info.txt) - broker_name=$(grep -oP 'dbname:\s*\K\w+' connection_info.txt) - echo "::set-output name=port::$port" - echo "::set-output name=username::$username" - echo "::set-output name=broker_name::$broker_name" - - - name: Connect to PostgreSQL - run: | - psql -h localhost -p ${{ steps.extract.outputs.port }} -U ${{ steps.extract.outputs.username }} -d ${{ steps.extract.outputs.broker_name }} - env: - PGPASSWORD: ${{ secrets.PG_PASSWORD }} - - - name: Get table names - id: get_tables - run: | - tables=$(psql -h localhost -p ${{ steps.extract.outputs.port }} -U ${{ steps.extract.outputs.username }} -d ${{ steps.extract.outputs.broker_name }} -c "\dt" -t | awk '{print $3}') - echo "::set-output name=tables::$tables" - - - name: Drop all tables - run: | - for table in ${{ steps.get_tables.outputs.tables }} - do - psql -h localhost -p ${{ steps.extract.outputs.port }} -U ${{ steps.extract.outputs.username }} -d ${{ steps.extract.outputs.broker_name }} -c "DROP TABLE IF EXISTS $table CASCADE;" - done - env: - PGPASSWORD: ${{ secrets.PG_PASSWORD }} - - # - name: Migrate - # run: | - # cf ssh getgov-${{ github.event.inputs.environment }} -c "/tmp/lifecycle/shell ./manage.py migrate" - - # - name: Run fixtures - # run: | - # cf ssh getgov-${{ github.event.inputs.environment }} -c "/tmp/lifecycle/shell ./manage.py load" - - # - name: Create cache table - # run: | - # cf ssh getgov-${{ github.event.inputs.environment }} -c "/tmp/lifecycle/shell ./manage.py createcachetable" From d4f26b8d061f162dddd1cdcae28b6276770d1fb3 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 24 Jul 2024 14:18:12 -0600 Subject: [PATCH 06/44] linted and corrected command --- .github/workflows/rebuild-db.yaml | 2 +- src/registrar/management/commands/drop_tables.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/rebuild-db.yaml b/.github/workflows/rebuild-db.yaml index fe47b43a8..3232bbe96 100644 --- a/.github/workflows/rebuild-db.yaml +++ b/.github/workflows/rebuild-db.yaml @@ -46,7 +46,7 @@ jobs: cf_password: ${{ secrets[env.CF_PASSWORD] }} cf_org: cisa-dotgov cf_space: ${{ github.event.inputs.environment }} - cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py drop_tables --no-input' --name flush" + cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py drop_tables' --name flush" - name: Run Django migrations for ${{ github.event.inputs.environment }} uses: cloud-gov/cg-cli-tools@main diff --git a/src/registrar/management/commands/drop_tables.py b/src/registrar/management/commands/drop_tables.py index 1a5a40ee0..e4d18f576 100644 --- a/src/registrar/management/commands/drop_tables.py +++ b/src/registrar/management/commands/drop_tables.py @@ -1,10 +1,6 @@ import logging from django.conf import settings from django.core.management import BaseCommand -from django.apps import apps -from django.db import connection, transaction - -from registrar.management.commands.utility.terminal_helper import TerminalHelper logger = logging.getLogger(__name__) From 02423cc6313f71b229f10eda0c19991ba6a32353 Mon Sep 17 00:00:00 2001 From: katypies Date: Wed, 24 Jul 2024 14:58:42 -0600 Subject: [PATCH 07/44] Update link to point to new section for updating contact information, including email address --- src/registrar/templates/includes/profile_form.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/includes/profile_form.html b/src/registrar/templates/includes/profile_form.html index 966b92b01..dbc0415a9 100644 --- a/src/registrar/templates/includes/profile_form.html +++ b/src/registrar/templates/includes/profile_form.html @@ -30,11 +30,11 @@ {% input_with_errors form.title %} - {% public_site_url "help/account-management/#get-help-with-login.gov" as login_help_url %} + {% public_site_url "help/account-management/#update-your-contact-information" as login_help_url %} {% with link_href=login_help_url %} - {% with sublabel_text="We recommend using your work email for your .gov account. If the wrong email is displayed below, you’ll need to update your Login.gov account and log back in. Get help with your Login.gov account." %} - {% with link_text="Get help with your Login.gov account" %} + {% with sublabel_text="We recommend using your work email for your .gov account. If the wrong email is displayed below, you’ll need to update your Login.gov account and log back in. Get help with updating your contact information." %} + {% with link_text="Get help with updating your contact information" %} {% with target_blank=True %} {% with do_not_show_max_chars=True %} {% input_with_errors form.email %} From 511f545f26527a29811117ee4117aa33a4a5ff39 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 30 Jul 2024 14:39:50 -0600 Subject: [PATCH 08/44] Clamp to perms --- src/registrar/models/user.py | 5 +++++ src/registrar/utility/csv_export.py | 11 +++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index b135e30c7..403eea4d2 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -301,6 +301,11 @@ class User(AbstractUser): or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) # or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_REQUESTS) ) + + + def has_view_all_domains_permission(self): + """Determines if the current user can view all available domains in a given portfolio""" + return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) @classmethod def needs_identity_verification(cls, email, uuid): diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 5fbd255aa..1a1bf7b34 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -578,8 +578,15 @@ class DomainDataTypeUser(DomainDataType): # Return nothing return Q(id__in=[]) - user_domain_roles = UserDomainRole.objects.filter(user=request.user) - domain_ids = user_domain_roles.values_list("domain_id", flat=True) + if ( + request.user.has_base_portfolio_permission() and + request.user.has_view_all_domains_permission() + ): + models = DomainInformation.objects.filter(portfolio=request.user.portfolio) + else: + models = UserDomainRole.objects.filter(user=request.user) + + domain_ids = models.values_list("domain_id", flat=True) return Q(domain__id__in=domain_ids) From 558207d1aae7adc17f1988094867f445f08935d0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 30 Jul 2024 14:43:25 -0600 Subject: [PATCH 09/44] Update user.py --- src/registrar/models/user.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 403eea4d2..6b8afcac9 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -301,8 +301,7 @@ class User(AbstractUser): or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) # or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_REQUESTS) ) - - + def has_view_all_domains_permission(self): """Determines if the current user can view all available domains in a given portfolio""" return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) From 1c7cf928d1ffa9665af5891b489609b59ab82f2a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 30 Jul 2024 15:25:39 -0600 Subject: [PATCH 10/44] basic tests --- src/registrar/tests/common.py | 2 + src/registrar/tests/test_reports.py | 74 +++++++++++++++++++++++++++++ src/registrar/utility/csv_export.py | 1 + 3 files changed, 77 insertions(+) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index a4c3e2ef4..d9c9001ce 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -27,6 +27,7 @@ from registrar.models import ( PublicContact, Domain, FederalAgency, + Portfolio, ) from epplibwrapper import ( commands, @@ -793,6 +794,7 @@ class MockDb(TestCase): UserDomainRole.objects.all().delete() User.objects.all().delete() DomainInvitation.objects.all().delete() + Portfolio.objects.all().delete() cls.federal_agency_1.delete() cls.federal_agency_2.delete() diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 74b84834e..e18439d88 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -6,6 +6,7 @@ from registrar.models import ( Domain, UserDomainRole, ) +from registrar.models import Portfolio, DomainInformation, User from registrar.utility.csv_export import ( DomainDataFull, DomainDataType, @@ -32,6 +33,7 @@ from registrar.utility.s3_bucket import S3ClientError, S3ClientErrorCodes # typ from django.utils import timezone from api.tests.common import less_console_noise_decorator from .common import MockDbForSharedTests, MockDbForIndividualTests, MockEppLib, less_console_noise, get_time_aware_date +from waffle.testutils import override_flag class CsvReportsTest(MockDbForSharedTests): @@ -311,6 +313,78 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.maxDiff = None self.assertEqual(csv_content, expected_content) + @less_console_noise_decorator + @override_flag("profile_feature", active=True) + def test_domain_data_type_user_with_portfolio(self): + """Tests DomainDataTypeUser export with portfolio permissions""" + + # Create a portfolio and assign it to the user + portfolio = Portfolio.objects.create(creator=self.user, organization_name="Test Portfolio") + self.user.portfolio = portfolio + self.user.save() + + UserDomainRole.objects.create(user=self.user, domain=self.domain_2) + UserDomainRole.objects.filter(user=self.user, domain=self.domain_1).delete() + UserDomainRole.objects.filter(user=self.user, domain=self.domain_3).delete() + + # Add portfolios to the first and third domains + self.domain_1.domain_info.portfolio = portfolio + self.domain_3.domain_info.portfolio = portfolio + + self.domain_1.domain_info.save() + self.domain_3.domain_info.save() + + # Set up user permissions + self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + self.user.save() + self.user.refresh_from_db() + + # Create a request object + factory = RequestFactory() + request = factory.get("/") + request.user = self.user + + # Get the csv content + csv_content = self._run_domain_data_type_user_export(request) + + # We expect only domains associated with the user's portfolio + self.assertIn(self.domain_1.name, csv_content) + self.assertIn(self.domain_3.name, csv_content) + self.assertNotIn(self.domain_2.name, csv_content) + + # Test the output for readonly admin + self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] + self.user.save() + + self.assertIn(self.domain_1.name, csv_content) + self.assertIn(self.domain_3.name, csv_content) + self.assertNotIn(self.domain_2.name, csv_content) + + # Get the csv content + csv_content = self._run_domain_data_type_user_export(request) + self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + self.user.save() + + self.assertNotIn(self.domain_1.name, csv_content) + self.assertNotIn(self.domain_3.name, csv_content) + self.assertIn(self.domain_2.name, csv_content) + + # Get the csv content + csv_content = self._run_domain_data_type_user_export(request) + + def _run_domain_data_type_user_export(self, request): + """Helper function to run the export_data_to_csv function on DomainDataTypeUser""" + # Create a CSV file in memory + csv_file = StringIO() + # Call the export functions + DomainDataTypeUser.export_data_to_csv(csv_file, request=request) + # Reset the CSV file's position to the beginning + csv_file.seek(0) + # Read the content into a variable + csv_content = csv_file.read() + + return csv_content + @less_console_noise_decorator def test_domain_data_full(self): """Shows security contacts, filtered by state""" diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 1a1bf7b34..a18d74bae 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -582,6 +582,7 @@ class DomainDataTypeUser(DomainDataType): request.user.has_base_portfolio_permission() and request.user.has_view_all_domains_permission() ): + # Question: should we also include all domains in UserDomainRole as well? models = DomainInformation.objects.filter(portfolio=request.user.portfolio) else: models = UserDomainRole.objects.filter(user=request.user) From f3489ae5714ea6aa91f631bba28bb1178a1ab30b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 31 Jul 2024 08:43:49 -0600 Subject: [PATCH 11/44] Cleanup --- src/registrar/tests/common.py | 1 - src/registrar/tests/test_reports.py | 10 ++++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index d9c9001ce..69e3fa174 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -794,7 +794,6 @@ class MockDb(TestCase): UserDomainRole.objects.all().delete() User.objects.all().delete() DomainInvitation.objects.all().delete() - Portfolio.objects.all().delete() cls.federal_agency_1.delete() cls.federal_agency_2.delete() diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index e18439d88..06e6f264d 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -361,16 +361,18 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.assertNotIn(self.domain_2.name, csv_content) # Get the csv content - csv_content = self._run_domain_data_type_user_export(request) self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_MEMBER] self.user.save() + csv_content = self._run_domain_data_type_user_export(request) + self.assertNotIn(self.domain_1.name, csv_content) self.assertNotIn(self.domain_3.name, csv_content) self.assertIn(self.domain_2.name, csv_content) - - # Get the csv content - csv_content = self._run_domain_data_type_user_export(request) + self.domain_1.delete() + self.domain_2.delete() + self.domain_3.delete() + portfolio.delete() def _run_domain_data_type_user_export(self, request): """Helper function to run the export_data_to_csv function on DomainDataTypeUser""" From 280f7cbc1b9d60a0fe15d67e639d8581281576e9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 31 Jul 2024 11:02:18 -0600 Subject: [PATCH 12/44] Add logic --- src/registrar/models/user.py | 5 +++++ src/registrar/templates/home.html | 2 +- src/registrar/templates/includes/domains_table.html | 2 ++ src/registrar/templates/portfolio_domains.html | 2 +- src/registrar/views/index.py | 1 + src/registrar/views/portfolios.py | 1 + 6 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index b135e30c7..e7444d5ea 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -424,3 +424,8 @@ class User(AbstractUser): def is_org_user(self, request): has_organization_feature_flag = flag_is_active(request, "organization_feature") return has_organization_feature_flag and self.has_base_portfolio_permission() + + def user_domain_count(self): + """Returns the number of domains associated with this user through UserDomainRole""" + available_domains = UserDomainRole.objects.filter(user=self, domain__isnull=False) + return available_domains.count() diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index b79b69ebc..1392885e9 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -29,7 +29,7 @@

- {% include "includes/domains_table.html" %} + {% include "includes/domains_table.html" user_domain_count=user_domain_count %} {% include "includes/domain_requests_table.html" %} diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 309702d26..f373f4459 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -35,6 +35,7 @@ + {% if user_domains_count and user_domains_count > 0 %} + {% endif %} {% if portfolio %} diff --git a/src/registrar/templates/portfolio_domains.html b/src/registrar/templates/portfolio_domains.html index ede7886e6..84bbc1cf6 100644 --- a/src/registrar/templates/portfolio_domains.html +++ b/src/registrar/templates/portfolio_domains.html @@ -6,5 +6,5 @@ {% block portfolio_content %}

Domains

- {% include "includes/domains_table.html" with portfolio=portfolio %} + {% include "includes/domains_table.html" with portfolio=portfolio user_domain_count=user_domain_count %} {% endblock %} diff --git a/src/registrar/views/index.py b/src/registrar/views/index.py index 498434dca..9483cdd52 100644 --- a/src/registrar/views/index.py +++ b/src/registrar/views/index.py @@ -8,5 +8,6 @@ def index(request): if request.user.is_authenticated: # This controls the creation of a new domain request in the wizard request.session["new_request"] = True + context["user_domain_count"] = request.user.user_domain_count() return render(request, "home.html", context) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index abd9648ba..c9f2d5ec1 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -29,6 +29,7 @@ class PortfolioDomainsView(PortfolioDomainsPermissionView, View): context["has_organization_feature_flag"] = flag_is_active(request, "organization_feature") portfolio = get_object_or_404(Portfolio, id=portfolio_id) context["portfolio"] = portfolio + context["user_domain_count"] = self.request.user.user_domain_count() return render(request, "portfolio_domains.html", context) From 49d81f4c5823a760451808956e59eb9f7f1955ed Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 31 Jul 2024 11:15:57 -0600 Subject: [PATCH 13/44] add check for if the user doesn't have any domains --- src/registrar/templates/home.html | 2 +- src/registrar/templates/includes/domains_table.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 1392885e9..63924bc1d 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -29,7 +29,7 @@

- {% include "includes/domains_table.html" user_domain_count=user_domain_count %} + {% include "includes/domains_table.html" with user_domain_count=user_domain_count %} {% include "includes/domain_requests_table.html" %} diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index f373f4459..d4fafaed8 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -35,7 +35,7 @@ - {% if user_domains_count and user_domains_count > 0 %} + {% if user_domain_count and user_domain_count > 0 %}
From 117c7e0c42a84998c1cfa8608c403bb3665c87d4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 31 Jul 2024 11:55:50 -0600 Subject: [PATCH 14/44] Rebase logic --- src/registrar/models/user.py | 13 ++++++++----- src/registrar/utility/csv_export.py | 13 ++----------- src/registrar/views/index.py | 2 +- src/registrar/views/portfolios.py | 2 +- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 61a1a39fc..e87fb255a 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -4,6 +4,7 @@ from django.contrib.auth.models import AbstractUser from django.db import models from django.db.models import Q +from registrar.models.domain_information import DomainInformation from registrar.models.user_domain_role import UserDomainRole from .domain_invitation import DomainInvitation @@ -428,8 +429,10 @@ class User(AbstractUser): def is_org_user(self, request): has_organization_feature_flag = flag_is_active(request, "organization_feature") return has_organization_feature_flag and self.has_base_portfolio_permission() - - def user_domain_count(self): - """Returns the number of domains associated with this user through UserDomainRole""" - available_domains = UserDomainRole.objects.filter(user=self, domain__isnull=False) - return available_domains.count() + + def get_user_domain_ids(self): + """Returns either the domains ids associated with this user on UserDomainRole or Portfolio""" + if self.has_base_portfolio_permission() and self.has_view_all_domains_permission(): + return DomainInformation.objects.filter(portfolio=self.portfolio).values_list("domain_id", flat=True) + else: + return UserDomainRole.objects.filter(user=self).values_list("domain_id", flat=True) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index a18d74bae..78ced6518 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -577,18 +577,9 @@ class DomainDataTypeUser(DomainDataType): if request is None or not hasattr(request, "user") or not request.user: # Return nothing return Q(id__in=[]) - - if ( - request.user.has_base_portfolio_permission() and - request.user.has_view_all_domains_permission() - ): - # Question: should we also include all domains in UserDomainRole as well? - models = DomainInformation.objects.filter(portfolio=request.user.portfolio) else: - models = UserDomainRole.objects.filter(user=request.user) - - domain_ids = models.values_list("domain_id", flat=True) - return Q(domain__id__in=domain_ids) + # Get all domains the user is associated with + return Q(domain__id__in=request.user.get_user_domain_ids()) class DomainDataFull(DomainExport): diff --git a/src/registrar/views/index.py b/src/registrar/views/index.py index 9483cdd52..0d74a81cc 100644 --- a/src/registrar/views/index.py +++ b/src/registrar/views/index.py @@ -8,6 +8,6 @@ def index(request): if request.user.is_authenticated: # This controls the creation of a new domain request in the wizard request.session["new_request"] = True - context["user_domain_count"] = request.user.user_domain_count() + context["user_domain_count"] = request.user.get_user_domain_ids().count() return render(request, "home.html", context) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index c9f2d5ec1..7421487a0 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -29,7 +29,7 @@ class PortfolioDomainsView(PortfolioDomainsPermissionView, View): context["has_organization_feature_flag"] = flag_is_active(request, "organization_feature") portfolio = get_object_or_404(Portfolio, id=portfolio_id) context["portfolio"] = portfolio - context["user_domain_count"] = self.request.user.user_domain_count() + context["user_domain_count"] = self.request.user.get_user_domain_ids().count() return render(request, "portfolio_domains.html", context) From 3fafaf2d2add6a33de55c03277509537c9c47efc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:15:14 -0600 Subject: [PATCH 15/44] Update domains_json to reflect the correct data --- src/registrar/views/domains_json.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py index 3b3cae2c7..1213244c6 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -14,8 +14,7 @@ def get_domains_json(request): """Given the current request, get all domains that are associated with the UserDomainRole object""" - user_domain_roles = UserDomainRole.objects.filter(user=request.user).select_related("domain_info__sub_organization") - domain_ids = user_domain_roles.values_list("domain_id", flat=True) + domain_ids = request.user.get_user_domain_ids() objects = Domain.objects.filter(id__in=domain_ids) unfiltered_total = objects.count() From 7eba6b721d153d515dfda62628431148fcfff564 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:20:43 -0600 Subject: [PATCH 16/44] Linting cleanup --- src/registrar/tests/common.py | 1 - src/registrar/tests/test_reports.py | 2 +- src/registrar/views/domains_json.py | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 69e3fa174..a4c3e2ef4 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -27,7 +27,6 @@ from registrar.models import ( PublicContact, Domain, FederalAgency, - Portfolio, ) from epplibwrapper import ( commands, diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 06e6f264d..5b2112f4f 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -6,7 +6,7 @@ from registrar.models import ( Domain, UserDomainRole, ) -from registrar.models import Portfolio, DomainInformation, User +from registrar.models import Portfolio, User from registrar.utility.csv_export import ( DomainDataFull, DomainDataType, diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py index 1213244c6..2f0d33fc2 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -1,7 +1,7 @@ import logging from django.http import JsonResponse from django.core.paginator import Paginator -from registrar.models import UserDomainRole, Domain +from registrar.models import Domain from django.contrib.auth.decorators import login_required from django.urls import reverse from django.db.models import Q @@ -12,7 +12,7 @@ logger = logging.getLogger(__name__) @login_required def get_domains_json(request): """Given the current request, - get all domains that are associated with the UserDomainRole object""" + get all domains that are associated with the User object""" domain_ids = request.user.get_user_domain_ids() From 7297446a82a4747d31a946f8b23a27bd156729b4 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 31 Jul 2024 22:21:27 -0600 Subject: [PATCH 17/44] Remove abandoned rebuild-db workflow and add instructions for reset-db workflow --- .github/workflows/rebuild-db.yaml | 77 ------------------- .github/workflows/reset-db.yaml | 12 +++ .../management/commands/drop_tables.py | 34 -------- 3 files changed, 12 insertions(+), 111 deletions(-) delete mode 100644 .github/workflows/rebuild-db.yaml delete mode 100644 src/registrar/management/commands/drop_tables.py diff --git a/.github/workflows/rebuild-db.yaml b/.github/workflows/rebuild-db.yaml deleted file mode 100644 index 3232bbe96..000000000 --- a/.github/workflows/rebuild-db.yaml +++ /dev/null @@ -1,77 +0,0 @@ -# This workflow can be run from the CLI -# gh workflow run rebuild-db.yaml -f environment=ENVIRONMENT - -name: Rebuild database -run-name: Rebuild 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: - - development - - 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 - steps: - - name: Drop Tables for ${{ github.event.inputs.environment }} - uses: cloud-gov/cg-cli-tools@main - with: - cf_username: ${{ secrets[env.CF_USERNAME] }} - cf_password: ${{ secrets[env.CF_PASSWORD] }} - cf_org: cisa-dotgov - cf_space: ${{ github.event.inputs.environment }} - cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py drop_tables' --name flush" - - - name: Run Django migrations for ${{ github.event.inputs.environment }} - uses: cloud-gov/cg-cli-tools@main - with: - cf_username: ${{ secrets[env.CF_USERNAME] }} - cf_password: ${{ secrets[env.CF_PASSWORD] }} - cf_org: cisa-dotgov - cf_space: ${{ github.event.inputs.environment }} - cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py migrate' --name migrate" - - - name: Run fixtures for ${{ github.event.inputs.environment }} - uses: cloud-gov/cg-cli-tools@main - with: - cf_username: ${{ secrets[env.CF_USERNAME] }} - cf_password: ${{ secrets[env.CF_PASSWORD] }} - cf_org: cisa-dotgov - cf_space: ${{ github.event.inputs.environment }} - cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py load' --name load" - - - name: Create cache table for ${{ github.event.inputs.environment }} - uses: cloud-gov/cg-cli-tools@main - with: - cf_username: ${{ secrets[env.CF_USERNAME] }} - cf_password: ${{ secrets[env.CF_PASSWORD] }} - cf_org: cisa-dotgov - cf_space: ${{ github.event.inputs.environment }} - cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py createcachetable' --name createcachetable" - diff --git a/.github/workflows/reset-db.yaml b/.github/workflows/reset-db.yaml index 49e4b5e5f..f4af8bc12 100644 --- a/.github/workflows/reset-db.yaml +++ b/.github/workflows/reset-db.yaml @@ -4,6 +4,18 @@ # cf run-task getgov-ENVIRONMENT --command 'python manage.py flush' --name flush # cf run-task getgov-ENVIRONMENT --command 'python manage.py load' --name loaddata +# IMPORTANT NOTE: +# When running this workflow, it is important to grab the right set of migrations +# for your target sandbox. For example, if you just recently pushed new migrations +# from SOURCE_BRANCH to TARGET_SANDBOX, you must use that same SOURCE_BRANCH when +# running this workflow (NOT main) or else your sandbox will generate an error +# due to a db model mismatch. +# +# This means in the Github workflow settings you would select the following inputs: +# +# "Use workflow from": SOURCE_BRANCH +# "Which environment should we flush and re-load data for?"": TARGET_SANDBOX + name: Reset database run-name: Reset database for ${{ github.event.inputs.environment }} diff --git a/src/registrar/management/commands/drop_tables.py b/src/registrar/management/commands/drop_tables.py deleted file mode 100644 index e4d18f576..000000000 --- a/src/registrar/management/commands/drop_tables.py +++ /dev/null @@ -1,34 +0,0 @@ -import logging -from django.conf import settings -from django.core.management import BaseCommand - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - help = 'Drops all tables in the database' - - def handle(self, **options): - """Delete all rows from a list of tables""" - - if settings.IS_PRODUCTION: - logger.error("drop_tables cannot be run in production") - return - - logger.info(self.style.WARNING('Dropping all tables...')) - with connection.cursor() as cursor: - cursor.execute("SELECT table_name FROM information_schema.tables WHERE table_schema='public'") - table_names = cursor.fetchall() - if table_names: - try: - logger.info(self.style.NOTICE('Dropping tables in the database:')) - for name in table_names: - name_as_str = name[0] - logger.info(f"Dropping {name_as_str}") - cursor.execute(f"DROP TABLE {name_as_str} CASCADE;") - except Exception as err: - logger.error(f"Could not drop tables from DB: {err}") - else: - logger.info(self.style.SUCCESS('All tables dropped.')) - else: - logger.info(self.style.WARNING('No tables found.')) From ffe2ddebb4f831fe8d701b42be72a7436c9566a6 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 31 Jul 2024 22:33:37 -0600 Subject: [PATCH 18/44] wording adjustment --- .github/workflows/reset-db.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/reset-db.yaml b/.github/workflows/reset-db.yaml index f4af8bc12..50c83895a 100644 --- a/.github/workflows/reset-db.yaml +++ b/.github/workflows/reset-db.yaml @@ -5,11 +5,11 @@ # cf run-task getgov-ENVIRONMENT --command 'python manage.py load' --name loaddata # IMPORTANT NOTE: -# When running this workflow, it is important to grab the right set of migrations +# When running this workflow, it is important to use the right source branch # for your target sandbox. For example, if you just recently pushed new migrations # from SOURCE_BRANCH to TARGET_SANDBOX, you must use that same SOURCE_BRANCH when # running this workflow (NOT main) or else your sandbox will generate an error -# due to a db model mismatch. +# due to a db mismatch. # # This means in the Github workflow settings you would select the following inputs: # From 8becad81866dccf67314fcfd99340a51e3dc65ab Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 1 Aug 2024 08:58:38 -0600 Subject: [PATCH 19/44] Revert "Merge branch 'main' into za/2348-csv-export-org-member-domain-export" This reverts commit 4118bf17f9d2bce59694db6786ec07393bf3b821, reversing changes made to 7eba6b721d153d515dfda62628431148fcfff564. --- .../ISSUE_TEMPLATE/developer-onboarding.md | 49 ++-------- src/registrar/assets/js/get-gov.js | 11 +-- ...r_user_portfolio_additional_permissions.py | 38 -------- src/registrar/models/user.py | 29 ++++-- .../includes/domain_requests_table.html | 1 + .../templates/includes/domains_table.html | 14 ++- src/registrar/tests/test_models.py | 9 ++ .../tests/test_views_domains_json.py | 90 +------------------ src/registrar/views/domains_json.py | 34 ++----- 9 files changed, 54 insertions(+), 221 deletions(-) delete mode 100644 src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py diff --git a/.github/ISSUE_TEMPLATE/developer-onboarding.md b/.github/ISSUE_TEMPLATE/developer-onboarding.md index a0825ab52..d63cf2f94 100644 --- a/.github/ISSUE_TEMPLATE/developer-onboarding.md +++ b/.github/ISSUE_TEMPLATE/developer-onboarding.md @@ -14,29 +14,13 @@ assignees: abroddrick ## Installation -There are several tools we use locally that you will need to have. - -- [ ] [Cloudfoundry CLI](https://docs.cloudfoundry.org/cf-cli/install-go-cli.html#pkg-mac) Note: If you are on Windows the cli will be under `cf8` or `cf7` depending on which version you install. +There are several tools we use locally that you will need to have. +- [ ] [Install the cf CLI v7](https://docs.cloudfoundry.org/cf-cli/install-go-cli.html#pkg-mac) for the ability to deploy - If you are using Windows, installation information can be found [here](https://github.com/cloudfoundry/cli/wiki/V8-CLI-Installation-Guide#installers-and-compressed-binaries) - Alternatively, for Windows, [consider using chocolately](https://community.chocolatey.org/packages/cloudfoundry-cli/7.2.0) -- [ ] [GPG](https://gnupg.org/download/) - - Make sure you have `gpg` >2.1.7. Run `gpg --version` to check. If not, [install gnupg](https://formulae.brew.sh/formula/gnupg) - - This may not work on DHS devices. Alternatively, you can [use ssh keys](#setting-up-commit-signing-with-ssh) instead. -- [ ] Docker Community Edition* -- [ ] Git* -- [ ] VSCode (our preferred editor)* -- [ ] Github Desktop* or the Github CLI* - -The following tools are optional but recommended. For DHS devices, these can be requested through the DHS IT portal: -- [ ] Slack Desktop App** -- [ ] Python 3.10* -- [ ] NodeJS (latest version available)* -- [ ] Putty* -- [ ] Windows Subsystem for Linux* - -* Must be requested through DHS IT portal on DHS devices - -** Downloadable via DHS Software Center +- [ ] Make sure you have `gpg` >2.1.7. Run `gpg --version` to check. If not, [install gnupg](https://formulae.brew.sh/formula/gnupg) + - Alternatively, you can skip this step and [use ssh keys](#setting-up-commit-signing-with-ssh) instead +- [ ] Install the [Github CLI](https://cli.github.com/) ## Access @@ -53,12 +37,7 @@ cf login -a api.fr.cloud.gov --sso **Note:** As mentioned in the [Login documentation](https://developers.login.gov/testing/), the sandbox Login account is different account from your regular, production Login account. If you have not created a Login account for the sandbox before, you will need to create a new account first. -Follow the [.gov onboarding dev setup instructions](https://docs.google.com/document/d/1ukbpW4LSqkb_CCt8LWfpehP03qqfyYfvK3Fl21NaEq8/edit#heading=h.94jwfwkpkhdx). Confirm you successfully set up the following accounts: -- [ ] Identity sandbox accounts - 1 superuser access account and 1 analyst access account. -- [ ] Login.gov account to access stable - -**Optional** -- [ ] Add yourself as a codeowner if desired. See the [Developer readme](https://github.com/cisagov/getgov/blob/main/docs/developer/README.md) for how to do this and what it does. +- [ ] Optional- add yourself as a codeowner if desired. See the [Developer readme](https://github.com/cisagov/getgov/blob/main/docs/developer/README.md) for how to do this and what it does. ### Steps for the onboarder - [ ] Add the onboardee to cloud.gov org (cisa-dotgov) @@ -145,19 +124,3 @@ Additionally, consider a gpg key manager like Kleopatra if you run into issues w We have three types of environments: stable, staging, and sandbox. Stable (production)and staging (pre-prod) get deployed via tagged release, and developer sandboxes are given to get.gov developers to mess around in a production-like environment without disrupting stable or staging. Each sandbox is namespaced and will automatically be deployed too when the appropriate branch syntax is used for that space in an open pull request. There are several things you need to setup to make the sandbox work for a developer. All automation for setting up a developer sandbox is documented in the scripts for [creating a developer sandbox](../../ops/scripts/create_dev_sandbox.sh) and [removing a developer sandbox](../../ops/scripts/destroy_dev_sandbox.sh). A Cloud.gov organization administrator will have to perform the script in order to create the sandbox. - -## Known Issues - -### SSL Verification Failure -Some developers using Government Furnished Equipment (GFE) have problems using tools such as git and pip due to SSL verification failurse. This happens because GFE has a custom certificate chain installed, but these tools use their own certificate bundles. As a result, when they try to verify an ssl connection, they cannot and so the connection fails. To resolve this in pip you can use --use-feature=truststore to direct pip to use the local certificate store. If you are running into this issue when using git on windows, run ```git config --global http.sslbackend schannel```. - -If you are running into these issues in a docker container you will need to export the root certificate and pull it into the container. Ask another developer how to do this properly. - -### Puppeteer Download Error -When building the node image either individually or with docker compose, there may be an error caused by a node package call puppeteer. This can be resolved by adding `ENV PUPPETEER_SKIP_DOWNLOAD=true` to [node.Dockerfile](../../src/node.Dockerfile) after the COPY command. - -### Checksum Error -There is an unresolved issue with python package installation that occurs after the above SSL Verification failure has been resolved. It often manifests as a checksum error, where the hash of a download .whl file (python package) does not match the expected value. This appears to be because pythonhosted.org is cutting off download connections to some devices for some packages (the behavior is somewhat inconsistent). We have outstanding issues with PyPA and DHS IT to fix this. In the meantime we have a [workaround](#developing-using-docker). - -## Developing Using Docker -While we have unresolved issues with certain devices, you can pull a pre-built docker image from matthewswspence/getgov-base that comes with all the needed packages installed. To do this, you will need to change the very first line in the main [Dockerfile](../../src/Dockerfile) to `FROM matthewswspence/getgov-base:latest`. Note: this change will need to be reverted before any branch can be merged. Additionally, this will only resolve the [checksum error](#checksum-error), you will still need to resolve any other issues through the listed instructions. We are actively working to resolve this inconvenience. diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 0712da0f7..a60a59673 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1169,8 +1169,6 @@ document.addEventListener('DOMContentLoaded', function() { const statusIndicator = document.querySelector('.domain__filter-indicator'); const statusToggle = document.querySelector('.usa-button--filter'); const noPortfolioFlag = document.getElementById('no-portfolio-js-flag'); - const portfolioElement = document.getElementById('portfolio-js-value'); - const portfolioValue = portfolioElement ? portfolioElement.getAttribute('data-portfolio') : null; /** * Loads rows in the domains list, as well as updates pagination around the domains list @@ -1180,15 +1178,10 @@ document.addEventListener('DOMContentLoaded', function() { * @param {*} order - the sort order {asc, desc} * @param {*} scroll - control for the scrollToElement functionality * @param {*} searchTerm - the search term - * @param {*} portfolio - the portfolio id */ - function loadDomains(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, status = currentStatus, searchTerm = currentSearchTerm, portfolio = portfolioValue) { + function loadDomains(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, status = currentStatus, searchTerm = currentSearchTerm) { // fetch json of page of domains, given params - let url = `/get-domains-json/?page=${page}&sort_by=${sortBy}&order=${order}&status=${status}&search_term=${searchTerm}` - if (portfolio) - url += `&portfolio=${portfolio}` - - fetch(url) + fetch(`/get-domains-json/?page=${page}&sort_by=${sortBy}&order=${order}&status=${status}&search_term=${searchTerm}`) .then(response => response.json()) .then(data => { if (data.error) { diff --git a/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py b/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py deleted file mode 100644 index 55645298f..000000000 --- a/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py +++ /dev/null @@ -1,38 +0,0 @@ -# Generated by Django 4.2.10 on 2024-07-25 12:45 - -import django.contrib.postgres.fields -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0113_user_portfolio_user_portfolio_additional_permissions_and_more"), - ] - - operations = [ - migrations.AlterField( - model_name="user", - name="portfolio_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_member", "View members"), - ("edit_member", "Create and edit members"), - ("view_all_requests", "View all requests"), - ("view_created_requests", "View created 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/user.py b/src/registrar/models/user.py index 2e36f4f57..e87fb255a 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -77,6 +77,11 @@ class User(AbstractUser): VIEW_ALL_DOMAINS = "view_all_domains", "View all domains and domain reports" VIEW_MANAGED_DOMAINS = "view_managed_domains", "View managed domains" + # EDIT_DOMAINS is really self.domains. We add is hear and leverage it in has_permission + # so we have one way to test for portfolio and domain edit permissions + # Do we need to check for portfolio domains specifically? + # NOTE: A user on an org can currently invite a user outside the org + EDIT_DOMAINS = "edit_domains", "User is a manager on a domain" VIEW_MEMBER = "view_member", "View members" EDIT_MEMBER = "edit_member", "Create and edit members" @@ -264,6 +269,11 @@ class User(AbstractUser): def _has_portfolio_permission(self, portfolio_permission): """The views should only call this function when testing for perms and not rely on roles.""" + # EDIT_DOMAINS === user is a manager on a domain (has UserDomainRole) + # NOTE: Should we check whether the domain is in the portfolio? + if portfolio_permission == self.UserPortfolioPermissionChoices.EDIT_DOMAINS and self.domains.exists(): + return True + if not self.portfolio: return False @@ -277,14 +287,21 @@ class User(AbstractUser): return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO) def has_domains_portfolio_permission(self): - return self._has_portfolio_permission( - User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS - ) or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) + return ( + self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) + # or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_DOMAINS) + ) + + def has_edit_domains_portfolio_permission(self): + return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_DOMAINS) def has_domain_requests_portfolio_permission(self): - return self._has_portfolio_permission( - User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS - ) or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) + return ( + self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) + or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) + # or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_REQUESTS) + ) def has_view_all_domains_permission(self): """Determines if the current user can view all available domains in a given portfolio""" diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index ad91699ef..efebd1e28 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -2,6 +2,7 @@
+ {% if portfolio is None %}

Domain requests

diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index ba9a8dcb7..d4fafaed8 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -1,15 +1,11 @@ {% load static %}
-
+
+ {% if portfolio is None %} -
-

Domains

-
- - {% else %} - - +

Domains

+ {% endif %} {% endif %}
+ {% if portfolio %}
Filter by @@ -154,6 +151,7 @@ Domain name Expires Status + {% if portfolio %} Suborganization {% endif %} diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 5982d89b9..741ec5361 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1292,6 +1292,7 @@ class TestUser(TestCase): 1. Returns False when a user does not have a portfolio 2. Returns True when user has direct permission 3. Returns True when user has permission through a role + 4. Returns True EDIT_DOMAINS when user does not have the perm but has UserDomainRole Note: This tests _get_portfolio_permissions as a side effect """ @@ -1303,9 +1304,11 @@ class TestUser(TestCase): user_can_view_all_domains = self.user.has_domains_portfolio_permission() user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission() + user_can_edit_domains = self.user.has_edit_domains_portfolio_permission() self.assertFalse(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) + self.assertFalse(user_can_edit_domains) self.user.portfolio = portfolio self.user.save() @@ -1313,9 +1316,11 @@ class TestUser(TestCase): user_can_view_all_domains = self.user.has_domains_portfolio_permission() user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission() + user_can_edit_domains = self.user.has_edit_domains_portfolio_permission() self.assertTrue(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) + self.assertFalse(user_can_edit_domains) self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN] self.user.save() @@ -1323,9 +1328,11 @@ class TestUser(TestCase): user_can_view_all_domains = self.user.has_domains_portfolio_permission() user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission() + user_can_edit_domains = self.user.has_edit_domains_portfolio_permission() self.assertTrue(user_can_view_all_domains) self.assertTrue(user_can_view_all_requests) + self.assertFalse(user_can_edit_domains) UserDomainRole.objects.all().get_or_create( user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER @@ -1333,9 +1340,11 @@ class TestUser(TestCase): user_can_view_all_domains = self.user.has_domains_portfolio_permission() user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission() + user_can_edit_domains = self.user.has_edit_domains_portfolio_permission() self.assertTrue(user_can_view_all_domains) self.assertTrue(user_can_view_all_requests) + self.assertTrue(user_can_edit_domains) Portfolio.objects.all().delete() diff --git a/src/registrar/tests/test_views_domains_json.py b/src/registrar/tests/test_views_domains_json.py index 09a233768..18d6415a0 100644 --- a/src/registrar/tests/test_views_domains_json.py +++ b/src/registrar/tests/test_views_domains_json.py @@ -1,4 +1,4 @@ -from registrar.models import UserDomainRole, Domain, DomainInformation, Portfolio +from registrar.models import UserDomainRole, Domain from django.urls import reverse from .test_views import TestWithUser from django_webtest import WebTest # type: ignore @@ -15,25 +15,15 @@ class GetDomainsJsonTest(TestWithUser, WebTest): self.domain1 = Domain.objects.create(name="example1.com", expiration_date="2024-01-01", state="unknown") self.domain2 = Domain.objects.create(name="example2.com", expiration_date="2024-02-01", state="dns needed") self.domain3 = Domain.objects.create(name="example3.com", expiration_date="2024-03-01", state="ready") - self.domain4 = Domain.objects.create(name="example4.com", expiration_date="2024-03-01", state="ready") # Create UserDomainRoles UserDomainRole.objects.create(user=self.user, domain=self.domain1) UserDomainRole.objects.create(user=self.user, domain=self.domain2) UserDomainRole.objects.create(user=self.user, domain=self.domain3) - # Create Portfolio - self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Example org") - - # Add domain3 and domain4 to portfolio - DomainInformation.objects.create(creator=self.user, domain=self.domain3, portfolio=self.portfolio) - DomainInformation.objects.create(creator=self.user, domain=self.domain4, portfolio=self.portfolio) - def tearDown(self): - UserDomainRole.objects.all().delete() - DomainInformation.objects.all().delete() - Portfolio.objects.all().delete() super().tearDown() + UserDomainRole.objects.all().delete() @less_console_noise_decorator def test_get_domains_json_unauthenticated(self): @@ -114,82 +104,6 @@ class GetDomainsJsonTest(TestWithUser, WebTest): ) self.assertEqual(svg_icon_expected, svg_icons[i]) - @less_console_noise_decorator - def test_get_domains_json_with_portfolio(self): - """Test that an authenticated user gets the list of 2 domains for portfolio.""" - - response = self.app.get(reverse("get_domains_json"), {"portfolio": self.portfolio.id}) - self.assertEqual(response.status_code, 200) - data = response.json - - # Check pagination info - self.assertEqual(data["page"], 1) - self.assertFalse(data["has_next"]) - self.assertFalse(data["has_previous"]) - self.assertEqual(data["num_pages"], 1) - - # Check the number of domains - self.assertEqual(len(data["domains"]), 2) - - # Expected domains - expected_domains = [self.domain3, self.domain4] - - # Extract fields from response - domain_ids = [domain["id"] for domain in data["domains"]] - names = [domain["name"] for domain in data["domains"]] - expiration_dates = [domain["expiration_date"] for domain in data["domains"]] - states = [domain["state"] for domain in data["domains"]] - state_displays = [domain["state_display"] for domain in data["domains"]] - get_state_help_texts = [domain["get_state_help_text"] for domain in data["domains"]] - action_urls = [domain["action_url"] for domain in data["domains"]] - action_labels = [domain["action_label"] for domain in data["domains"]] - svg_icons = [domain["svg_icon"] for domain in data["domains"]] - - # Check fields for each domain - for i, expected_domain in enumerate(expected_domains): - self.assertEqual(expected_domain.id, domain_ids[i]) - self.assertEqual(expected_domain.name, names[i]) - self.assertEqual(expected_domain.expiration_date, expiration_dates[i]) - self.assertEqual(expected_domain.state, states[i]) - - # Parsing the expiration date from string to date - parsed_expiration_date = parse_date(expiration_dates[i]) - expected_domain.expiration_date = parsed_expiration_date - - # Check state_display and get_state_help_text - self.assertEqual(expected_domain.state_display(), state_displays[i]) - self.assertEqual(expected_domain.get_state_help_text(), get_state_help_texts[i]) - - self.assertEqual(reverse("domain", kwargs={"pk": expected_domain.id}), action_urls[i]) - - # Check action_label - user_domain_role_exists = UserDomainRole.objects.filter( - domain_id=expected_domains[i].id, user=self.user - ).exists() - action_label_expected = ( - "View" - if not user_domain_role_exists - or expected_domains[i].state - in [ - Domain.State.DELETED, - Domain.State.ON_HOLD, - ] - else "Manage" - ) - self.assertEqual(action_label_expected, action_labels[i]) - - # Check svg_icon - svg_icon_expected = ( - "visibility" - if expected_domains[i].state - in [ - Domain.State.DELETED, - Domain.State.ON_HOLD, - ] - else "settings" - ) - self.assertEqual(svg_icon_expected, svg_icons[i]) - @less_console_noise_decorator def test_get_domains_json_search(self): """Test search.""" diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py index 2cfa7ce02..2f0d33fc2 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -6,8 +6,6 @@ from django.contrib.auth.decorators import login_required from django.urls import reverse from django.db.models import Q -from registrar.models.domain_information import DomainInformation - logger = logging.getLogger(__name__) @@ -16,9 +14,9 @@ def get_domains_json(request): """Given the current request, get all domains that are associated with the User object""" - domain_ids = get_domain_ids_from_request(request) + domain_ids = request.user.get_user_domain_ids() - objects = Domain.objects.filter(id__in=domain_ids).select_related("domain_info__sub_organization") + objects = Domain.objects.filter(id__in=domain_ids) unfiltered_total = objects.count() objects = apply_search(objects, request) @@ -29,7 +27,7 @@ def get_domains_json(request): page_number = request.GET.get("page") page_obj = paginator.get_page(page_number) - domains = [serialize_domain(domain, request.user) for domain in page_obj.object_list] + domains = [serialize_domain(domain) for domain in page_obj.object_list] return JsonResponse( { @@ -44,21 +42,6 @@ def get_domains_json(request): ) -def get_domain_ids_from_request(request): - """Get domain ids from request. - - If portfolio specified, return domain ids associated with portfolio. - Otherwise, return domain ids associated with request.user. - """ - portfolio = request.GET.get("portfolio") - if portfolio: - domain_infos = DomainInformation.objects.filter(portfolio=portfolio) - return domain_infos.values_list("domain_id", flat=True) - else: - user_domain_roles = UserDomainRole.objects.filter(user=request.user) - return user_domain_roles.values_list("domain_id", flat=True) - - def apply_search(queryset, request): search_term = request.GET.get("search_term") if search_term: @@ -110,7 +93,7 @@ def apply_sorting(queryset, request): return queryset.order_by(sort_by) -def serialize_domain(domain, user): +def serialize_domain(domain): suborganization_name = None try: domain_info = domain.domain_info @@ -122,9 +105,6 @@ def serialize_domain(domain, user): domain_info = None logger.debug(f"Issue in domains_json: We could not find domain_info for {domain}") - # Check if there is a UserDomainRole for this domain and user - user_domain_role_exists = UserDomainRole.objects.filter(domain_id=domain.id, user=user).exists() - return { "id": domain.id, "name": domain.name, @@ -133,11 +113,7 @@ def serialize_domain(domain, user): "state_display": domain.state_display(), "get_state_help_text": domain.get_state_help_text(), "action_url": reverse("domain", kwargs={"pk": domain.id}), - "action_label": ( - "View" - if not user_domain_role_exists or domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] - else "Manage" - ), + "action_label": ("View" if domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] else "Manage"), "svg_icon": ("visibility" if domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] else "settings"), "suborganization": suborganization_name, } From 86f040e4b774b08bf1c1a02b239f80ad6dee4224 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 1 Aug 2024 09:53:59 -0600 Subject: [PATCH 20/44] Reapply "Merge branch 'main' into za/2348-csv-export-org-member-domain-export" This reverts commit 8becad81866dccf67314fcfd99340a51e3dc65ab. --- .../ISSUE_TEMPLATE/developer-onboarding.md | 49 ++++++++-- src/registrar/assets/js/get-gov.js | 11 ++- ...r_user_portfolio_additional_permissions.py | 38 ++++++++ src/registrar/models/user.py | 29 ++---- .../includes/domain_requests_table.html | 1 - .../templates/includes/domains_table.html | 14 +-- src/registrar/tests/test_models.py | 9 -- .../tests/test_views_domains_json.py | 90 ++++++++++++++++++- src/registrar/views/domains_json.py | 34 +++++-- 9 files changed, 221 insertions(+), 54 deletions(-) create mode 100644 src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py diff --git a/.github/ISSUE_TEMPLATE/developer-onboarding.md b/.github/ISSUE_TEMPLATE/developer-onboarding.md index d63cf2f94..a0825ab52 100644 --- a/.github/ISSUE_TEMPLATE/developer-onboarding.md +++ b/.github/ISSUE_TEMPLATE/developer-onboarding.md @@ -14,13 +14,29 @@ assignees: abroddrick ## Installation -There are several tools we use locally that you will need to have. -- [ ] [Install the cf CLI v7](https://docs.cloudfoundry.org/cf-cli/install-go-cli.html#pkg-mac) for the ability to deploy +There are several tools we use locally that you will need to have. + +- [ ] [Cloudfoundry CLI](https://docs.cloudfoundry.org/cf-cli/install-go-cli.html#pkg-mac) Note: If you are on Windows the cli will be under `cf8` or `cf7` depending on which version you install. - If you are using Windows, installation information can be found [here](https://github.com/cloudfoundry/cli/wiki/V8-CLI-Installation-Guide#installers-and-compressed-binaries) - Alternatively, for Windows, [consider using chocolately](https://community.chocolatey.org/packages/cloudfoundry-cli/7.2.0) -- [ ] Make sure you have `gpg` >2.1.7. Run `gpg --version` to check. If not, [install gnupg](https://formulae.brew.sh/formula/gnupg) - - Alternatively, you can skip this step and [use ssh keys](#setting-up-commit-signing-with-ssh) instead -- [ ] Install the [Github CLI](https://cli.github.com/) +- [ ] [GPG](https://gnupg.org/download/) + - Make sure you have `gpg` >2.1.7. Run `gpg --version` to check. If not, [install gnupg](https://formulae.brew.sh/formula/gnupg) + - This may not work on DHS devices. Alternatively, you can [use ssh keys](#setting-up-commit-signing-with-ssh) instead. +- [ ] Docker Community Edition* +- [ ] Git* +- [ ] VSCode (our preferred editor)* +- [ ] Github Desktop* or the Github CLI* + +The following tools are optional but recommended. For DHS devices, these can be requested through the DHS IT portal: +- [ ] Slack Desktop App** +- [ ] Python 3.10* +- [ ] NodeJS (latest version available)* +- [ ] Putty* +- [ ] Windows Subsystem for Linux* + +* Must be requested through DHS IT portal on DHS devices + +** Downloadable via DHS Software Center ## Access @@ -37,7 +53,12 @@ cf login -a api.fr.cloud.gov --sso **Note:** As mentioned in the [Login documentation](https://developers.login.gov/testing/), the sandbox Login account is different account from your regular, production Login account. If you have not created a Login account for the sandbox before, you will need to create a new account first. -- [ ] Optional- add yourself as a codeowner if desired. See the [Developer readme](https://github.com/cisagov/getgov/blob/main/docs/developer/README.md) for how to do this and what it does. +Follow the [.gov onboarding dev setup instructions](https://docs.google.com/document/d/1ukbpW4LSqkb_CCt8LWfpehP03qqfyYfvK3Fl21NaEq8/edit#heading=h.94jwfwkpkhdx). Confirm you successfully set up the following accounts: +- [ ] Identity sandbox accounts - 1 superuser access account and 1 analyst access account. +- [ ] Login.gov account to access stable + +**Optional** +- [ ] Add yourself as a codeowner if desired. See the [Developer readme](https://github.com/cisagov/getgov/blob/main/docs/developer/README.md) for how to do this and what it does. ### Steps for the onboarder - [ ] Add the onboardee to cloud.gov org (cisa-dotgov) @@ -124,3 +145,19 @@ Additionally, consider a gpg key manager like Kleopatra if you run into issues w We have three types of environments: stable, staging, and sandbox. Stable (production)and staging (pre-prod) get deployed via tagged release, and developer sandboxes are given to get.gov developers to mess around in a production-like environment without disrupting stable or staging. Each sandbox is namespaced and will automatically be deployed too when the appropriate branch syntax is used for that space in an open pull request. There are several things you need to setup to make the sandbox work for a developer. All automation for setting up a developer sandbox is documented in the scripts for [creating a developer sandbox](../../ops/scripts/create_dev_sandbox.sh) and [removing a developer sandbox](../../ops/scripts/destroy_dev_sandbox.sh). A Cloud.gov organization administrator will have to perform the script in order to create the sandbox. + +## Known Issues + +### SSL Verification Failure +Some developers using Government Furnished Equipment (GFE) have problems using tools such as git and pip due to SSL verification failurse. This happens because GFE has a custom certificate chain installed, but these tools use their own certificate bundles. As a result, when they try to verify an ssl connection, they cannot and so the connection fails. To resolve this in pip you can use --use-feature=truststore to direct pip to use the local certificate store. If you are running into this issue when using git on windows, run ```git config --global http.sslbackend schannel```. + +If you are running into these issues in a docker container you will need to export the root certificate and pull it into the container. Ask another developer how to do this properly. + +### Puppeteer Download Error +When building the node image either individually or with docker compose, there may be an error caused by a node package call puppeteer. This can be resolved by adding `ENV PUPPETEER_SKIP_DOWNLOAD=true` to [node.Dockerfile](../../src/node.Dockerfile) after the COPY command. + +### Checksum Error +There is an unresolved issue with python package installation that occurs after the above SSL Verification failure has been resolved. It often manifests as a checksum error, where the hash of a download .whl file (python package) does not match the expected value. This appears to be because pythonhosted.org is cutting off download connections to some devices for some packages (the behavior is somewhat inconsistent). We have outstanding issues with PyPA and DHS IT to fix this. In the meantime we have a [workaround](#developing-using-docker). + +## Developing Using Docker +While we have unresolved issues with certain devices, you can pull a pre-built docker image from matthewswspence/getgov-base that comes with all the needed packages installed. To do this, you will need to change the very first line in the main [Dockerfile](../../src/Dockerfile) to `FROM matthewswspence/getgov-base:latest`. Note: this change will need to be reverted before any branch can be merged. Additionally, this will only resolve the [checksum error](#checksum-error), you will still need to resolve any other issues through the listed instructions. We are actively working to resolve this inconvenience. diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index a60a59673..0712da0f7 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1169,6 +1169,8 @@ document.addEventListener('DOMContentLoaded', function() { const statusIndicator = document.querySelector('.domain__filter-indicator'); const statusToggle = document.querySelector('.usa-button--filter'); const noPortfolioFlag = document.getElementById('no-portfolio-js-flag'); + const portfolioElement = document.getElementById('portfolio-js-value'); + const portfolioValue = portfolioElement ? portfolioElement.getAttribute('data-portfolio') : null; /** * Loads rows in the domains list, as well as updates pagination around the domains list @@ -1178,10 +1180,15 @@ document.addEventListener('DOMContentLoaded', function() { * @param {*} order - the sort order {asc, desc} * @param {*} scroll - control for the scrollToElement functionality * @param {*} searchTerm - the search term + * @param {*} portfolio - the portfolio id */ - function loadDomains(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, status = currentStatus, searchTerm = currentSearchTerm) { + function loadDomains(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, status = currentStatus, searchTerm = currentSearchTerm, portfolio = portfolioValue) { // fetch json of page of domains, given params - fetch(`/get-domains-json/?page=${page}&sort_by=${sortBy}&order=${order}&status=${status}&search_term=${searchTerm}`) + let url = `/get-domains-json/?page=${page}&sort_by=${sortBy}&order=${order}&status=${status}&search_term=${searchTerm}` + if (portfolio) + url += `&portfolio=${portfolio}` + + fetch(url) .then(response => response.json()) .then(data => { if (data.error) { diff --git a/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py b/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py new file mode 100644 index 000000000..55645298f --- /dev/null +++ b/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py @@ -0,0 +1,38 @@ +# Generated by Django 4.2.10 on 2024-07-25 12:45 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0113_user_portfolio_user_portfolio_additional_permissions_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="user", + name="portfolio_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_member", "View members"), + ("edit_member", "Create and edit members"), + ("view_all_requests", "View all requests"), + ("view_created_requests", "View created 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/user.py b/src/registrar/models/user.py index e87fb255a..2e36f4f57 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -77,11 +77,6 @@ class User(AbstractUser): VIEW_ALL_DOMAINS = "view_all_domains", "View all domains and domain reports" VIEW_MANAGED_DOMAINS = "view_managed_domains", "View managed domains" - # EDIT_DOMAINS is really self.domains. We add is hear and leverage it in has_permission - # so we have one way to test for portfolio and domain edit permissions - # Do we need to check for portfolio domains specifically? - # NOTE: A user on an org can currently invite a user outside the org - EDIT_DOMAINS = "edit_domains", "User is a manager on a domain" VIEW_MEMBER = "view_member", "View members" EDIT_MEMBER = "edit_member", "Create and edit members" @@ -269,11 +264,6 @@ class User(AbstractUser): def _has_portfolio_permission(self, portfolio_permission): """The views should only call this function when testing for perms and not rely on roles.""" - # EDIT_DOMAINS === user is a manager on a domain (has UserDomainRole) - # NOTE: Should we check whether the domain is in the portfolio? - if portfolio_permission == self.UserPortfolioPermissionChoices.EDIT_DOMAINS and self.domains.exists(): - return True - if not self.portfolio: return False @@ -287,21 +277,14 @@ class User(AbstractUser): return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO) def has_domains_portfolio_permission(self): - return ( - self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) - or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) - # or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_DOMAINS) - ) - - def has_edit_domains_portfolio_permission(self): - return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_DOMAINS) + return self._has_portfolio_permission( + User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS + ) or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) def has_domain_requests_portfolio_permission(self): - return ( - self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) - or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) - # or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_REQUESTS) - ) + return self._has_portfolio_permission( + User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS + ) or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) def has_view_all_domains_permission(self): """Determines if the current user can view all available domains in a given portfolio""" diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index efebd1e28..ad91699ef 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -2,7 +2,6 @@
- {% if portfolio is None %}

Domain requests

diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index d4fafaed8..ba9a8dcb7 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -1,11 +1,15 @@ {% load static %}
-
- +
{% if portfolio is None %} -

Domains

- +
+

Domains

+
+ + {% else %} + + {% endif %} {% endif %}
- {% if portfolio %}
Filter by @@ -151,7 +154,6 @@ Domain name Expires Status - {% if portfolio %} Suborganization {% endif %} diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 741ec5361..5982d89b9 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1292,7 +1292,6 @@ class TestUser(TestCase): 1. Returns False when a user does not have a portfolio 2. Returns True when user has direct permission 3. Returns True when user has permission through a role - 4. Returns True EDIT_DOMAINS when user does not have the perm but has UserDomainRole Note: This tests _get_portfolio_permissions as a side effect """ @@ -1304,11 +1303,9 @@ class TestUser(TestCase): user_can_view_all_domains = self.user.has_domains_portfolio_permission() user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission() - user_can_edit_domains = self.user.has_edit_domains_portfolio_permission() self.assertFalse(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) - self.assertFalse(user_can_edit_domains) self.user.portfolio = portfolio self.user.save() @@ -1316,11 +1313,9 @@ class TestUser(TestCase): user_can_view_all_domains = self.user.has_domains_portfolio_permission() user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission() - user_can_edit_domains = self.user.has_edit_domains_portfolio_permission() self.assertTrue(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) - self.assertFalse(user_can_edit_domains) self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN] self.user.save() @@ -1328,11 +1323,9 @@ class TestUser(TestCase): user_can_view_all_domains = self.user.has_domains_portfolio_permission() user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission() - user_can_edit_domains = self.user.has_edit_domains_portfolio_permission() self.assertTrue(user_can_view_all_domains) self.assertTrue(user_can_view_all_requests) - self.assertFalse(user_can_edit_domains) UserDomainRole.objects.all().get_or_create( user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER @@ -1340,11 +1333,9 @@ class TestUser(TestCase): user_can_view_all_domains = self.user.has_domains_portfolio_permission() user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission() - user_can_edit_domains = self.user.has_edit_domains_portfolio_permission() self.assertTrue(user_can_view_all_domains) self.assertTrue(user_can_view_all_requests) - self.assertTrue(user_can_edit_domains) Portfolio.objects.all().delete() diff --git a/src/registrar/tests/test_views_domains_json.py b/src/registrar/tests/test_views_domains_json.py index 18d6415a0..09a233768 100644 --- a/src/registrar/tests/test_views_domains_json.py +++ b/src/registrar/tests/test_views_domains_json.py @@ -1,4 +1,4 @@ -from registrar.models import UserDomainRole, Domain +from registrar.models import UserDomainRole, Domain, DomainInformation, Portfolio from django.urls import reverse from .test_views import TestWithUser from django_webtest import WebTest # type: ignore @@ -15,15 +15,25 @@ class GetDomainsJsonTest(TestWithUser, WebTest): self.domain1 = Domain.objects.create(name="example1.com", expiration_date="2024-01-01", state="unknown") self.domain2 = Domain.objects.create(name="example2.com", expiration_date="2024-02-01", state="dns needed") self.domain3 = Domain.objects.create(name="example3.com", expiration_date="2024-03-01", state="ready") + self.domain4 = Domain.objects.create(name="example4.com", expiration_date="2024-03-01", state="ready") # Create UserDomainRoles UserDomainRole.objects.create(user=self.user, domain=self.domain1) UserDomainRole.objects.create(user=self.user, domain=self.domain2) UserDomainRole.objects.create(user=self.user, domain=self.domain3) + # Create Portfolio + self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Example org") + + # Add domain3 and domain4 to portfolio + DomainInformation.objects.create(creator=self.user, domain=self.domain3, portfolio=self.portfolio) + DomainInformation.objects.create(creator=self.user, domain=self.domain4, portfolio=self.portfolio) + def tearDown(self): - super().tearDown() UserDomainRole.objects.all().delete() + DomainInformation.objects.all().delete() + Portfolio.objects.all().delete() + super().tearDown() @less_console_noise_decorator def test_get_domains_json_unauthenticated(self): @@ -104,6 +114,82 @@ class GetDomainsJsonTest(TestWithUser, WebTest): ) self.assertEqual(svg_icon_expected, svg_icons[i]) + @less_console_noise_decorator + def test_get_domains_json_with_portfolio(self): + """Test that an authenticated user gets the list of 2 domains for portfolio.""" + + response = self.app.get(reverse("get_domains_json"), {"portfolio": self.portfolio.id}) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertFalse(data["has_next"]) + self.assertFalse(data["has_previous"]) + self.assertEqual(data["num_pages"], 1) + + # Check the number of domains + self.assertEqual(len(data["domains"]), 2) + + # Expected domains + expected_domains = [self.domain3, self.domain4] + + # Extract fields from response + domain_ids = [domain["id"] for domain in data["domains"]] + names = [domain["name"] for domain in data["domains"]] + expiration_dates = [domain["expiration_date"] for domain in data["domains"]] + states = [domain["state"] for domain in data["domains"]] + state_displays = [domain["state_display"] for domain in data["domains"]] + get_state_help_texts = [domain["get_state_help_text"] for domain in data["domains"]] + action_urls = [domain["action_url"] for domain in data["domains"]] + action_labels = [domain["action_label"] for domain in data["domains"]] + svg_icons = [domain["svg_icon"] for domain in data["domains"]] + + # Check fields for each domain + for i, expected_domain in enumerate(expected_domains): + self.assertEqual(expected_domain.id, domain_ids[i]) + self.assertEqual(expected_domain.name, names[i]) + self.assertEqual(expected_domain.expiration_date, expiration_dates[i]) + self.assertEqual(expected_domain.state, states[i]) + + # Parsing the expiration date from string to date + parsed_expiration_date = parse_date(expiration_dates[i]) + expected_domain.expiration_date = parsed_expiration_date + + # Check state_display and get_state_help_text + self.assertEqual(expected_domain.state_display(), state_displays[i]) + self.assertEqual(expected_domain.get_state_help_text(), get_state_help_texts[i]) + + self.assertEqual(reverse("domain", kwargs={"pk": expected_domain.id}), action_urls[i]) + + # Check action_label + user_domain_role_exists = UserDomainRole.objects.filter( + domain_id=expected_domains[i].id, user=self.user + ).exists() + action_label_expected = ( + "View" + if not user_domain_role_exists + or expected_domains[i].state + in [ + Domain.State.DELETED, + Domain.State.ON_HOLD, + ] + else "Manage" + ) + self.assertEqual(action_label_expected, action_labels[i]) + + # Check svg_icon + svg_icon_expected = ( + "visibility" + if expected_domains[i].state + in [ + Domain.State.DELETED, + Domain.State.ON_HOLD, + ] + else "settings" + ) + self.assertEqual(svg_icon_expected, svg_icons[i]) + @less_console_noise_decorator def test_get_domains_json_search(self): """Test search.""" diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py index 2f0d33fc2..2cfa7ce02 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -6,6 +6,8 @@ from django.contrib.auth.decorators import login_required from django.urls import reverse from django.db.models import Q +from registrar.models.domain_information import DomainInformation + logger = logging.getLogger(__name__) @@ -14,9 +16,9 @@ def get_domains_json(request): """Given the current request, get all domains that are associated with the User object""" - domain_ids = request.user.get_user_domain_ids() + domain_ids = get_domain_ids_from_request(request) - objects = Domain.objects.filter(id__in=domain_ids) + objects = Domain.objects.filter(id__in=domain_ids).select_related("domain_info__sub_organization") unfiltered_total = objects.count() objects = apply_search(objects, request) @@ -27,7 +29,7 @@ def get_domains_json(request): page_number = request.GET.get("page") page_obj = paginator.get_page(page_number) - domains = [serialize_domain(domain) for domain in page_obj.object_list] + domains = [serialize_domain(domain, request.user) for domain in page_obj.object_list] return JsonResponse( { @@ -42,6 +44,21 @@ def get_domains_json(request): ) +def get_domain_ids_from_request(request): + """Get domain ids from request. + + If portfolio specified, return domain ids associated with portfolio. + Otherwise, return domain ids associated with request.user. + """ + portfolio = request.GET.get("portfolio") + if portfolio: + domain_infos = DomainInformation.objects.filter(portfolio=portfolio) + return domain_infos.values_list("domain_id", flat=True) + else: + user_domain_roles = UserDomainRole.objects.filter(user=request.user) + return user_domain_roles.values_list("domain_id", flat=True) + + def apply_search(queryset, request): search_term = request.GET.get("search_term") if search_term: @@ -93,7 +110,7 @@ def apply_sorting(queryset, request): return queryset.order_by(sort_by) -def serialize_domain(domain): +def serialize_domain(domain, user): suborganization_name = None try: domain_info = domain.domain_info @@ -105,6 +122,9 @@ def serialize_domain(domain): domain_info = None logger.debug(f"Issue in domains_json: We could not find domain_info for {domain}") + # Check if there is a UserDomainRole for this domain and user + user_domain_role_exists = UserDomainRole.objects.filter(domain_id=domain.id, user=user).exists() + return { "id": domain.id, "name": domain.name, @@ -113,7 +133,11 @@ def serialize_domain(domain): "state_display": domain.state_display(), "get_state_help_text": domain.get_state_help_text(), "action_url": reverse("domain", kwargs={"pk": domain.id}), - "action_label": ("View" if domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] else "Manage"), + "action_label": ( + "View" + if not user_domain_role_exists or domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] + else "Manage" + ), "svg_icon": ("visibility" if domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] else "settings"), "suborganization": suborganization_name, } From ec88af56c31277a8ee5444b942728cc061cd5818 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 1 Aug 2024 09:59:43 -0600 Subject: [PATCH 21/44] Update domains_json.py --- src/registrar/views/domains_json.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py index 2cfa7ce02..7bee98d2f 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -6,7 +6,7 @@ from django.contrib.auth.decorators import login_required from django.urls import reverse from django.db.models import Q -from registrar.models.domain_information import DomainInformation +from registrar.models import DomainInformation, UserDomainRole logger = logging.getLogger(__name__) From dfd66ef86ac4ffc29102d318b3c12d2a00bcec88 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 08:37:08 -0600 Subject: [PATCH 22/44] Update portfolios.py --- src/registrar/views/portfolios.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index c46f30080..1f2c15d26 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -20,6 +20,7 @@ logger = logging.getLogger(__name__) class PortfolioDomainsView(PortfolioDomainsPermissionView, View): template_name = "portfolio_domains.html" + def get(self, request): context = {} if self.request.user.is_authenticated: From dc7075860310d996d8826cf0758a39338c729326 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 08:47:16 -0600 Subject: [PATCH 23/44] Add context --- src/registrar/views/portfolios.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 1f2c15d26..bcf8e95c1 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -25,7 +25,7 @@ class PortfolioDomainsView(PortfolioDomainsPermissionView, View): context = {} if self.request.user.is_authenticated: context["user_domain_count"] = self.request.user.get_user_domain_ids().count() - return render(request, "portfolio_domains.html") + return render(request, "portfolio_domains.html", context) class PortfolioDomainRequestsView(PortfolioDomainRequestsPermissionView, View): From 2769a9e5bafe2fa9402234b766188dbd817c7c2f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 09:05:47 -0600 Subject: [PATCH 24/44] Fix merge conflicts --- src/registrar/context_processors.py | 14 ++------------ src/registrar/models/user.py | 4 ++-- .../templates/includes/domains_table.html | 2 +- src/registrar/utility/csv_export.py | 2 +- src/registrar/views/domains_json.py | 8 +++----- src/registrar/views/index.py | 4 ++-- src/registrar/views/portfolios.py | 4 ++-- 7 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 861a4e701..99b0aceaf 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -39,17 +39,6 @@ def is_production(request): return {"IS_PRODUCTION": settings.IS_PRODUCTION} -def org_user_status(request): - if request.user.is_authenticated: - is_org_user = request.user.is_org_user(request) - else: - is_org_user = False - - return { - "is_org_user": is_org_user, - } - - def add_path_to_context(request): return {"path": getattr(request, "path", None)} @@ -68,13 +57,14 @@ def portfolio_permissions(request): "has_domain_requests_portfolio_permission": False, "portfolio": None, "has_organization_feature_flag": False, + "is_org_user": False, } return { "has_base_portfolio_permission": request.user.has_base_portfolio_permission(), "has_domains_portfolio_permission": request.user.has_domains_portfolio_permission(), "has_domain_requests_portfolio_permission": request.user.has_domain_requests_portfolio_permission(), - "portfolio": request.user.portfolio, "has_organization_feature_flag": flag_is_active(request, "organization_feature"), + "is_org_user": request.user.is_org_user(request) } except AttributeError: # Handles cases where request.user might not exist diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 2e36f4f57..169e15430 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -413,9 +413,9 @@ class User(AbstractUser): has_organization_feature_flag = flag_is_active(request, "organization_feature") return has_organization_feature_flag and self.has_base_portfolio_permission() - def get_user_domain_ids(self): + def get_user_domain_ids(self, request): """Returns either the domains ids associated with this user on UserDomainRole or Portfolio""" - if self.has_base_portfolio_permission() and self.has_view_all_domains_permission(): + if self.is_org_user(request) and self.has_view_all_domains_permission(): return DomainInformation.objects.filter(portfolio=self.portfolio).values_list("domain_id", flat=True) else: return UserDomainRole.objects.filter(user=self).values_list("domain_id", flat=True) diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index bce89fd12..1098e1659 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -2,7 +2,7 @@
- {% if portfolio is None %} + {% if not portfolio %}

Domains

{% else %} diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 0cc112557..db961a36d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -580,7 +580,7 @@ class DomainDataTypeUser(DomainDataType): return Q(id__in=[]) else: # Get all domains the user is associated with - return Q(domain__id__in=request.user.get_user_domain_ids()) + return Q(domain__id__in=request.user.get_user_domain_ids(request)) class DomainDataFull(DomainExport): diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py index 1387b4ecb..6f16b74bd 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -1,20 +1,18 @@ import logging from django.http import JsonResponse from django.core.paginator import Paginator -from registrar.models import Domain +from registrar.models import UserDomainRole, Domain, DomainInformation from django.contrib.auth.decorators import login_required from django.urls import reverse from django.db.models import Q -from registrar.models import DomainInformation, UserDomainRole - logger = logging.getLogger(__name__) @login_required def get_domains_json(request): """Given the current request, - get all domains that are associated with the User object""" + get all domains that are associated with the UserDomainRole object""" domain_ids = get_domain_ids_from_request(request) @@ -51,7 +49,7 @@ def get_domain_ids_from_request(request): Otherwise, return domain ids associated with request.user. """ portfolio = request.GET.get("portfolio") - if portfolio: + if portfolio and request.user.is_org_user(request): domain_infos = DomainInformation.objects.filter(portfolio=portfolio) return domain_infos.values_list("domain_id", flat=True) else: diff --git a/src/registrar/views/index.py b/src/registrar/views/index.py index 0d74a81cc..53900a4a7 100644 --- a/src/registrar/views/index.py +++ b/src/registrar/views/index.py @@ -5,9 +5,9 @@ def index(request): """This page is available to anyone without logging in.""" context = {} - if request.user.is_authenticated: + if request and request.user and request.user.is_authenticated: # This controls the creation of a new domain request in the wizard request.session["new_request"] = True - context["user_domain_count"] = request.user.get_user_domain_ids().count() + context["user_domain_count"] = request.user.get_user_domain_ids(request).count() return render(request, "home.html", context) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index bcf8e95c1..7a1dc4c8f 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -23,8 +23,8 @@ class PortfolioDomainsView(PortfolioDomainsPermissionView, View): def get(self, request): context = {} - if self.request.user.is_authenticated: - context["user_domain_count"] = self.request.user.get_user_domain_ids().count() + if self.request and self.request.user and self.request.user.is_authenticated: + context["user_domain_count"] = self.request.user.get_user_domain_ids(request).count() return render(request, "portfolio_domains.html", context) From e91af30b9e012f59cf7ecaaf0aea707b5e35b74d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 09:10:42 -0600 Subject: [PATCH 25/44] Update context_processors.py --- src/registrar/context_processors.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 99b0aceaf..ddabfc571 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -47,6 +47,17 @@ def add_has_profile_feature_flag_to_context(request): return {"has_profile_feature_flag": flag_is_active(request, "profile_feature")} +def org_user_status(request): + if request.user.is_authenticated: + is_org_user = request.user.is_org_user(request) + else: + is_org_user = False + + return { + "is_org_user": is_org_user, + } + + def portfolio_permissions(request): """Make portfolio permissions for the request user available in global context""" try: @@ -57,14 +68,12 @@ def portfolio_permissions(request): "has_domain_requests_portfolio_permission": False, "portfolio": None, "has_organization_feature_flag": False, - "is_org_user": False, } return { "has_base_portfolio_permission": request.user.has_base_portfolio_permission(), "has_domains_portfolio_permission": request.user.has_domains_portfolio_permission(), "has_domain_requests_portfolio_permission": request.user.has_domain_requests_portfolio_permission(), "has_organization_feature_flag": flag_is_active(request, "organization_feature"), - "is_org_user": request.user.is_org_user(request) } except AttributeError: # Handles cases where request.user might not exist From 0814b862270d4e310fa8e8604f363c4fbca8c7f5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 09:31:01 -0600 Subject: [PATCH 26/44] Update domains_json.py --- src/registrar/views/domains_json.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py index 6f16b74bd..06c211227 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -49,7 +49,7 @@ def get_domain_ids_from_request(request): Otherwise, return domain ids associated with request.user. """ portfolio = request.GET.get("portfolio") - if portfolio and request.user.is_org_user(request): + if portfolio: domain_infos = DomainInformation.objects.filter(portfolio=portfolio) return domain_infos.values_list("domain_id", flat=True) else: From a8b362e32e69d7867f3adf6a267aff2e21532d1a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 09:40:48 -0600 Subject: [PATCH 27/44] Fix merge conflicts pt 2 --- src/registrar/context_processors.py | 9 +-------- src/registrar/templates/includes/domains_table.html | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index ddabfc571..bffa6d5c4 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -39,14 +39,6 @@ def is_production(request): return {"IS_PRODUCTION": settings.IS_PRODUCTION} -def add_path_to_context(request): - return {"path": getattr(request, "path", None)} - - -def add_has_profile_feature_flag_to_context(request): - return {"has_profile_feature_flag": flag_is_active(request, "profile_feature")} - - def org_user_status(request): if request.user.is_authenticated: is_org_user = request.user.is_org_user(request) @@ -73,6 +65,7 @@ def portfolio_permissions(request): "has_base_portfolio_permission": request.user.has_base_portfolio_permission(), "has_domains_portfolio_permission": request.user.has_domains_portfolio_permission(), "has_domain_requests_portfolio_permission": request.user.has_domain_requests_portfolio_permission(), + "portfolio": request.user.portfolio, "has_organization_feature_flag": flag_is_active(request, "organization_feature"), } except AttributeError: diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 1098e1659..bce89fd12 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -2,7 +2,7 @@
- {% if not portfolio %} + {% if portfolio is None %}

Domains

{% else %} From 5796df0531c97bdd34aebb5dadf1367b65ccdf47 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 09:47:39 -0600 Subject: [PATCH 28/44] Cleanup (once again) --- src/registrar/context_processors.py | 8 ++++++++ src/registrar/tests/test_reports.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index bffa6d5c4..861a4e701 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -50,6 +50,14 @@ def org_user_status(request): } +def add_path_to_context(request): + return {"path": getattr(request, "path", None)} + + +def add_has_profile_feature_flag_to_context(request): + return {"has_profile_feature_flag": flag_is_active(request, "profile_feature")} + + def portfolio_permissions(request): """Make portfolio permissions for the request user available in global context""" try: diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 5b2112f4f..6b8cb8882 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -314,7 +314,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.assertEqual(csv_content, expected_content) @less_console_noise_decorator - @override_flag("profile_feature", active=True) + @override_flag("organization_feature", active=True) def test_domain_data_type_user_with_portfolio(self): """Tests DomainDataTypeUser export with portfolio permissions""" From 80a39401339c18b4f039b55091ec8f435ccbf721 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 5 Aug 2024 12:28:32 -0600 Subject: [PATCH 29/44] Removed instructions --- .github/workflows/reset-db.yaml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/.github/workflows/reset-db.yaml b/.github/workflows/reset-db.yaml index 50c83895a..49e4b5e5f 100644 --- a/.github/workflows/reset-db.yaml +++ b/.github/workflows/reset-db.yaml @@ -4,18 +4,6 @@ # cf run-task getgov-ENVIRONMENT --command 'python manage.py flush' --name flush # cf run-task getgov-ENVIRONMENT --command 'python manage.py load' --name loaddata -# IMPORTANT NOTE: -# When running this workflow, it is important to use the right source branch -# for your target sandbox. For example, if you just recently pushed new migrations -# from SOURCE_BRANCH to TARGET_SANDBOX, you must use that same SOURCE_BRANCH when -# running this workflow (NOT main) or else your sandbox will generate an error -# due to a db mismatch. -# -# This means in the Github workflow settings you would select the following inputs: -# -# "Use workflow from": SOURCE_BRANCH -# "Which environment should we flush and re-load data for?"": TARGET_SANDBOX - name: Reset database run-name: Reset database for ${{ github.event.inputs.environment }} From 477c4f5bd64746873684e94c8e3880a1da94e23c Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 5 Aug 2024 13:41:43 -0500 Subject: [PATCH 30/44] Remove logic that causes bug --- .../models/utility/generic_helper.py | 20 +++++++++++----- src/registrar/tests/test_models.py | 23 +++---------------- src/registrar/tests/test_views_request.py | 2 +- 3 files changed, 18 insertions(+), 27 deletions(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 4f74b4163..df9a969df 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -4,7 +4,6 @@ import time import logging from urllib.parse import urlparse, urlunparse, urlencode - logger = logging.getLogger(__name__) @@ -82,9 +81,13 @@ class CreateOrUpdateOrganizationTypeHelper: def _handle_new_instance(self): # == Check for invalid conditions before proceeding == # - should_proceed = self._validate_new_instance() - if not should_proceed: + try: + should_proceed = self._validate_new_instance() + if not should_proceed: + return None + except ValueError: return None + # == Program flow will halt here if there is no reason to update == # # == Update the linked values == # @@ -104,6 +107,7 @@ class CreateOrUpdateOrganizationTypeHelper: def _handle_existing_instance(self, force_update_when_no_changes_are_found=False): # == Init variables == # try: + # Instance is already in the database, fetch its current state current_instance = self.sender.objects.get(id=self.instance.id) @@ -174,8 +178,8 @@ class CreateOrUpdateOrganizationTypeHelper: self.instance.organization_type = generic_org_type else: # This can only happen with manual data tinkering, which causes these to be out of sync. - if self.instance.is_election_board is None: - self.instance.is_election_board = False + # if self.instance.is_election_board is None: + # self.instance.is_election_board = False if self.instance.is_election_board: self.instance.organization_type = self.generic_org_to_org_map[generic_org_type] @@ -219,12 +223,15 @@ class CreateOrUpdateOrganizationTypeHelper: self.instance.is_election_board = None self.instance.generic_org_type = None - def _validate_new_instance(self): + def _validate_new_instance(self) -> bool: """ Validates whether a new instance of DomainRequest or DomainInformation can proceed with the update based on the consistency between organization_type, generic_org_type, and is_election_board. Returns a boolean determining if execution should proceed or not. + + Raises: + ValueError if there is a mismatch between organization_type, generic_org_type, and is_election_board """ # We conditionally accept both of these values to exist simultaneously, as long as @@ -249,6 +256,7 @@ class CreateOrUpdateOrganizationTypeHelper: "Cannot add organization_type and generic_org_type simultaneously " "when generic_org_type, is_election_board, and organization_type values do not match." ) + logger.error("_validate_new_instance: %s", message) raise ValueError(message) return True diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index b50525e27..9b80e556d 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1495,13 +1495,6 @@ class TestDomainRequestCustomSave(TestCase): self.assertEqual(domain_request.is_election_board, False) self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) - # Try reverting setting an invalid value for election board (should revert to False) - domain_request.is_election_board = None - domain_request.save() - - self.assertEqual(domain_request.is_election_board, False) - self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) - @less_console_noise_decorator def test_create_or_update_organization_type_existing_instance_updates_generic_org_type(self): """Test create_or_update_organization_type when modifying generic_org_type on an existing instance.""" @@ -1654,13 +1647,6 @@ class TestDomainInformationCustomSave(TestCase): self.assertEqual(domain_information.is_election_board, False) self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) - # Try reverting setting an invalid value for election board (should revert to False) - domain_information.is_election_board = None - domain_information.save() - - self.assertEqual(domain_information.is_election_board, False) - self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) - @less_console_noise_decorator def test_create_or_update_organization_type_existing_instance_updates_generic_org_type(self): """Test create_or_update_organization_type when modifying generic_org_type on an existing instance.""" @@ -1858,8 +1844,7 @@ class TestDomainRequestIncomplete(TestCase): self.assertTrue(self.domain_request._is_state_or_territory_complete()) self.domain_request.is_election_board = None self.domain_request.save() - # is_election_board will overwrite to False bc of _update_org_type_from_generic_org_and_election - self.assertTrue(self.domain_request._is_state_or_territory_complete()) + self.assertFalse(self.domain_request._is_state_or_territory_complete()) @less_console_noise_decorator def test_is_tribal_complete(self): @@ -1882,8 +1867,7 @@ class TestDomainRequestIncomplete(TestCase): self.assertTrue(self.domain_request._is_county_complete()) self.domain_request.is_election_board = None self.domain_request.save() - # is_election_board will overwrite to False bc of _update_org_type_from_generic_org_and_election - self.assertTrue(self.domain_request._is_county_complete()) + self.assertFalse(self.domain_request._is_county_complete()) @less_console_noise_decorator def test_is_city_complete(self): @@ -1893,8 +1877,7 @@ class TestDomainRequestIncomplete(TestCase): self.assertTrue(self.domain_request._is_city_complete()) self.domain_request.is_election_board = None self.domain_request.save() - # is_election_board will overwrite to False bc of _update_org_type_from_generic_org_and_election - self.assertTrue(self.domain_request._is_city_complete()) + self.assertFalse(self.domain_request._is_city_complete()) @less_console_noise_decorator def test_is_special_district_complete(self): diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index 0cee9d563..efad41d8f 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -2979,7 +2979,7 @@ class TestWizardUnlockingSteps(TestWithUser, WebTest): expected_dict = [] self.assertEqual(unlocked_steps, expected_dict) - @less_console_noise_decorator + # @less_console_noise_decorator def test_unlocked_steps_full_domain_request(self): """Test when all fields in the domain request are filled.""" From 9a531d4cec8ca264976bd2def71558a4831371ad Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 5 Aug 2024 12:46:27 -0600 Subject: [PATCH 31/44] Updated migration-troubleshooting MD to have a note on usage of reset-db workflow for migration troubleshooting --- docs/developer/migration-troubleshooting.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/developer/migration-troubleshooting.md b/docs/developer/migration-troubleshooting.md index 22a02503d..395d23ee8 100644 --- a/docs/developer/migration-troubleshooting.md +++ b/docs/developer/migration-troubleshooting.md @@ -30,7 +30,19 @@ You should end up with `40_some_migration_from_main`, `41_local_migration` Alternatively, assuming that the conflicting migrations are not dependent on each other, you can manually edit the migration file such that your new migration is incremented by one (file name, and definition inside the file) but this approach is not recommended. -### Scenario 2: Conflicting migrations on sandbox +### Scenario 2: Conflicting migrations on sandbox (can be fixed with GH workflow) +A 500 error on a sanbox after a fresh push usually indicates a migration issue. +Most of the time, these migration issues can easily be fixed by simply running the +"reset-db" workflow in Github. + +For the workflow, select the following inputs before running it; +"Use workflow from": Branch-main +"Which environment should we flush and re-load data for?" + +This is not a cure-all since it simply flushes and re-runs migrations against your sandbox. +If running this workflow does not solve your issue, proceed examining the scenarios below. + +### Scenario 3: Conflicting migrations on sandbox (cannot be fixed with GH workflow) This occurs when the logs return the following: >Conflicting migrations detected; multiple leaf nodes in the migration graph: (0040_example, 0041_example in base). From 62957d2c5518b889a3cab6b4bd9ec770371ca877 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 5 Aug 2024 13:47:57 -0500 Subject: [PATCH 32/44] clean up testing changes --- src/registrar/models/utility/generic_helper.py | 11 ++--------- src/registrar/tests/test_views_request.py | 2 +- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index df9a969df..6594a77a3 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -81,11 +81,8 @@ class CreateOrUpdateOrganizationTypeHelper: def _handle_new_instance(self): # == Check for invalid conditions before proceeding == # - try: - should_proceed = self._validate_new_instance() - if not should_proceed: - return None - except ValueError: + should_proceed = self._validate_new_instance() + if not should_proceed: return None # == Program flow will halt here if there is no reason to update == # @@ -177,10 +174,6 @@ class CreateOrUpdateOrganizationTypeHelper: self.instance.is_election_board = None self.instance.organization_type = generic_org_type else: - # This can only happen with manual data tinkering, which causes these to be out of sync. - # if self.instance.is_election_board is None: - # self.instance.is_election_board = False - if self.instance.is_election_board: self.instance.organization_type = self.generic_org_to_org_map[generic_org_type] else: diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index efad41d8f..0cee9d563 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -2979,7 +2979,7 @@ class TestWizardUnlockingSteps(TestWithUser, WebTest): expected_dict = [] self.assertEqual(unlocked_steps, expected_dict) - # @less_console_noise_decorator + @less_console_noise_decorator def test_unlocked_steps_full_domain_request(self): """Test when all fields in the domain request are filled.""" From 97d2c5e3a2a1f5dfda7fc2c4d29103bf6228ff23 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 5 Aug 2024 15:17:22 -0500 Subject: [PATCH 33/44] Remove some small whitespace changes --- src/registrar/models/utility/generic_helper.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 6594a77a3..e0a3aa183 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -84,7 +84,6 @@ class CreateOrUpdateOrganizationTypeHelper: should_proceed = self._validate_new_instance() if not should_proceed: return None - # == Program flow will halt here if there is no reason to update == # # == Update the linked values == # @@ -104,7 +103,6 @@ class CreateOrUpdateOrganizationTypeHelper: def _handle_existing_instance(self, force_update_when_no_changes_are_found=False): # == Init variables == # try: - # Instance is already in the database, fetch its current state current_instance = self.sender.objects.get(id=self.instance.id) From 57da90d8a0e56ab48049c263756b8ba332c8e827 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 5 Aug 2024 15:36:28 -0500 Subject: [PATCH 34/44] add more test cases --- src/registrar/tests/test_models.py | 56 +++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 9b80e556d..30ce36b1e 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1495,6 +1495,28 @@ class TestDomainRequestCustomSave(TestCase): self.assertEqual(domain_request.is_election_board, False) self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + @less_console_noise_decorator + def test_create_or_update_organization_type_existing_instance_updates_election_board_to_none(self): + """Test create_or_update_organization_type for an existing instance.""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=False, + ) + domain_request.is_election_board = True + domain_request.save() + + self.assertEqual(domain_request.is_election_board, True) + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION) + + # Try reverting the election board value + domain_request.is_election_board = None + domain_request.save() + + self.assertEqual(domain_request.is_election_board, None) + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + @less_console_noise_decorator def test_create_or_update_organization_type_existing_instance_updates_generic_org_type(self): """Test create_or_update_organization_type when modifying generic_org_type on an existing instance.""" @@ -1647,6 +1669,30 @@ class TestDomainInformationCustomSave(TestCase): self.assertEqual(domain_information.is_election_board, False) self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + @less_console_noise_decorator + def test_create_or_update_organization_type_existing_instance_updates_election_board_to_none(self): + """Test create_or_update_organization_type for an existing instance.""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=False, + ) + domain_information = DomainInformation.create_from_da(domain_request) + domain_information.is_election_board = True + domain_information.save() + + self.assertEqual(domain_information.is_election_board, True) + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION) + + # Try reverting the election board value + domain_information.is_election_board = None + domain_information.save() + domain_information.refresh_from_db() + + self.assertEqual(domain_information.is_election_board, None) + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + @less_console_noise_decorator def test_create_or_update_organization_type_existing_instance_updates_generic_org_type(self): """Test create_or_update_organization_type when modifying generic_org_type on an existing instance.""" @@ -1853,10 +1899,11 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.is_election_board = False self.domain_request.save() self.assertTrue(self.domain_request._is_tribal_complete()) - self.domain_request.tribe_name = None self.domain_request.is_election_board = None self.domain_request.save() - # is_election_board will overwrite to False bc of _update_org_type_from_generic_org_and_election + self.assertFalse(self.domain_request._is_tribal_complete()) + self.domain_request.tribe_name = None + self.domain_request.save() self.assertFalse(self.domain_request._is_tribal_complete()) @less_console_noise_decorator @@ -1886,10 +1933,11 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.is_election_board = False self.domain_request.save() self.assertTrue(self.domain_request._is_special_district_complete()) - self.domain_request.about_your_organization = None self.domain_request.is_election_board = None self.domain_request.save() - # is_election_board will overwrite to False bc of _update_org_type_from_generic_org_and_election + self.assertFalse(self.domain_request._is_special_district_complete()) + self.domain_request.about_your_organization = None + self.domain_request.save() self.assertFalse(self.domain_request._is_special_district_complete()) @less_console_noise_decorator From dafac977763011fd5b0a156aa3a00d558eb79ced Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 5 Aug 2024 18:14:35 -0500 Subject: [PATCH 35/44] print values in error log --- src/registrar/models/utility/generic_helper.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index e0a3aa183..a894d5836 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -245,7 +245,8 @@ class CreateOrUpdateOrganizationTypeHelper: if election_board_mismatch or org_type_mismatch: message = ( "Cannot add organization_type and generic_org_type simultaneously " - "when generic_org_type, is_election_board, and organization_type values do not match." + "when generic_org_type ({}), is_election_board ({}), and organization_type ({}) values do not match." + .format(generic_org_type, self.instance.is_election_board, organization_type) ) logger.error("_validate_new_instance: %s", message) raise ValueError(message) From f339266bd0a1d323fbb92e0c86f94debe4af93d8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 6 Aug 2024 08:07:19 -0600 Subject: [PATCH 36/44] Update user.py --- src/registrar/models/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 451ebcc49..ab70364e7 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -268,7 +268,7 @@ class User(AbstractUser): def has_view_all_domains_permission(self): """Determines if the current user can view all available domains in a given portfolio""" - return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + return self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) @classmethod def needs_identity_verification(cls, email, uuid): From 9aa8224103fd625cee11b5c7656d9b5e7a3ed9fd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 6 Aug 2024 08:15:55 -0600 Subject: [PATCH 37/44] Fix merge conflict pt 2 --- src/registrar/tests/test_reports.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 6b8cb8882..bf3e4d528 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -7,6 +7,7 @@ from registrar.models import ( UserDomainRole, ) from registrar.models import Portfolio, User +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.utility.csv_export import ( DomainDataFull, DomainDataType, @@ -335,7 +336,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.domain_3.domain_info.save() # Set up user permissions - self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] self.user.save() self.user.refresh_from_db() @@ -353,7 +354,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.assertNotIn(self.domain_2.name, csv_content) # Test the output for readonly admin - self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] self.user.save() self.assertIn(self.domain_1.name, csv_content) @@ -361,7 +362,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.assertNotIn(self.domain_2.name, csv_content) # Get the csv content - self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_MEMBER] self.user.save() csv_content = self._run_domain_data_type_user_export(request) From b17252dd1260f70ee5724033a1b10715028a8061 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 6 Aug 2024 08:19:33 -0600 Subject: [PATCH 38/44] Remove unused import --- src/registrar/tests/test_reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index bf3e4d528..52aaa8c38 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -6,7 +6,7 @@ from registrar.models import ( Domain, UserDomainRole, ) -from registrar.models import Portfolio, User +from registrar.models import Portfolio from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.utility.csv_export import ( DomainDataFull, From bf1d3f1ad71c14ee8f6701316651739c971ed542 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 6 Aug 2024 14:39:10 -0500 Subject: [PATCH 39/44] Clarify test names --- src/registrar/tests/test_models.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 30ce36b1e..8c69517e9 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1496,13 +1496,15 @@ class TestDomainRequestCustomSave(TestCase): self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) @less_console_noise_decorator - def test_create_or_update_organization_type_existing_instance_updates_election_board_to_none(self): - """Test create_or_update_organization_type for an existing instance.""" + def test_existing_instance_updates_election_board_to_none(self): + """Test create_or_update_organization_type for an existing instance, first to True and then to None. + Start our with is_election_board as none to simulate a situation where the request was started, but + only completed to the point of filling out the generic_org_type.""" domain_request = completed_domain_request( status=DomainRequest.DomainRequestStatus.STARTED, name="started.gov", generic_org_type=DomainRequest.OrganizationChoices.CITY, - is_election_board=False, + is_election_board=None, ) domain_request.is_election_board = True domain_request.save() @@ -1510,7 +1512,7 @@ class TestDomainRequestCustomSave(TestCase): self.assertEqual(domain_request.is_election_board, True) self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION) - # Try reverting the election board value + # Try reverting the election board value. domain_request.is_election_board = None domain_request.save() @@ -1670,13 +1672,15 @@ class TestDomainInformationCustomSave(TestCase): self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) @less_console_noise_decorator - def test_create_or_update_organization_type_existing_instance_updates_election_board_to_none(self): - """Test create_or_update_organization_type for an existing instance.""" + def test_existing_instance_update_election_board_to_none(self): + """Test create_or_update_organization_type for an existing instance, first to True and then to None. + Start our with is_election_board as none to simulate a situation where the request was started, but + only completed to the point of filling out the generic_org_type.""" domain_request = completed_domain_request( status=DomainRequest.DomainRequestStatus.STARTED, name="started.gov", generic_org_type=DomainRequest.OrganizationChoices.CITY, - is_election_board=False, + is_election_board=None, ) domain_information = DomainInformation.create_from_da(domain_request) domain_information.is_election_board = True From a74e0beb6f93f256f081bc8335d93b07e4a36413 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 6 Aug 2024 16:07:19 -0500 Subject: [PATCH 40/44] Update helper to handle None in is_election_board --- src/registrar/models/utility/generic_helper.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index a894d5836..1ca6a6ca2 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -240,7 +240,7 @@ class CreateOrUpdateOrganizationTypeHelper: is_election_type = "_election" in organization_type can_have_election_board = organization_type in self.generic_org_to_org_map - election_board_mismatch = (is_election_type != self.instance.is_election_board) and can_have_election_board + election_board_mismatch = (is_election_type and (not self.instance.is_election_board or self.instance.is_election_board == None)) and can_have_election_board org_type_mismatch = mapped_org_type is not None and (generic_org_type != mapped_org_type) if election_board_mismatch or org_type_mismatch: message = ( @@ -248,6 +248,8 @@ class CreateOrUpdateOrganizationTypeHelper: "when generic_org_type ({}), is_election_board ({}), and organization_type ({}) values do not match." .format(generic_org_type, self.instance.is_election_board, organization_type) ) + message = "Mismatch on election board, {}".format(message) if election_board_mismatch else message + message = "Mistmatch on org type, {}".format(message) if org_type_mismatch else message logger.error("_validate_new_instance: %s", message) raise ValueError(message) From 734f800cfcd4bf71f29b4d707b07cf3e7bfb1638 Mon Sep 17 00:00:00 2001 From: katypies Date: Wed, 7 Aug 2024 15:47:52 -0600 Subject: [PATCH 41/44] Adjust copy for the finish profile form, too --- src/registrar/templates/includes/finish_profile_form.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index 88f7a73af..5176b0462 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -53,11 +53,11 @@ {% endwith %}
- {% public_site_url "help/account-management/#get-help-with-login.gov" as login_help_url %} + {% public_site_url "help/account-management/#update-your-contact-information" as login_help_url %} {% with show_readonly=True add_class="display-none" group_classes="usa-form-editable usa-form-editable padding-top-2 bold-usa-label" %} {% with link_href=login_help_url %} - {% with sublabel_text="We recommend using your work email for your .gov account. If the wrong email is displayed below, you’ll need to update your Login.gov account and log back in. Get help with your Login.gov account." %} - {% with link_text="Get help with your Login.gov account" target_blank=True do_not_show_max_chars=True %} + {% with sublabel_text="We recommend using your work email for your .gov account. If the wrong email is displayed below, you’ll need to update your Login.gov account and log back in. Get help with updating your contact information." %} + {% with link_text="Get help with updating your contact information" target_blank=True do_not_show_max_chars=True %} {% input_with_errors form.email %} {% endwith %} {% endwith %} From 8abe94389737ad5aacf7341105d92c79b9dfc163 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 8 Aug 2024 13:39:49 -0500 Subject: [PATCH 42/44] review changes --- src/registrar/models/utility/generic_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 1ca6a6ca2..42ddd2f2f 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -240,7 +240,7 @@ class CreateOrUpdateOrganizationTypeHelper: is_election_type = "_election" in organization_type can_have_election_board = organization_type in self.generic_org_to_org_map - election_board_mismatch = (is_election_type and (not self.instance.is_election_board or self.instance.is_election_board == None)) and can_have_election_board + election_board_mismatch = is_election_type and not self.instance.is_election_board and can_have_election_board org_type_mismatch = mapped_org_type is not None and (generic_org_type != mapped_org_type) if election_board_mismatch or org_type_mismatch: message = ( From b742c5d57743afe6b0534a107f5ae69a1d5cde40 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 8 Aug 2024 14:18:35 -0500 Subject: [PATCH 43/44] reduce line length --- src/registrar/models/utility/generic_helper.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 42ddd2f2f..8c44a3bdd 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -221,8 +221,8 @@ class CreateOrUpdateOrganizationTypeHelper: Returns a boolean determining if execution should proceed or not. - Raises: - ValueError if there is a mismatch between organization_type, generic_org_type, and is_election_board + Raises: + ValueError if there is a mismatch between organization_type, generic_org_type, and is_election_board """ # We conditionally accept both of these values to exist simultaneously, as long as @@ -240,13 +240,16 @@ class CreateOrUpdateOrganizationTypeHelper: is_election_type = "_election" in organization_type can_have_election_board = organization_type in self.generic_org_to_org_map - election_board_mismatch = is_election_type and not self.instance.is_election_board and can_have_election_board + election_board_mismatch = ( + is_election_type and not self.instance.is_election_board and can_have_election_board + ) org_type_mismatch = mapped_org_type is not None and (generic_org_type != mapped_org_type) if election_board_mismatch or org_type_mismatch: message = ( - "Cannot add organization_type and generic_org_type simultaneously " - "when generic_org_type ({}), is_election_board ({}), and organization_type ({}) values do not match." - .format(generic_org_type, self.instance.is_election_board, organization_type) + "Cannot add organization_type and generic_org_type simultaneously when" + "generic_org_type ({}), is_election_board ({}), and organization_type ({}) don't match.".format( + generic_org_type, self.instance.is_election_board, organization_type + ) ) message = "Mismatch on election board, {}".format(message) if election_board_mismatch else message message = "Mistmatch on org type, {}".format(message) if org_type_mismatch else message From 37145866d91f5bb58b6b83f7c71e4f103fb8cdc6 Mon Sep 17 00:00:00 2001 From: katypies Date: Thu, 8 Aug 2024 13:34:57 -0600 Subject: [PATCH 44/44] Fix copy and link specifically to the email address content in the help page --- src/registrar/templates/includes/finish_profile_form.html | 6 +++--- src/registrar/templates/includes/profile_form.html | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index 6ffff2950..119862269 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -57,12 +57,12 @@ {% input_with_errors form.title %} {% endwith %} - {% public_site_url "help/account-management/#update-your-contact-information" as login_help_url %} + {% public_site_url "help/account-management/#email-address" as login_help_url %} {% with toggleable_input=True add_class="display-none" group_classes="usa-form-editable usa-form-editable padding-top-2 bold-usa-label" %} {% with link_href=login_help_url %} - {% with sublabel_text="We recommend using your work email for your .gov account. If the wrong email is displayed below, you’ll need to update your Login.gov account and log back in. Get help with updating your contact information." %} - {% with link_text="Get help with updating your contact information" target_blank=True do_not_show_max_chars=True %} + {% with sublabel_text="We recommend using a Login.gov account that's only connected to your work email address. If you need to change your email, you'll need to make a change to your Login.gov account. Get help with updating your email address." %} + {% with link_text="Get help with updating your email address" target_blank=True do_not_show_max_chars=True %} {% input_with_errors form.email %} {% endwith %} {% endwith %} diff --git a/src/registrar/templates/includes/profile_form.html b/src/registrar/templates/includes/profile_form.html index dbc0415a9..bd4b0209b 100644 --- a/src/registrar/templates/includes/profile_form.html +++ b/src/registrar/templates/includes/profile_form.html @@ -30,11 +30,11 @@ {% input_with_errors form.title %} - {% public_site_url "help/account-management/#update-your-contact-information" as login_help_url %} + {% public_site_url "help/account-management/#email-address" as login_help_url %} {% with link_href=login_help_url %} - {% with sublabel_text="We recommend using your work email for your .gov account. If the wrong email is displayed below, you’ll need to update your Login.gov account and log back in. Get help with updating your contact information." %} - {% with link_text="Get help with updating your contact information" %} + {% with sublabel_text="We recommend using a Login.gov account that's only connected to your work email address. If you need to change your email, you'll need to make a change to your Login.gov account. Get help with updating your email address." %} + {% with link_text="Get help with updating your email address" %} {% with target_blank=True %} {% with do_not_show_max_chars=True %} {% input_with_errors form.email %}