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). diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index bd2af40b7..ab70364e7 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 registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices @@ -265,6 +266,10 @@ class User(AbstractUser): UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS ) or self._has_portfolio_permission(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""" + return self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + @classmethod def needs_identity_verification(cls, email, uuid): """A method used by our oidc classes to test whether a user needs email/uuid verification @@ -406,3 +411,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 get_user_domain_ids(self, request): + """Returns either the domains ids associated with this user on UserDomainRole or Portfolio""" + 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/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 4f74b4163..8c44a3bdd 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__) @@ -173,10 +172,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: @@ -219,12 +214,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 @@ -242,13 +240,20 @@ 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 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." + "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 + logger.error("_validate_new_instance: %s", message) raise ValueError(message) return True diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index b79b69ebc..63924bc1d 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" 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 64eddec41..a45abd757 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -37,6 +37,7 @@ + {% if user_domain_count and user_domain_count > 0 %}
@@ -46,6 +47,7 @@
+ {% endif %} {% if has_domains_portfolio_permission %}
diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index 42a2de1a5..119862269 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -56,12 +56,13 @@ {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable padding-top-2" %} {% input_with_errors form.title %} {% endwith %} - - {% public_site_url "help/account-management/#get-help-with-login.gov" 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 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 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 966b92b01..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/#get-help-with-login.gov" 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 your Login.gov account." %} - {% with link_text="Get help with your Login.gov account" %} + {% 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 %} 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/tests/test_models.py b/src/registrar/tests/test_models.py index b50525e27..8c69517e9 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1495,11 +1495,28 @@ 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) + @less_console_noise_decorator + 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=None, + ) + 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, False) + self.assertEqual(domain_request.is_election_board, None) self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) @less_console_noise_decorator @@ -1654,11 +1671,30 @@ 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 + @less_console_noise_decorator + 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=None, + ) + domain_information = DomainInformation.create_from_da(domain_request) + domain_information.is_election_board = True domain_information.save() - self.assertEqual(domain_information.is_election_board, False) + 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 @@ -1858,8 +1894,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): @@ -1868,10 +1903,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 @@ -1882,8 +1918,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 +1928,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): @@ -1903,10 +1937,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 diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 74b84834e..52aaa8c38 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -6,6 +6,8 @@ from registrar.models import ( Domain, UserDomainRole, ) +from registrar.models import Portfolio +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.utility.csv_export import ( DomainDataFull, DomainDataType, @@ -32,6 +34,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 +314,80 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.maxDiff = None self.assertEqual(csv_content, expected_content) + @less_console_noise_decorator + @override_flag("organization_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 = [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 = [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 + self.user.portfolio_roles = [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) + 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""" + # 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 d852df5db..db961a36d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -578,10 +578,9 @@ class DomainDataTypeUser(DomainDataType): if request is None or not hasattr(request, "user") or not request.user: # 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) - return Q(domain__id__in=domain_ids) + else: + # Get all domains the user is associated with + 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 5bb9b037f..06c211227 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -1,13 +1,11 @@ import logging from django.http import JsonResponse from django.core.paginator import Paginator -from registrar.models import UserDomainRole, 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.domain_information import DomainInformation - logger = logging.getLogger(__name__) diff --git a/src/registrar/views/index.py b/src/registrar/views/index.py index 498434dca..53900a4a7 100644 --- a/src/registrar/views/index.py +++ b/src/registrar/views/index.py @@ -5,8 +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(request).count() return render(request, "home.html", context) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index a7c7d1356..b9042d617 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -22,7 +22,10 @@ class PortfolioDomainsView(PortfolioDomainsPermissionView, View): template_name = "portfolio_domains.html" def get(self, request): - return render(request, "portfolio_domains.html") + context = {} + 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) class PortfolioDomainRequestsView(PortfolioDomainRequestsPermissionView, View):