From 17274a606d7b3a8bb44d34934fd53491e53267e4 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Wed, 17 Jul 2024 14:09:50 -0700 Subject: [PATCH 01/34] Removed unused comment job from manual deploy --- .github/workflows/deploy-sandbox.yaml | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/.github/workflows/deploy-sandbox.yaml b/.github/workflows/deploy-sandbox.yaml index 4bd7f99dd..7f8637a04 100644 --- a/.github/workflows/deploy-sandbox.yaml +++ b/.github/workflows/deploy-sandbox.yaml @@ -64,20 +64,4 @@ jobs: cf_password: ${{ secrets[env.CF_PASSWORD] }} cf_org: cisa-dotgov cf_space: ${{ env.ENVIRONMENT }} - cf_manifest: ops/manifests/manifest-${{ env.ENVIRONMENT }}.yaml - comment: - runs-on: ubuntu-latest - needs: [variables, deploy] - steps: - - uses: actions/github-script@v6 - env: - ENVIRONMENT: ${{ needs.variables.outputs.environment }} - with: - github-token: ${{secrets.GITHUB_TOKEN}} - script: | - github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body: 'šŸ„³ Successfully deployed to developer sandbox **[${{ env.ENVIRONMENT }}](https://getgov-${{ env.ENVIRONMENT }}.app.cloud.gov/)**.' - }) \ No newline at end of file + cf_manifest: ops/manifests/manifest-${{ env.ENVIRONMENT }}.yaml \ No newline at end of file From f2fd764d6735c4a8bc177c5466ba6cfea6cf668c Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Wed, 17 Jul 2024 14:20:23 -0700 Subject: [PATCH 02/34] Correct removed comment job in deploy workflow --- ...anch-to-sandbox.yaml => deploy-manual.yaml} | 16 ---------------- .github/workflows/deploy-sandbox.yaml | 18 +++++++++++++++++- 2 files changed, 17 insertions(+), 17 deletions(-) rename .github/workflows/{deploy-branch-to-sandbox.yaml => deploy-manual.yaml} (78%) diff --git a/.github/workflows/deploy-branch-to-sandbox.yaml b/.github/workflows/deploy-manual.yaml similarity index 78% rename from .github/workflows/deploy-branch-to-sandbox.yaml rename to .github/workflows/deploy-manual.yaml index 14a3d8ef8..97415a0d9 100644 --- a/.github/workflows/deploy-branch-to-sandbox.yaml +++ b/.github/workflows/deploy-manual.yaml @@ -71,20 +71,4 @@ jobs: cf_org: cisa-dotgov cf_space: ${{ env.ENVIRONMENT }} cf_manifest: ops/manifests/manifest-${{ env.ENVIRONMENT }}.yaml - comment: - runs-on: ubuntu-latest - needs: [deploy] - steps: - - uses: actions/github-script@v6 - env: - ENVIRONMENT: ${{ github.event.inputs.environment }} - with: - github-token: ${{secrets.GITHUB_TOKEN}} - script: | - github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - body: 'šŸ„³ Successfully deployed to developer sandbox **[${{ env.ENVIRONMENT }}](https://getgov-${{ env.ENVIRONMENT }}.app.cloud.gov/)**.' - }) - diff --git a/.github/workflows/deploy-sandbox.yaml b/.github/workflows/deploy-sandbox.yaml index 7f8637a04..4bd7f99dd 100644 --- a/.github/workflows/deploy-sandbox.yaml +++ b/.github/workflows/deploy-sandbox.yaml @@ -64,4 +64,20 @@ jobs: cf_password: ${{ secrets[env.CF_PASSWORD] }} cf_org: cisa-dotgov cf_space: ${{ env.ENVIRONMENT }} - cf_manifest: ops/manifests/manifest-${{ env.ENVIRONMENT }}.yaml \ No newline at end of file + cf_manifest: ops/manifests/manifest-${{ env.ENVIRONMENT }}.yaml + comment: + runs-on: ubuntu-latest + needs: [variables, deploy] + steps: + - uses: actions/github-script@v6 + env: + ENVIRONMENT: ${{ needs.variables.outputs.environment }} + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: 'šŸ„³ Successfully deployed to developer sandbox **[${{ env.ENVIRONMENT }}](https://getgov-${{ env.ENVIRONMENT }}.app.cloud.gov/)**.' + }) \ No newline at end of file From 7820c362433d714c1e47a9aa0de7ad75dcd1967b Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Wed, 17 Jul 2024 14:21:55 -0700 Subject: [PATCH 03/34] Revert workflow name for testing purposes --- .../{deploy-manual.yaml => deploy-branch-to-sandbox.yaml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/workflows/{deploy-manual.yaml => deploy-branch-to-sandbox.yaml} (100%) diff --git a/.github/workflows/deploy-manual.yaml b/.github/workflows/deploy-branch-to-sandbox.yaml similarity index 100% rename from .github/workflows/deploy-manual.yaml rename to .github/workflows/deploy-branch-to-sandbox.yaml From 3e5401a549d0fc4b8abb756730482a1d6e23dbc3 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Mon, 22 Jul 2024 11:31:20 -0700 Subject: [PATCH 04/34] Add new sandboxes to deploy-manual workflow. Rename workflow name to deploy-manual --- .../{deploy-branch-to-sandbox.yaml => deploy-manual.yaml} | 0 ops/scripts/create_dev_sandbox.sh | 6 +++++- 2 files changed, 5 insertions(+), 1 deletion(-) rename .github/workflows/{deploy-branch-to-sandbox.yaml => deploy-manual.yaml} (100%) diff --git a/.github/workflows/deploy-branch-to-sandbox.yaml b/.github/workflows/deploy-manual.yaml similarity index 100% rename from .github/workflows/deploy-branch-to-sandbox.yaml rename to .github/workflows/deploy-manual.yaml diff --git a/ops/scripts/create_dev_sandbox.sh b/ops/scripts/create_dev_sandbox.sh index 676fcf7ae..b3a970584 100755 --- a/ops/scripts/create_dev_sandbox.sh +++ b/ops/scripts/create_dev_sandbox.sh @@ -112,10 +112,14 @@ sed -i '' '/ - development/ {a\ - '"$1"' }' .github/workflows/reset-db.yaml -sed -i '' '/ - development/ {a\ +sed -i '' '/ - backup/ {a\ - '"$1"' }' .github/workflows/migrate.yaml +sed -i '' '/ - development/ {a\ + - '"$1"' +}' .github/workflows/deploy-manual.yaml + sed -i '' '/${{startsWith(github.head_ref, / {a\ || startsWith(github.head_ref, '"'$1'"') }' .github/workflows/deploy-sandbox.yaml From 05d86ad11caf9bc5776e8af8e28112f3c0d79cc6 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Mon, 22 Jul 2024 15:37:25 -0500 Subject: [PATCH 05/34] Link to new FSM diagram in developer README --- docs/developer/README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index f63f01938..e978ab666 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -357,4 +357,7 @@ Then, copy the variables under the section labled `s3`. 1. On the app, navigate to `\admin`. 2. Under models, click `Waffle flags`. 3. Click the `disable_email_sending` record. This should exist by default, if not - create one with that name. -4. (Important) Set the field `everyone` to `Yes`. This field overrides all other settings \ No newline at end of file +4. (Important) Set the field `everyone` to `Yes`. This field overrides all other settings + +## Request Flow FSM Diagram +There is a diagram detailing the flow of domain requests and resulting domain objects [here](https://miro.com/app/board/uXjVMuqbLOk=/?moveToWidget=3458764594819017396&cot=14) \ No newline at end of file From 427094edca500c210e9e943b6d0404846ba10c50 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 29 Jul 2024 11:53:34 -0500 Subject: [PATCH 06/34] add another cert format and a cert folder to .gitignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index ddd75475d..f2d82f599 100644 --- a/.gitignore +++ b/.gitignore @@ -7,8 +7,10 @@ docs/research/data/** public/ credentials* +src/certs/ *.pem *.crt +*.cer *.bk From ee18be87c7ed8e64277046713cf88447e7003f55 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Jul 2024 11:39:48 -0400 Subject: [PATCH 07/34] use css for read only styles --- src/registrar/assets/sass/_theme/_forms.scss | 10 +++ ...rtfolio_additional_permissions_and_more.py | 52 ++++++++++++ src/registrar/models/user.py | 17 ++-- .../includes/finish_profile_form.html | 8 +- .../templates/includes/input_read_only.html | 7 ++ .../templates/includes/input_with_errors.html | 8 +- ...donly_input.html => toggleable_input.html} | 0 ...edit_button.html => toggleable_label.html} | 0 .../templates/portfolio_organization.html | 78 ++++++++++-------- src/registrar/templatetags/field_helpers.py | 4 +- src/registrar/tests/test_views_portfolio.py | 80 ++++++++++--------- src/registrar/views/portfolios.py | 3 + 12 files changed, 179 insertions(+), 88 deletions(-) create mode 100644 src/registrar/migrations/0114_alter_user_portfolio_additional_permissions_and_more.py create mode 100644 src/registrar/templates/includes/input_read_only.html rename src/registrar/templates/includes/{readonly_input.html => toggleable_input.html} (100%) rename src/registrar/templates/includes/{label_with_edit_button.html => toggleable_label.html} (100%) diff --git a/src/registrar/assets/sass/_theme/_forms.scss b/src/registrar/assets/sass/_theme/_forms.scss index c025bdb29..0aedfcdba 100644 --- a/src/registrar/assets/sass/_theme/_forms.scss +++ b/src/registrar/assets/sass/_theme/_forms.scss @@ -82,3 +82,13 @@ legend.float-left-tablet + button.float-right-tablet { color: var(--close-button-hover-bg); } } + +.read-only-label { + font-size: size('body', 'sm'); + color: color('primary'); + margin-bottom: units(0.5); +} + +.read-only-value { + margin-top: units(0); +} diff --git a/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions_and_more.py b/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions_and_more.py new file mode 100644 index 000000000..f70c5388c --- /dev/null +++ b/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions_and_more.py @@ -0,0 +1,52 @@ +# Generated by Django 4.2.10 on 2024-07-30 02:51 + +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"), + ("edit_domains", "User is a manager on a domain"), + ("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"), + ("edit_portfolio", "Edit organization"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + migrations.AlterField( + model_name="user", + name="portfolio_roles", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[("organization_admin", "Admin"), ("organization_admin_read_only", "Admin read only")], + max_length=50, + ), + blank=True, + help_text="Select one or more roles.", + null=True, + size=None, + ), + ), + ] diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index b135e30c7..dd826bc11 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -69,7 +69,7 @@ class User(AbstractUser): ORGANIZATION_ADMIN = "organization_admin", "Admin" ORGANIZATION_ADMIN_READ_ONLY = "organization_admin_read_only", "Admin read only" - ORGANIZATION_MEMBER = "organization_member", "Member" + # ORGANIZATION_MEMBER is an abstract role where user.portfolio is true class UserPortfolioPermissionChoices(models.TextChoices): """ """ @@ -89,7 +89,7 @@ class User(AbstractUser): VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests" EDIT_REQUESTS = "edit_requests", "Create and edit requests" - VIEW_PORTFOLIO = "view_portfolio", "View organization" + # VIEW_PORTFOLIO is an abstract permission that returns true when user.portfolio is true EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" PORTFOLIO_ROLE_PERMISSIONS = { @@ -99,17 +99,12 @@ class User(AbstractUser): UserPortfolioPermissionChoices.EDIT_MEMBER, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ], UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, UserPortfolioPermissionChoices.VIEW_MEMBER, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - ], - UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, ], } @@ -280,10 +275,14 @@ class User(AbstractUser): return portfolio_permission in portfolio_permissions - # the methods below are checks for individual portfolio permissions. they are defined here + # the methods below are checks for individual portfolio permissions. They are defined here # to make them easier to call elsewhere throughout the application def has_base_portfolio_permission(self): - return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO) + """Base role/permission, the user is simply linked to a portfolio""" + return self.portfolio is not None + + def has_edit_org_portfolio_permission(self): + return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO) def has_domains_portfolio_permission(self): return ( diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index 88f7a73af..8369511a5 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -35,7 +35,7 @@ - {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable usa-form-editable--no-border padding-top-2" %} + {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable usa-form-editable--no-border padding-top-2" %} {% input_with_errors form.full_name %} {% endwith %} @@ -54,7 +54,7 @@ {% public_site_url "help/account-management/#get-help-with-login.gov" 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 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 %} @@ -64,11 +64,11 @@ {% endwith %} {% endwith %} - {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable padding-top-2" %} + {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable padding-top-2" %} {% input_with_errors form.title %} {% endwith %} - {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable padding-top-2" %} + {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable padding-top-2" %} {% with add_class="usa-input--medium" %} {% input_with_errors form.phone %} {% endwith %} diff --git a/src/registrar/templates/includes/input_read_only.html b/src/registrar/templates/includes/input_read_only.html new file mode 100644 index 000000000..b76f82e8b --- /dev/null +++ b/src/registrar/templates/includes/input_read_only.html @@ -0,0 +1,7 @@ +{% comment %} +Template include for read-only form fields +{% endcomment %} + + +

{{ field.label }}

+

{{ field.value }}

diff --git a/src/registrar/templates/includes/input_with_errors.html b/src/registrar/templates/includes/input_with_errors.html index 623ec0a33..d1e53968e 100644 --- a/src/registrar/templates/includes/input_with_errors.html +++ b/src/registrar/templates/includes/input_with_errors.html @@ -27,8 +27,8 @@ error messages, if necessary. {% endif %} {% if not field.widget_type == "checkbox" %} - {% if show_edit_button %} - {% include "includes/label_with_edit_button.html" with bold_label=True %} + {% if toggleable_label %} + {% include "includes/toggleable_label.html" with bold_label=True %} {% else %} {% include "django/forms/label.html" %} {% endif %} @@ -63,8 +63,8 @@ error messages, if necessary.
{% endif %} - {% if show_readonly %} - {% include "includes/readonly_input.html" %} + {% if toggleable_input %} + {% include "includes/toggleable_input.html" %} {% endif %} {# this is the input field, itself #} diff --git a/src/registrar/templates/includes/readonly_input.html b/src/registrar/templates/includes/toggleable_input.html similarity index 100% rename from src/registrar/templates/includes/readonly_input.html rename to src/registrar/templates/includes/toggleable_input.html diff --git a/src/registrar/templates/includes/label_with_edit_button.html b/src/registrar/templates/includes/toggleable_label.html similarity index 100% rename from src/registrar/templates/includes/label_with_edit_button.html rename to src/registrar/templates/includes/toggleable_label.html diff --git a/src/registrar/templates/portfolio_organization.html b/src/registrar/templates/portfolio_organization.html index 0dede3c32..cc9cf5b6a 100644 --- a/src/registrar/templates/portfolio_organization.html +++ b/src/registrar/templates/portfolio_organization.html @@ -23,42 +23,56 @@

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

-

- The federal agency for your organization canā€™t be updated here. - To suggest an update, email help@get.gov. -

- - {% include "includes/form_errors.html" with form=form %} - - {% include "includes/required_fields.html" %} - -
- {% csrf_token %} - + {% if has_edit_org_portfolio_permission %}

- Federal agency - {{ portfolio }} + The federal agency for your organization canā€™t be updated here. + To suggest an update, email help@get.gov.

- {% input_with_errors form.address_line1 %} + {% include "includes/form_errors.html" with form=form %} + {% include "includes/required_fields.html" %} + + {% csrf_token %} +

Federal agency

+

+ {{ portfolio }} +

+ {% input_with_errors form.address_line1 %} + {% input_with_errors form.address_line2 %} + {% input_with_errors form.city %} + {% input_with_errors form.state_territory %} + {% with add_class="usa-input--small" %} + {% input_with_errors form.zipcode %} + {% endwith %} + +
+ {% else %} +

Federal agency

+

+ {{ portfolio }} +

+ {% if form.address_line1.value is not None %} + {% include "includes/input_read_only.html" with field=form.address_line1 %} + {% endif %} + {% if form.address_line2.value is not None %} + {% include "includes/input_read_only.html" with field=form.address_line2 %} + {% endif %} + {% if form.city.value is not None %} + {% include "includes/input_read_only.html" with field=form.city %} + {% endif %} + {% if form.state_territory.value is not None %} + {% include "includes/input_read_only.html" with field=form.state_territory %} + {% endif %} + {% if form.zipcode.value is not None %} + {% include "includes/input_read_only.html" with field=form.zipcode %} + {% endif %} + {% endif %} - {% input_with_errors form.address_line2 %} - - {% input_with_errors form.city %} - - {% input_with_errors form.state_territory %} - - {% with add_class="usa-input--small" %} - {% input_with_errors form.zipcode %} - {% endwith %} - - -
{% endblock %} diff --git a/src/registrar/templatetags/field_helpers.py b/src/registrar/templatetags/field_helpers.py index b72f77e21..68a803711 100644 --- a/src/registrar/templatetags/field_helpers.py +++ b/src/registrar/templatetags/field_helpers.py @@ -26,7 +26,7 @@ def input_with_errors(context, field=None): # noqa: C901 add_group_class: append to input element's surrounding tag's `class` attribute attr_* - adds or replaces any single html attribute for the input add_error_attr_* - like `attr_*` but only if field.errors is not empty - show_edit_button: shows a simple edit button, and adds display-none to the input field. + toggleable_input: shows a simple edit button, and adds display-none to the input field. Example usage: ``` @@ -92,7 +92,7 @@ def input_with_errors(context, field=None): # noqa: C901 elif key == "add_group_class": group_classes.append(value) - elif key == "show_edit_button": + elif key == "toggleable_input": # Hide the primary input field. # Used such that we can toggle it with JS if "display-none" not in classes: diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 3596bf567..c4cdcc2b2 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -37,25 +37,10 @@ class TestPortfolio(WebTest): User.objects.all().delete() super().tearDown() - @less_console_noise_decorator - def test_middleware_does_not_redirect_if_no_permission(self): - """Test that user with no portfolio permission is not redirected when attempting to access home""" - self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.save() - self.user.refresh_from_db() - with override_flag("organization_feature", active=True): - # This will redirect the user to the portfolio page. - # Follow implicity checks if our redirect is working. - portfolio_page = self.app.get(reverse("home")) - # Assert that we're on the right page - self.assertNotContains(portfolio_page, self.portfolio.organization_name) - @less_console_noise_decorator def test_middleware_does_not_redirect_if_no_portfolio(self): """Test that user with no assigned portfolio is not redirected when attempting to access home""" self.app.set_user(self.user.username) - self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): @@ -67,10 +52,9 @@ class TestPortfolio(WebTest): @less_console_noise_decorator def test_middleware_redirects_to_portfolio_organization_page(self): - """Test that user with VIEW_PORTFOLIO is redirected to portfolio organization page""" + """Test that user with a portfolio is redirected to portfolio organization page""" self.app.set_user(self.user.username) self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): @@ -83,11 +67,10 @@ class TestPortfolio(WebTest): @less_console_noise_decorator def test_middleware_redirects_to_portfolio_domains_page(self): - """Test that user with VIEW_PORTFOLIO and VIEW_ALL_DOMAINS is redirected to portfolio domains page""" + """Test that user with a portfolio and VIEW_ALL_DOMAINS is redirected to portfolio domains page""" self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ] self.user.save() @@ -134,20 +117,47 @@ class TestPortfolio(WebTest): self.assertEqual(response.status_code, 403) @less_console_noise_decorator - def test_portfolio_organization_page_403_when_user_not_have_permission(self): - """Test that user without proper permission is not allowed access to portfolio organization page""" + def test_portfolio_organization_page_read_only(self): + """Test that user with a portfolio can access the portfolio organization page, read only""" self.app.set_user(self.user.username) self.user.portfolio = self.portfolio + self.portfolio.city = "Los Angeles" + self.portfolio.save() self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): - # This will redirect the user to the portfolio page. - # Follow implicity checks if our redirect is working. - response = self.app.get( - reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk}), status=403 - ) - # Assert the response is a 403 Forbidden - self.assertEqual(response.status_code, 403) + response = self.app.get(reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk})) + # Assert the response is a 200 + self.assertEqual(response.status_code, 200) + # The label for Federal agency will always be a h4 + self.assertContains(response, '

Federal agency

') + # The read only label for city will be a h4 + self.assertContains(response, '

City

') + self.assertNotContains(response, 'for="id_city"') + self.assertContains(response, '

Los Angeles

') + + @less_console_noise_decorator + def test_portfolio_organization_page_edit_access(self): + """Test that user with a portfolio can access the portfolio organization page, read only""" + self.app.set_user(self.user.username) + self.user.portfolio = self.portfolio + self.user.portfolio_additional_permissions = [ + User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, + ] + self.portfolio.city = "Los Angeles" + self.portfolio.save() + self.user.save() + self.user.refresh_from_db() + with override_flag("organization_feature", active=True): + response = self.app.get(reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk})) + # Assert the response is a 200 + self.assertEqual(response.status_code, 200) + # The label for Federal agency will always be a h4 + self.assertContains(response, '

Federal agency

') + # The read only label for city will be a h4 + self.assertNotContains(response, '

City

') + self.assertNotContains(response, '

Los Angeles

>') + self.assertContains(response, 'for="id_city"') @less_console_noise_decorator def test_navigation_links_hidden_when_user_not_have_permission(self): @@ -155,7 +165,6 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, ] @@ -176,9 +185,9 @@ class TestPortfolio(WebTest): portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk}) ) - # reducing portfolio permissions to just VIEW_PORTFOLIO, which should remove domains + # removing non-basic portfolio perms, which should remove domains # and domain requests from nav - self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + self.user.portfolio_additional_permissions = [] self.user.save() self.user.refresh_from_db() @@ -194,16 +203,13 @@ class TestPortfolio(WebTest): portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk}) ) - -class TestPortfolioOrganization(TestPortfolio): - + @less_console_noise_decorator def test_portfolio_org_name(self): """Can load portfolio's org name page.""" with override_flag("organization_feature", active=True): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() @@ -214,13 +220,13 @@ class TestPortfolioOrganization(TestPortfolio): page, "The name of your federal agency will be publicly listed as the domain registrant." ) + @less_console_noise_decorator def test_domain_org_name_address_content(self): """Org name and address information appears on the page.""" with override_flag("organization_feature", active=True): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() @@ -232,13 +238,13 @@ class TestPortfolioOrganization(TestPortfolio): # Once in the sidenav, once in the main nav, once in the form self.assertContains(page, "Hotel California", count=3) + @less_console_noise_decorator def test_domain_org_name_address_form(self): """Submitting changes works on the org name address page.""" with override_flag("organization_feature", active=True): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index abd9648ba..c04044664 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -64,6 +64,9 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): """Add additional context data to the template.""" context = super().get_context_data(**kwargs) # no need to add portfolio to request context here + + context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission() + context["has_profile_feature_flag"] = flag_is_active(self.request, "profile_feature") context["has_organization_feature_flag"] = flag_is_active(self.request, "organization_feature") return context From f183090e24bd05461fb7c2b1b084914df26bd0e7 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Jul 2024 12:04:25 -0400 Subject: [PATCH 08/34] fix unit tests --- src/registrar/tests/test_views_portfolio.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index c4cdcc2b2..f28b10516 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -130,9 +130,9 @@ class TestPortfolio(WebTest): # Assert the response is a 200 self.assertEqual(response.status_code, 200) # The label for Federal agency will always be a h4 - self.assertContains(response, '

Federal agency

') + self.assertContains(response, '

Federal agency

') # The read only label for city will be a h4 - self.assertContains(response, '

City

') + self.assertContains(response, '

City

') self.assertNotContains(response, 'for="id_city"') self.assertContains(response, '

Los Angeles

') @@ -153,9 +153,9 @@ class TestPortfolio(WebTest): # Assert the response is a 200 self.assertEqual(response.status_code, 200) # The label for Federal agency will always be a h4 - self.assertContains(response, '

Federal agency

') + self.assertContains(response, '

Federal agency

') # The read only label for city will be a h4 - self.assertNotContains(response, '

City

') + self.assertNotContains(response, '

City

') self.assertNotContains(response, '

Los Angeles

>') self.assertContains(response, 'for="id_city"') From 6e900bc501cf00238b006fff9719f54de287bc13 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Jul 2024 12:35:17 -0400 Subject: [PATCH 09/34] fix unit tests --- src/registrar/tests/test_views_portfolio.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index f28b10516..089b710aa 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -134,7 +134,7 @@ class TestPortfolio(WebTest): # The read only label for city will be a h4 self.assertContains(response, '

City

') self.assertNotContains(response, 'for="id_city"') - self.assertContains(response, '

Los Angeles

') + self.assertContains(response, '

Los Angeles

') @less_console_noise_decorator def test_portfolio_organization_page_edit_access(self): @@ -156,7 +156,7 @@ class TestPortfolio(WebTest): self.assertContains(response, '

Federal agency

') # The read only label for city will be a h4 self.assertNotContains(response, '

City

') - self.assertNotContains(response, '

Los Angeles

>') + self.assertNotContains(response, '

Los Angeles

>') self.assertContains(response, 'for="id_city"') @less_console_noise_decorator From 7f212edda5d61ad0b82ead1df71818369fb4959b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Jul 2024 19:58:39 -0400 Subject: [PATCH 10/34] revert some of the tweaks --- ..._user_portfolio_additional_permissions.py} | 17 +------- src/registrar/models/user.py | 14 +++--- src/registrar/tests/test_views_portfolio.py | 43 ++++++++++++++++++- 3 files changed, 51 insertions(+), 23 deletions(-) rename src/registrar/migrations/{0114_alter_user_portfolio_additional_permissions_and_more.py => 0114_alter_user_portfolio_additional_permissions.py} (70%) diff --git a/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions_and_more.py b/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py similarity index 70% rename from src/registrar/migrations/0114_alter_user_portfolio_additional_permissions_and_more.py rename to src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py index f70c5388c..27f6e699f 100644 --- a/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions_and_more.py +++ b/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-07-30 02:51 +# Generated by Django 4.2.10 on 2024-07-30 23:58 import django.contrib.postgres.fields from django.db import migrations, models @@ -26,6 +26,7 @@ class Migration(migrations.Migration): ("view_created_requests", "View created requests"), ("edit_requests", "Create and edit requests"), ("edit_portfolio", "Edit organization"), + ("view_portfolio", "View organization"), ], max_length=50, ), @@ -35,18 +36,4 @@ class Migration(migrations.Migration): size=None, ), ), - migrations.AlterField( - model_name="user", - name="portfolio_roles", - field=django.contrib.postgres.fields.ArrayField( - base_field=models.CharField( - choices=[("organization_admin", "Admin"), ("organization_admin_read_only", "Admin read only")], - max_length=50, - ), - blank=True, - help_text="Select one or more roles.", - null=True, - size=None, - ), - ), ] diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index dd826bc11..48e6dc420 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -64,15 +64,15 @@ class User(AbstractUser): class UserPortfolioRoleChoices(models.TextChoices): """ - Roles make it easier for admins to look at + Roles make it easier for admins to look at groups of users """ ORGANIZATION_ADMIN = "organization_admin", "Admin" ORGANIZATION_ADMIN_READ_ONLY = "organization_admin_read_only", "Admin read only" - # ORGANIZATION_MEMBER is an abstract role where user.portfolio is true + ORGANIZATION_MEMBER = "organization_member", "Member" class UserPortfolioPermissionChoices(models.TextChoices): - """ """ + """We test against permissions to manage access""" VIEW_ALL_DOMAINS = "view_all_domains", "View all domains and domain reports" VIEW_MANAGED_DOMAINS = "view_managed_domains", "View managed domains" @@ -89,8 +89,8 @@ class User(AbstractUser): VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests" EDIT_REQUESTS = "edit_requests", "Create and edit requests" - # VIEW_PORTFOLIO is an abstract permission that returns true when user.portfolio is true EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" + VIEW_PORTFOLIO = "view_portfolio", "View organization" PORTFOLIO_ROLE_PERMISSIONS = { UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ @@ -106,6 +106,9 @@ class User(AbstractUser): UserPortfolioPermissionChoices.VIEW_MEMBER, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, ], + UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + ], } # #### Constants for choice fields #### @@ -278,8 +281,7 @@ class User(AbstractUser): # the methods below are checks for individual portfolio permissions. They are defined here # to make them easier to call elsewhere throughout the application def has_base_portfolio_permission(self): - """Base role/permission, the user is simply linked to a portfolio""" - return self.portfolio is not None + return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO) def has_edit_org_portfolio_permission(self): return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 089b710aa..21b794cf7 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -37,10 +37,25 @@ class TestPortfolio(WebTest): User.objects.all().delete() super().tearDown() + @less_console_noise_decorator + def test_middleware_does_not_redirect_if_no_permission(self): + """Test that user with no portfolio permission is not redirected when attempting to access home""" + self.app.set_user(self.user.username) + self.user.portfolio = self.portfolio + self.user.save() + self.user.refresh_from_db() + with override_flag("organization_feature", active=True): + # This will redirect the user to the portfolio page. + # Follow implicity checks if our redirect is working. + portfolio_page = self.app.get(reverse("home")) + # Assert that we're on the right page + self.assertNotContains(portfolio_page, self.portfolio.organization_name) + @less_console_noise_decorator def test_middleware_does_not_redirect_if_no_portfolio(self): """Test that user with no assigned portfolio is not redirected when attempting to access home""" self.app.set_user(self.user.username) + self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): @@ -52,9 +67,10 @@ class TestPortfolio(WebTest): @less_console_noise_decorator def test_middleware_redirects_to_portfolio_organization_page(self): - """Test that user with a portfolio is redirected to portfolio organization page""" + """Test that user with a portfolio and VIEW_PORTFOLIO is redirected to portfolio organization page""" self.app.set_user(self.user.username) self.user.portfolio = self.portfolio + self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): @@ -67,10 +83,12 @@ class TestPortfolio(WebTest): @less_console_noise_decorator def test_middleware_redirects_to_portfolio_domains_page(self): - """Test that user with a portfolio and VIEW_ALL_DOMAINS is redirected to portfolio domains page""" + """Test that user with a portfolio, VIEW_PORTFOLIO, VIEW_ALL_DOMAINS + is redirected to portfolio domains page""" self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ + User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ] self.user.save() @@ -116,12 +134,29 @@ class TestPortfolio(WebTest): # Assert the response is a 403 Forbidden self.assertEqual(response.status_code, 403) + @less_console_noise_decorator + def test_portfolio_organization_page_403_when_user_not_have_permission(self): + """Test that user without proper permission is not allowed access to portfolio organization page""" + self.app.set_user(self.user.username) + self.user.portfolio = self.portfolio + self.user.save() + self.user.refresh_from_db() + with override_flag("organization_feature", active=True): + # This will redirect the user to the portfolio page. + # Follow implicity checks if our redirect is working. + response = self.app.get( + reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk}), status=403 + ) + # Assert the response is a 403 Forbidden + self.assertEqual(response.status_code, 403) + @less_console_noise_decorator def test_portfolio_organization_page_read_only(self): """Test that user with a portfolio can access the portfolio organization page, read only""" self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.portfolio.city = "Los Angeles" + self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.portfolio.save() self.user.save() self.user.refresh_from_db() @@ -142,6 +177,7 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ + User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.portfolio.city = "Los Angeles" @@ -210,6 +246,7 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ + User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() @@ -227,6 +264,7 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ + User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() @@ -245,6 +283,7 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ + User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() From 867ccd097e32c29493da3e75241bbe4053b262b9 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Jul 2024 20:21:41 -0400 Subject: [PATCH 11/34] remove migration --- ...r_user_portfolio_additional_permissions.py | 39 ------------------- src/registrar/models/user.py | 2 +- 2 files changed, 1 insertion(+), 40 deletions(-) delete mode 100644 src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py 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 27f6e699f..000000000 --- a/src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py +++ /dev/null @@ -1,39 +0,0 @@ -# Generated by Django 4.2.10 on 2024-07-30 23:58 - -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"), - ("edit_domains", "User is a manager on a domain"), - ("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"), - ("edit_portfolio", "Edit organization"), - ("view_portfolio", "View 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 48e6dc420..f4d7635cd 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -89,8 +89,8 @@ class User(AbstractUser): VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests" EDIT_REQUESTS = "edit_requests", "Create and edit requests" - EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" VIEW_PORTFOLIO = "view_portfolio", "View organization" + EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" PORTFOLIO_ROLE_PERMISSIONS = { UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ From 6a94da8a5d96cc0788cb375525ec763bfc3e3373 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Jul 2024 20:22:53 -0400 Subject: [PATCH 12/34] cleanup --- src/registrar/models/user.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index f4d7635cd..19097a96e 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -99,12 +99,14 @@ class User(AbstractUser): UserPortfolioPermissionChoices.EDIT_MEMBER, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ], UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, UserPortfolioPermissionChoices.VIEW_MEMBER, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, ], UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, From 3fef7664e43c4341088038829f8718c9eb8e1b5f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 31 Jul 2024 13:59:00 -0400 Subject: [PATCH 13/34] cleanup --- src/registrar/templates/includes/finish_profile_form.html | 4 ---- src/registrar/tests/test_views_portfolio.py | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index 58bc604ae..1806c2603 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -68,10 +68,6 @@ {% endwith %} {% endwith %} - {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable padding-top-2" %} - {% input_with_errors form.title %} - {% endwith %} - {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable padding-top-2" %} {% with add_class="usa-input--medium" %} {% input_with_errors form.phone %} diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 21b794cf7..0554d9908 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -201,6 +201,7 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ + User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, ] From 6523fca640b88ecac5de5befbd4403832663c80f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 31 Jul 2024 18:16:51 -0400 Subject: [PATCH 14/34] test clean up --- src/registrar/tests/test_views_portfolio.py | 43 ++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 0554d9908..a870f771a 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -224,7 +224,48 @@ class TestPortfolio(WebTest): # removing non-basic portfolio perms, which should remove domains # and domain requests from nav - self.user.portfolio_additional_permissions = [] + self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + self.user.save() + self.user.refresh_from_db() + + portfolio_page = self.app.get(reverse("home")).follow() + + self.assertContains(portfolio_page, self.portfolio.organization_name) + self.assertContains(portfolio_page, "

Organization

") + self.assertNotContains(portfolio_page, '

Domains

') + self.assertNotContains( + portfolio_page, reverse("portfolio-domains", kwargs={"portfolio_id": self.portfolio.pk}) + ) + self.assertNotContains( + portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk}) + ) + + @less_console_noise_decorator + def test_navigation_links_hidden_when_user_not_have_role(self): + """Test that admin / memmber roles are associated with the right access""" + self.app.set_user(self.user.username) + self.user.portfolio = self.portfolio + self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + self.user.save() + self.user.refresh_from_db() + with override_flag("organization_feature", active=True): + # This will redirect the user to the portfolio page. + # Follow implicity checks if our redirect is working. + portfolio_page = self.app.get(reverse("home")).follow() + # Assert that we're on the right page + self.assertContains(portfolio_page, self.portfolio.organization_name) + self.assertNotContains(portfolio_page, "

Organization

") + self.assertContains(portfolio_page, '

Domains

') + self.assertContains( + portfolio_page, reverse("portfolio-domains", kwargs={"portfolio_id": self.portfolio.pk}) + ) + self.assertContains( + portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk}) + ) + + # removing non-basic portfolio role, which should remove domains + # and domain requests from nav + self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_MEMBER] self.user.save() self.user.refresh_from_db() From b866886cff7c23faa6125b02390bd416e933e39a Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Wed, 31 Jul 2024 16:23:32 -0700 Subject: [PATCH 15/34] Revert newly created sandboxes to populate after development --- ops/scripts/create_dev_sandbox.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ops/scripts/create_dev_sandbox.sh b/ops/scripts/create_dev_sandbox.sh index b3a970584..fab062b30 100755 --- a/ops/scripts/create_dev_sandbox.sh +++ b/ops/scripts/create_dev_sandbox.sh @@ -112,7 +112,7 @@ sed -i '' '/ - development/ {a\ - '"$1"' }' .github/workflows/reset-db.yaml -sed -i '' '/ - backup/ {a\ +sed -i '' '/ - development/ {a\ - '"$1"' }' .github/workflows/migrate.yaml From fd5b763b421a79ac00f9f44251f4a3eba4879733 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Wed, 31 Jul 2024 16:34:34 -0700 Subject: [PATCH 16/34] Add new sandboxes under backup in deploy-manual --- ops/scripts/create_dev_sandbox.sh | 2 +- ops/scripts/destroy_dev_sandbox.sh | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ops/scripts/create_dev_sandbox.sh b/ops/scripts/create_dev_sandbox.sh index fab062b30..6cbad9c4f 100755 --- a/ops/scripts/create_dev_sandbox.sh +++ b/ops/scripts/create_dev_sandbox.sh @@ -116,7 +116,7 @@ sed -i '' '/ - development/ {a\ - '"$1"' }' .github/workflows/migrate.yaml -sed -i '' '/ - development/ {a\ +sed -i '' '/ - backup/ {a\ - '"$1"' }' .github/workflows/deploy-manual.yaml diff --git a/ops/scripts/destroy_dev_sandbox.sh b/ops/scripts/destroy_dev_sandbox.sh index 9e233b2f1..c8a00937f 100755 --- a/ops/scripts/destroy_dev_sandbox.sh +++ b/ops/scripts/destroy_dev_sandbox.sh @@ -49,6 +49,7 @@ rm ops/manifests/manifest-$1.yaml sed -i '' "/getgov-$1.app.cloud.gov/d" src/registrar/config/settings.py sed -i '' "/- $1/d" .github/workflows/reset-db.yaml sed -i '' "/- $1/d" .github/workflows/migrate.yaml +sed -i '' "/- $1/d" .github/workflows/deploy-manual.yaml echo "Cleaning up services, applications, and the Cloud.gov space for $1..." cf delete getgov-$1 From 1a30772e384cb4055d3cc18a591a9a515a465352 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Wed, 31 Jul 2024 16:41:09 -0700 Subject: [PATCH 17/34] Add docs to manually deploy --- docs/operations/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/operations/README.md b/docs/operations/README.md index 9aaee4c86..cc73d82cb 100644 --- a/docs/operations/README.md +++ b/docs/operations/README.md @@ -45,6 +45,8 @@ When deploying to your personal sandbox, you should make sure all of the USWDS a For ease of use, you can run the `deploy.sh ` script in the `/src` directory to build the assets and deploy to your sandbox. Similarly, you could run `build.sh ` script to just compile and collect the assets without deploying. +You may also manually deploy to a sandbox using our [manual deploy workflow](https://github.com/cisagov/manage.get.gov/actions/workflows/deploy-manual.yaml) on GitHub Actions. Select Run workflow and enter the branch you want to deploy to your sandbox of choice. + Your sandbox space should've been setup as part of the onboarding process. If this was not the case, please have an admin follow the instructions below. ## Creating a sandbox or new environment From 8f84f33756012e117eab9d3fe7b567d5df9c8adb Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 31 Jul 2024 20:34:34 -0400 Subject: [PATCH 18/34] portfolio invitation model --- src/registrar/admin.py | 74 ++++++++++++++- .../migrations/0114_portfolioinvitation.py | 84 ++++++++++++++++ src/registrar/models/__init__.py | 5 +- src/registrar/models/portfolio_invitation.py | 95 +++++++++++++++++++ src/registrar/models/user.py | 65 ++++++------- .../models/utility/portfolio_helper.py | 33 +++++++ .../templates/admin/model_descriptions.html | 2 + .../portfolio_invitation_description.html | 11 +++ src/registrar/tests/test_admin.py | 73 ++++++++++++++ src/registrar/tests/test_models.py | 65 ++++++++++++- 10 files changed, 465 insertions(+), 42 deletions(-) create mode 100644 src/registrar/migrations/0114_portfolioinvitation.py create mode 100644 src/registrar/models/portfolio_invitation.py create mode 100644 src/registrar/models/utility/portfolio_helper.py create mode 100644 src/registrar/templates/django/admin/includes/descriptions/portfolio_invitation_description.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 46f6cc68c..5dd0b1852 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -11,6 +11,7 @@ from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField from registrar.models.domain_group import DomainGroup from registrar.models.suborganization import Suborganization +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from waffle.decorators import flag_is_active from django.contrib import admin, messages from django.contrib.auth.admin import UserAdmin as BaseUserAdmin @@ -131,12 +132,12 @@ class MyUserAdminForm(UserChangeForm): "groups": NoAutocompleteFilteredSelectMultiple("groups", False), "user_permissions": NoAutocompleteFilteredSelectMultiple("user_permissions", False), "portfolio_roles": FilteredSelectMultipleArrayWidget( - "portfolio_roles", is_stacked=False, choices=User.UserPortfolioRoleChoices.choices + "portfolio_roles", is_stacked=False, choices=UserPortfolioRoleChoices.choices ), "portfolio_additional_permissions": FilteredSelectMultipleArrayWidget( "portfolio_additional_permissions", is_stacked=False, - choices=User.UserPortfolioPermissionChoices.choices, + choices=UserPortfolioPermissionChoices.choices, ), } @@ -169,6 +170,24 @@ class MyUserAdminForm(UserChangeForm): ) +class PortfolioInvitationAdminForm(UserChangeForm): + """This form utilizes the custom widget for its class's ManyToMany UIs.""" + + class Meta: + model = models.PortfolioInvitation + fields = "__all__" + widgets = { + "portfolio_roles": FilteredSelectMultipleArrayWidget( + "portfolio_roles", is_stacked=False, choices=UserPortfolioRoleChoices.choices + ), + "portfolio_additional_permissions": FilteredSelectMultipleArrayWidget( + "portfolio_additional_permissions", + is_stacked=False, + choices=UserPortfolioPermissionChoices.choices, + ), + } + + class DomainInformationAdminForm(forms.ModelForm): """This form utilizes the custom widget for its class's ManyToMany UIs.""" @@ -1299,6 +1318,56 @@ class DomainInvitationAdmin(ListHeaderAdmin): return super().changelist_view(request, extra_context=extra_context) +class PortfolioInvitationAdmin(ListHeaderAdmin): + """Custom portfolio invitation admin class.""" + + form = PortfolioInvitationAdminForm + + class Meta: + model = models.PortfolioInvitation + fields = "__all__" + + _meta = Meta() + + # Columns + list_display = [ + "email", + "portfolio", + "portfolio_roles", + "portfolio_additional_permissions", + "status", + ] + + # Search + search_fields = [ + "email", + "portfolio__name", + ] + + # Filters + list_filter = ("status",) + + search_help_text = "Search by email or portfolio." + + # Mark the FSM field 'status' as readonly + # to allow admin users to create Domain Invitations + # without triggering the FSM Transition Not Allowed + # error. + readonly_fields = ["status"] + + autocomplete_fields = ["portfolio"] + + change_form_template = "django/admin/email_clipboard_change_form.html" + + # Select portfolio invitations to change -> Portfolio invitations + def changelist_view(self, request, extra_context=None): + if extra_context is None: + extra_context = {} + extra_context["tabtitle"] = "Portfolio invitations" + # Get the filtered values + return super().changelist_view(request, extra_context=extra_context) + + class DomainInformationResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" @@ -2900,6 +2969,7 @@ admin.site.register(models.PublicContact, PublicContactAdmin) admin.site.register(models.DomainRequest, DomainRequestAdmin) admin.site.register(models.TransitionDomain, TransitionDomainAdmin) admin.site.register(models.VerifiedByStaff, VerifiedByStaffAdmin) +admin.site.register(models.PortfolioInvitation, PortfolioInvitationAdmin) admin.site.register(models.Portfolio, PortfolioAdmin) admin.site.register(models.DomainGroup, DomainGroupAdmin) admin.site.register(models.Suborganization, SuborganizationAdmin) diff --git a/src/registrar/migrations/0114_portfolioinvitation.py b/src/registrar/migrations/0114_portfolioinvitation.py new file mode 100644 index 000000000..afd1dd714 --- /dev/null +++ b/src/registrar/migrations/0114_portfolioinvitation.py @@ -0,0 +1,84 @@ +# Generated by Django 4.2.10 on 2024-07-31 22:49 + +import django.contrib.postgres.fields +from django.db import migrations, models +import django.db.models.deletion +import django_fsm + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0113_user_portfolio_user_portfolio_additional_permissions_and_more"), + ] + + operations = [ + migrations.CreateModel( + name="PortfolioInvitation", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ("email", models.EmailField(max_length=254)), + ( + "portfolio_roles", + django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("organization_admin", "Admin"), + ("organization_admin_read_only", "Admin read only"), + ("organization_member", "Member"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more roles.", + null=True, + size=None, + ), + ), + ( + "portfolio_additional_permissions", + 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"), + ("edit_domains", "User is a manager on a domain"), + ("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, + ), + ), + ( + "status", + django_fsm.FSMField( + choices=[("invited", "Invited"), ("retrieved", "Retrieved")], + default="invited", + max_length=50, + protected=True, + ), + ), + ( + "portfolio", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, related_name="portfolios", to="registrar.portfolio" + ), + ), + ], + options={ + "indexes": [models.Index(fields=["status"], name="registrar_p_status_aa4218_idx")], + }, + ), + ] diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index a68633aff..1e0aad0b1 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -1,4 +1,4 @@ -from auditlog.registry import auditlog # type: ignore +from auditlog.registry import auditlog from .contact import Contact from .domain_request import DomainRequest from .domain_information import DomainInformation @@ -16,6 +16,7 @@ from .website import Website from .transition_domain import TransitionDomain from .verified_by_staff import VerifiedByStaff from .waffle_flag import WaffleFlag +from .portfolio_invitation import PortfolioInvitation from .portfolio import Portfolio from .domain_group import DomainGroup from .suborganization import Suborganization @@ -40,6 +41,7 @@ __all__ = [ "TransitionDomain", "VerifiedByStaff", "WaffleFlag", + "PortfolioInvitation", "Portfolio", "DomainGroup", "Suborganization", @@ -63,6 +65,7 @@ auditlog.register(Website) auditlog.register(TransitionDomain) auditlog.register(VerifiedByStaff) auditlog.register(WaffleFlag) +auditlog.register(PortfolioInvitation) auditlog.register(Portfolio) auditlog.register(DomainGroup) auditlog.register(Suborganization) diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py new file mode 100644 index 000000000..2ad780429 --- /dev/null +++ b/src/registrar/models/portfolio_invitation.py @@ -0,0 +1,95 @@ +"""People are invited by email to administer domains.""" + +import logging + +from django.contrib.auth import get_user_model +from django.db import models + +from django_fsm import FSMField, transition +from .utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices # type: ignore + +from .utility.time_stamped_model import TimeStampedModel +from django.contrib.postgres.fields import ArrayField + + +logger = logging.getLogger(__name__) + + +class PortfolioInvitation(TimeStampedModel): + class Meta: + """Contains meta information about this class""" + + indexes = [ + models.Index(fields=["status"]), + ] + + # Constants for status field + class PortfolioInvitationStatus(models.TextChoices): + INVITED = "invited", "Invited" + RETRIEVED = "retrieved", "Retrieved" + + email = models.EmailField( + null=False, + blank=False, + ) + + portfolio = models.ForeignKey( + "registrar.Portfolio", + on_delete=models.CASCADE, # delete portfolio, then get rid of invitations + null=False, + related_name="portfolios", + ) + + portfolio_roles = ArrayField( + models.CharField( + max_length=50, + choices=UserPortfolioRoleChoices.choices, + ), + null=True, + blank=True, + help_text="Select one or more roles.", + ) + + portfolio_additional_permissions = ArrayField( + models.CharField( + max_length=50, + choices=UserPortfolioPermissionChoices.choices, + ), + null=True, + blank=True, + help_text="Select one or more additional permissions.", + ) + + status = FSMField( + choices=PortfolioInvitationStatus.choices, + default=PortfolioInvitationStatus.INVITED, + protected=True, # can't alter state except through transition methods! + ) + + def __str__(self): + return f"Invitation for {self.email} on {self.portfolio} is {self.status}" + + @transition(field="status", source=PortfolioInvitationStatus.INVITED, target=PortfolioInvitationStatus.RETRIEVED) + def retrieve(self): + """When an invitation is retrieved, create the corresponding permission. + + Raises: + RuntimeError if no matching user can be found. + """ + + # get a user with this email address + User = get_user_model() + try: + user = User.objects.get(email=self.email) + except User.DoesNotExist: + # should not happen because a matching user should exist before + # we retrieve this invitation + raise RuntimeError("Cannot find the user to retrieve this portfolio invitation.") + + # and create a role for that user on this portfolio + user.portfolio = self.portfolio + if self.portfolio_roles and len(self.portfolio_roles) > 0: + user.portfolio_roles = self.portfolio_roles + if self.portfolio_additional_permissions and len(self.portfolio_additional_permissions) > 0: + user.portfolio_additional_permissions = self.portfolio_additional_permissions + user.save() diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index b135e30c7..9c39c4d85 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -5,8 +5,10 @@ from django.db import models from django.db.models import Q from registrar.models.user_domain_role import UserDomainRole +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .domain_invitation import DomainInvitation +from .portfolio_invitation import PortfolioInvitation from .transition_domain import TransitionDomain from .verified_by_staff import VerifiedByStaff from .domain import Domain @@ -62,36 +64,6 @@ class User(AbstractUser): # after they login. FIXTURE_USER = "fixture_user", "Created by fixtures" - class UserPortfolioRoleChoices(models.TextChoices): - """ - Roles make it easier for admins to look at - """ - - ORGANIZATION_ADMIN = "organization_admin", "Admin" - ORGANIZATION_ADMIN_READ_ONLY = "organization_admin_read_only", "Admin read only" - ORGANIZATION_MEMBER = "organization_member", "Member" - - class UserPortfolioPermissionChoices(models.TextChoices): - """ """ - - 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" - - VIEW_ALL_REQUESTS = "view_all_requests", "View all requests" - VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests" - EDIT_REQUESTS = "edit_requests", "Create and edit requests" - - VIEW_PORTFOLIO = "view_portfolio", "View organization" - EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" - PORTFOLIO_ROLE_PERMISSIONS = { UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, @@ -270,7 +242,7 @@ class User(AbstractUser): # 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(): + if portfolio_permission == UserPortfolioPermissionChoices.EDIT_DOMAINS and self.domains.exists(): return True if not self.portfolio: @@ -283,22 +255,22 @@ class User(AbstractUser): # the methods below are checks for individual portfolio permissions. they are defined here # to make them easier to call elsewhere throughout the application def has_base_portfolio_permission(self): - return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO) + return self._has_portfolio_permission(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) + self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + or self._has_portfolio_permission(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(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) + self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) + or self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) # or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_REQUESTS) ) @@ -409,6 +381,24 @@ class User(AbstractUser): new_domain_invitation = DomainInvitation(email=transition_domain_email.lower(), domain=new_domain) new_domain_invitation.save() + def check_portfolio_invitations_on_login(self): + """When a user first arrives on the site, we need to retrieve any portfolio + invitations that match their email address.""" + for invitation in PortfolioInvitation.objects.filter( + email__iexact=self.email, status=PortfolioInvitation.PortfolioInvitationStatus.INVITED + ): + if self.portfolio is None: + try: + invitation.retrieve() + invitation.save() + except RuntimeError: + # retrieving should not fail because of a missing user, but + # if it does fail, log the error so a new user can continue + # logging in + logger.warn("Failed to retrieve invitation %s", invitation, exc_info=True) + else: + logger.warn("User already has a portfolio, did not retrieve invitation %s", invitation, exc_info=True) + def on_each_login(self): """Callback each time the user is authenticated. @@ -420,6 +410,7 @@ class User(AbstractUser): """ self.check_domain_invitations_on_login() + self.check_portfolio_invitations_on_login() def is_org_user(self, request): has_organization_feature_flag = flag_is_active(request, "organization_feature") diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py new file mode 100644 index 000000000..cadf12135 --- /dev/null +++ b/src/registrar/models/utility/portfolio_helper.py @@ -0,0 +1,33 @@ +from django.db import models + + +class UserPortfolioRoleChoices(models.TextChoices): + """ + Roles make it easier for admins to look at + """ + + ORGANIZATION_ADMIN = "organization_admin", "Admin" + ORGANIZATION_ADMIN_READ_ONLY = "organization_admin_read_only", "Admin read only" + ORGANIZATION_MEMBER = "organization_member", "Member" + + +class UserPortfolioPermissionChoices(models.TextChoices): + """ """ + + 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" + + VIEW_ALL_REQUESTS = "view_all_requests", "View all requests" + VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests" + EDIT_REQUESTS = "edit_requests", "Create and edit requests" + + VIEW_PORTFOLIO = "view_portfolio", "View organization" + EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" diff --git a/src/registrar/templates/admin/model_descriptions.html b/src/registrar/templates/admin/model_descriptions.html index c075e03a5..4b61e21bd 100644 --- a/src/registrar/templates/admin/model_descriptions.html +++ b/src/registrar/templates/admin/model_descriptions.html @@ -30,6 +30,8 @@ {% include "django/admin/includes/descriptions/verified_by_staff_description.html" %} {% elif opts.model_name == 'website' %} {% include "django/admin/includes/descriptions/website_description.html" %} + {% elif opts.model_name == 'portfolioinvitation' %} + {% include "django/admin/includes/descriptions/portfolio_invitation_description.html" %} {% else %}

This table does not have a description yet.

{% endif %} diff --git a/src/registrar/templates/django/admin/includes/descriptions/portfolio_invitation_description.html b/src/registrar/templates/django/admin/includes/descriptions/portfolio_invitation_description.html new file mode 100644 index 000000000..51515bcb2 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/descriptions/portfolio_invitation_description.html @@ -0,0 +1,11 @@ +

+Portfolio invitations contain all individuals who have been invited to become members of an organization. +Invitations are sent via email, and the recipient must log in to the registrar to officially +accept and become a member. +

+ +

+An ā€œinvitedā€ status indicates that the recipient has not logged in to the registrar since the invitation was sent +or that the recipient has logged in but is already a member of an organization. +A ā€œreceivedā€ status indicates that the recipient has logged in. +

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index c145e1f98..aa139b74e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -13,6 +13,7 @@ from registrar.admin import ( ContactAdmin, DomainInformationAdmin, MyHostAdmin, + PortfolioInvitationAdmin, UserDomainRoleAdmin, VerifiedByStaffAdmin, FsmModelResource, @@ -38,6 +39,7 @@ from registrar.models import ( UserGroup, TransitionDomain, ) +from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial from registrar.models.user_domain_role import UserDomainRole from registrar.models.verified_by_staff import VerifiedByStaff @@ -177,6 +179,77 @@ class TestDomainInvitationAdmin(TestCase): self.assertContains(response, retrieved_html, count=1) +class TestPortfolioInvitationAdmin(TestCase): + """Tests for the PortfolioInvitationAdmin class as super user + + Notes: + all tests share superuser; do not change this model in tests + tests have available superuser, client, and admin + """ + + @classmethod + def setUpClass(cls): + cls.factory = RequestFactory() + cls.admin = ListHeaderAdmin(model=PortfolioInvitationAdmin, admin_site=AdminSite()) + cls.superuser = create_superuser() + + def setUp(self): + """Create a client object""" + self.client = Client(HTTP_HOST="localhost:8080") + + def tearDown(self): + """Delete all DomainInvitation objects""" + PortfolioInvitation.objects.all().delete() + Contact.objects.all().delete() + + @classmethod + def tearDownClass(self): + User.objects.all().delete() + + @less_console_noise_decorator + def test_has_model_description(self): + """Tests if this model has a model description on the table view""" + self.client.force_login(self.superuser) + response = self.client.get( + "/admin/registrar/portfolioinvitation/", + follow=True, + ) + + # Make sure that the page is loaded correctly + self.assertEqual(response.status_code, 200) + + # Test for a description snippet + self.assertContains( + response, + "Portfolio invitations contain all individuals who have been invited to become members of an organization.", + ) + self.assertContains(response, "Show more") + + def test_get_filters(self): + """Ensures that our filters are displaying correctly""" + with less_console_noise(): + self.client.force_login(self.superuser) + + response = self.client.get( + "/admin/registrar/portfolioinvitation/", + {}, + follow=True, + ) + + # Assert that the filters are added + self.assertContains(response, "invited", count=4) + self.assertContains(response, "Invited", count=2) + self.assertContains(response, "retrieved", count=2) + self.assertContains(response, "Retrieved", count=2) + + # Check for the HTML context specificially + invited_html = 'Invited' + retrieved_html = 'Retrieved' + + self.assertContains(response, invited_html, count=1) + self.assertContains(response, retrieved_html, count=1) + + class TestHostAdmin(TestCase): """Tests for the HostAdmin class as super user diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 741ec5361..994f45480 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -20,7 +20,9 @@ from registrar.models import ( import boto3_mocking from registrar.models.portfolio import Portfolio +from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.transition_domain import TransitionDomain +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.models.verified_by_staff import VerifiedByStaff # type: ignore from registrar.utility.constants import BranchChoices @@ -1071,8 +1073,8 @@ class TestDomainInformation(TestCase): return {k: v for k, v in dict_obj.items() if k not in bad_fields} -class TestInvitations(TestCase): - """Test the retrieval of invitations.""" +class TestDomainInvitations(TestCase): + """Test the retrieval of domain invitations.""" @less_console_noise_decorator def setUp(self): @@ -1116,6 +1118,65 @@ class TestInvitations(TestCase): self.assertTrue(UserDomainRole.objects.get(user=self.user, domain=self.domain)) +class TestPortfolioInvitations(TestCase): + """Test the retrieval of portfolio invitations.""" + + @less_console_noise_decorator + def setUp(self): + self.email = "mayor@igorville.gov" + self.email2 = "creator@igorville.gov" + self.user, _ = User.objects.get_or_create(email=self.email) + self.user2, _ = User.objects.get_or_create(email=self.email2, username="creator") + self.portfolio, _ = Portfolio.objects.get_or_create(creator=self.user2, organization_name="Hotel California") + self.portfolio_role_base = UserPortfolioRoleChoices.ORGANIZATION_MEMBER + self.portfolio_role_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN + self.portfolio_permission_1 = UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS + self.portfolio_permission_2 = UserPortfolioPermissionChoices.EDIT_REQUESTS + self.invitation, _ = PortfolioInvitation.objects.get_or_create( + email=self.email, + portfolio=self.portfolio, + portfolio_roles=[self.portfolio_role_base, self.portfolio_role_admin], + portfolio_additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], + ) + + def tearDown(self): + super().tearDown() + Portfolio.objects.all().delete() + PortfolioInvitation.objects.all().delete() + User.objects.all().delete() + + @less_console_noise_decorator + def test_retrieval(self): + self.assertFalse(self.user.portfolio) + self.invitation.retrieve() + self.user.refresh_from_db() + self.assertEqual(self.user.portfolio.organization_name, "Hotel California") + self.assertEqual(self.user.portfolio_roles, [self.portfolio_role_base, self.portfolio_role_admin]) + self.assertEqual( + self.user.portfolio_additional_permissions, [self.portfolio_permission_1, self.portfolio_permission_2] + ) + self.assertEqual(self.invitation.status, PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) + + @less_console_noise_decorator + def test_retrieve_missing_user_error(self): + # get rid of matching users + User.objects.filter(email=self.email).delete() + with self.assertRaises(RuntimeError): + self.invitation.retrieve() + + @less_console_noise_decorator + def test_retrieve_user_already_member_error(self): + self.assertFalse(self.user.portfolio) + portfolio2, _ = Portfolio.objects.get_or_create(creator=self.user2, organization_name="Tokyo Hotel") + self.user.portfolio = portfolio2 + self.assertEqual(self.user.portfolio.organization_name, "Tokyo Hotel") + self.user.save() + self.user.check_portfolio_invitations_on_login() + self.user.refresh_from_db() + self.assertEqual(self.user.portfolio.organization_name, "Tokyo Hotel") + self.assertEqual(self.invitation.status, PortfolioInvitation.PortfolioInvitationStatus.INVITED) + + class TestUser(TestCase): """Test actions that occur on user login, test class method that controls how users get validated.""" From f22316ccc2b326ed079fd0488897a603064221cc Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 1 Aug 2024 08:14:26 -0400 Subject: [PATCH 19/34] fix unit tests --- src/registrar/tests/test_models.py | 3 ++- src/registrar/tests/test_views_portfolio.py | 27 +++++++++++---------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 994f45480..c1059012b 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1196,6 +1196,7 @@ class TestUser(TestCase): DomainRequest.objects.all().delete() DraftDomain.objects.all().delete() TransitionDomain.objects.all().delete() + Portfolio.objects.all().delete() User.objects.all().delete() UserDomainRole.objects.all().delete() @@ -1359,7 +1360,7 @@ class TestUser(TestCase): """ portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") - self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS] + self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS] self.user.save() self.user.refresh_from_db() diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 3596bf567..f1db5b29e 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -10,6 +10,7 @@ from registrar.models import ( UserDomainRole, User, ) +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices from .common import create_test_user from waffle.testutils import override_flag @@ -55,7 +56,7 @@ class TestPortfolio(WebTest): def test_middleware_does_not_redirect_if_no_portfolio(self): """Test that user with no assigned portfolio is not redirected when attempting to access home""" self.app.set_user(self.user.username) - self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): @@ -70,7 +71,7 @@ class TestPortfolio(WebTest): """Test that user with VIEW_PORTFOLIO is redirected to portfolio organization page""" self.app.set_user(self.user.username) self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): @@ -87,8 +88,8 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ] self.user.save() self.user.refresh_from_db() @@ -155,9 +156,9 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, ] self.user.save() self.user.refresh_from_db() @@ -203,8 +204,8 @@ class TestPortfolioOrganization(TestPortfolio): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() self.user.refresh_from_db() @@ -220,8 +221,8 @@ class TestPortfolioOrganization(TestPortfolio): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() self.user.refresh_from_db() @@ -238,8 +239,8 @@ class TestPortfolioOrganization(TestPortfolio): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.user.save() self.user.refresh_from_db() From 108adce05e30b3b620408ac091446b542d4a0998 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 1 Aug 2024 08:28:36 -0400 Subject: [PATCH 20/34] fix migrations --- ...14_portfolioinvitation.py => 0115_portfolioinvitation.py} | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) rename src/registrar/migrations/{0114_portfolioinvitation.py => 0115_portfolioinvitation.py} (93%) diff --git a/src/registrar/migrations/0114_portfolioinvitation.py b/src/registrar/migrations/0115_portfolioinvitation.py similarity index 93% rename from src/registrar/migrations/0114_portfolioinvitation.py rename to src/registrar/migrations/0115_portfolioinvitation.py index afd1dd714..82a171f10 100644 --- a/src/registrar/migrations/0114_portfolioinvitation.py +++ b/src/registrar/migrations/0115_portfolioinvitation.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-07-31 22:49 +# Generated by Django 4.2.10 on 2024-08-01 12:28 import django.contrib.postgres.fields from django.db import migrations, models @@ -9,7 +9,7 @@ import django_fsm class Migration(migrations.Migration): dependencies = [ - ("registrar", "0113_user_portfolio_user_portfolio_additional_permissions_and_more"), + ("registrar", "0114_alter_user_portfolio_additional_permissions"), ] operations = [ @@ -44,7 +44,6 @@ class Migration(migrations.Migration): choices=[ ("view_all_domains", "View all domains and domain reports"), ("view_managed_domains", "View managed domains"), - ("edit_domains", "User is a manager on a domain"), ("view_member", "View members"), ("edit_member", "Create and edit members"), ("view_all_requests", "View all requests"), From ca24078f50f39fccbccb6a9cd9a9efe91e20c8c6 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 1 Aug 2024 08:37:21 -0400 Subject: [PATCH 21/34] fix unit tests --- src/registrar/tests/test_views_portfolio.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index f1db5b29e..29515d18f 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -179,7 +179,7 @@ class TestPortfolio(WebTest): # reducing portfolio permissions to just VIEW_PORTFOLIO, which should remove domains # and domain requests from nav - self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.user.save() self.user.refresh_from_db() @@ -195,9 +195,7 @@ class TestPortfolio(WebTest): portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk}) ) - -class TestPortfolioOrganization(TestPortfolio): - + @less_console_noise_decorator def test_portfolio_org_name(self): """Can load portfolio's org name page.""" with override_flag("organization_feature", active=True): @@ -215,6 +213,7 @@ class TestPortfolioOrganization(TestPortfolio): page, "The name of your federal agency will be publicly listed as the domain registrant." ) + @less_console_noise_decorator def test_domain_org_name_address_content(self): """Org name and address information appears on the page.""" with override_flag("organization_feature", active=True): @@ -233,6 +232,7 @@ class TestPortfolioOrganization(TestPortfolio): # Once in the sidenav, once in the main nav, once in the form self.assertContains(page, "Hotel California", count=3) + @less_console_noise_decorator def test_domain_org_name_address_form(self): """Submitting changes works on the org name address page.""" with override_flag("organization_feature", active=True): From 7b46e39a89ad50df6627c5680ac94ea7d14018f4 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 1 Aug 2024 08:42:57 -0400 Subject: [PATCH 22/34] fix unit tests --- src/registrar/tests/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 5fffa878f..b50525e27 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1379,7 +1379,7 @@ class TestUser(TestCase): self.assertTrue(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) - self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] self.user.save() self.user.refresh_from_db() From 70d30f9cee2f9495ae4a76643542f9ea7f0a1ee2 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 1 Aug 2024 08:52:02 -0400 Subject: [PATCH 23/34] fix unit tests --- src/registrar/models/user.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index d81c17123..33d8fa1ac 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -254,13 +254,13 @@ class User(AbstractUser): 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) + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS + ) or self._has_portfolio_permission(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) + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS + ) or self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) @classmethod def needs_identity_verification(cls, email, uuid): From 0c8b90850dadc5f7bbdd762f2c5cff496adf1fea Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Thu, 1 Aug 2024 12:11:39 -0500 Subject: [PATCH 24/34] Update docs/developer/README.md Co-authored-by: Erin Song <121973038+erinysong@users.noreply.github.com> --- docs/developer/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index e978ab666..358df649c 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -360,4 +360,5 @@ Then, copy the variables under the section labled `s3`. 4. (Important) Set the field `everyone` to `Yes`. This field overrides all other settings ## Request Flow FSM Diagram -There is a diagram detailing the flow of domain requests and resulting domain objects [here](https://miro.com/app/board/uXjVMuqbLOk=/?moveToWidget=3458764594819017396&cot=14) \ No newline at end of file + +The [.gov Domain Request & Domain Status Digram](https://miro.com/app/board/uXjVMuqbLOk=/?moveToWidget=3458764594819017396&cot=14) visualizes the domain request flow and resulting domain objects. From 165e27307bd0fb79cff17d1d9abed05cc2aaad3b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 1 Aug 2024 14:10:20 -0400 Subject: [PATCH 25/34] fix federal agency --- src/registrar/templates/portfolio_organization.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/portfolio_organization.html b/src/registrar/templates/portfolio_organization.html index cc9cf5b6a..b17d18b62 100644 --- a/src/registrar/templates/portfolio_organization.html +++ b/src/registrar/templates/portfolio_organization.html @@ -35,7 +35,7 @@ {% csrf_token %}

Federal agency

- {{ portfolio }} + {{ portfolio.federal_agency }}

{% input_with_errors form.address_line1 %} {% input_with_errors form.address_line2 %} @@ -54,7 +54,7 @@ {% else %}

Federal agency

- {{ portfolio }} + {{ portfolio.federal_agency }}

{% if form.address_line1.value is not None %} {% include "includes/input_read_only.html" with field=form.address_line1 %} From aedb05dd34852358504c7fe7ff17117f14c2342e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 1 Aug 2024 14:16:45 -0400 Subject: [PATCH 26/34] fix unit test --- src/registrar/tests/test_views_portfolio.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index a870f771a..3ec8efc62 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -315,8 +315,9 @@ class TestPortfolio(WebTest): self.portfolio.organization_name = "Hotel California" self.portfolio.save() page = self.app.get(reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk})) - # Once in the sidenav, once in the main nav, once in the form - self.assertContains(page, "Hotel California", count=3) + # Once in the sidenav, once in the main nav + self.assertContains(page, "Hotel California", count=2) + self.assertContains(page, "Non-Federal Agency") @less_console_noise_decorator def test_domain_org_name_address_form(self): From 3e70b344e3db77848f0486e64089c6430e915be9 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 1 Aug 2024 14:26:13 -0400 Subject: [PATCH 27/34] cleanup --- src/registrar/models/user.py | 2 +- src/registrar/templates/includes/finish_profile_form.html | 2 +- src/registrar/templates/portfolio_organization.html | 7 ++----- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 19097a96e..c5895cdf0 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -64,7 +64,7 @@ class User(AbstractUser): class UserPortfolioRoleChoices(models.TextChoices): """ - Roles make it easier for admins to look at groups of users + Roles are containers of UserPortfolioPermissionChoices """ ORGANIZATION_ADMIN = "organization_admin", "Admin" diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index 1806c2603..42a2de1a5 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -53,7 +53,7 @@ {% endwith %} - {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable padding-top-2" %} + {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable padding-top-2" %} {% input_with_errors form.title %} {% endwith %} diff --git a/src/registrar/templates/portfolio_organization.html b/src/registrar/templates/portfolio_organization.html index b17d18b62..51adba3d9 100644 --- a/src/registrar/templates/portfolio_organization.html +++ b/src/registrar/templates/portfolio_organization.html @@ -31,7 +31,7 @@ {% include "includes/form_errors.html" with form=form %} {% include "includes/required_fields.html" %} -
+ {% csrf_token %}

Federal agency

@@ -44,10 +44,7 @@ {% with add_class="usa-input--small" %} {% input_with_errors form.zipcode %} {% endwith %} -

From 25b9484a7ada89c67acea41ab6e357464a92b4a0 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Fri, 2 Aug 2024 11:41:03 -0400 Subject: [PATCH 28/34] Update domain_request_dotgov_domain.html Updating in line with https://github.com/cisagov/get.gov/pull/311 --- src/registrar/templates/domain_request_dotgov_domain.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/templates/domain_request_dotgov_domain.html b/src/registrar/templates/domain_request_dotgov_domain.html index 60220e014..5864cad29 100644 --- a/src/registrar/templates/domain_request_dotgov_domain.html +++ b/src/registrar/templates/domain_request_dotgov_domain.html @@ -6,8 +6,7 @@
  • Be available
  • Relate to your organizationā€™s name, location, and/or services
    • -
  • Be clear to the general public. Your domain name must not be easily confused - with other organizations.
    • +
  • Be unlikely to mislead or confuse the general public (even if your domain is only intended for a specific audience)

From 77c16734793c73c22d21d29904537035a500849b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 2 Aug 2024 13:10:46 -0400 Subject: [PATCH 29/34] update to context_processor --- src/registrar/context_processors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 861a4e701..ee5f8aee1 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -61,7 +61,7 @@ def add_has_profile_feature_flag_to_context(request): def portfolio_permissions(request): """Make portfolio permissions for the request user available in global context""" try: - if not request.user or not request.user.is_authenticated: + if not request.user or not request.user.is_authenticated or not flag_is_active(request, "organization_feature"): return { "has_base_portfolio_permission": False, "has_domains_portfolio_permission": False, @@ -74,7 +74,7 @@ def portfolio_permissions(request): "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"), + "has_organization_feature_flag": True, } except AttributeError: # Handles cases where request.user might not exist From d8fd49fa10264c5193b82da0a0c48162e647b32d Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 2 Aug 2024 13:19:09 -0400 Subject: [PATCH 30/34] cleanup --- src/registrar/tests/test_views_portfolio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 49a5679c1..f21321e96 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -300,7 +300,7 @@ class TestPortfolio(WebTest): self.portfolio.organization_name = "Hotel California" self.portfolio.save() - page = self.app.get(reverse("portfolio-organization")) + page = self.app.get(reverse("organization")) # Once in the sidenav, once in the main nav self.assertContains(page, "Hotel California", count=2) self.assertContains(page, "Non-Federal Agency") From 53d9f69420837f1d6cb06dbc26d5f332dd343e75 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 2 Aug 2024 13:46:00 -0400 Subject: [PATCH 31/34] clean up merge errors --- src/registrar/tests/test_views_portfolio.py | 12 ++++++------ src/registrar/views/portfolios.py | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index f21321e96..05d0b61ef 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -155,7 +155,7 @@ class TestPortfolio(WebTest): self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): - response = self.app.get(reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk})) + response = self.app.get(reverse("organization")) # Assert the response is a 200 self.assertEqual(response.status_code, 200) # The label for Federal agency will always be a h4 @@ -179,7 +179,7 @@ class TestPortfolio(WebTest): self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): - response = self.app.get(reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk})) + response = self.app.get(reverse("organization")) # Assert the response is a 200 self.assertEqual(response.status_code, 200) # The label for Federal agency will always be a h4 @@ -243,10 +243,10 @@ class TestPortfolio(WebTest): self.assertNotContains(portfolio_page, "

Organization

") self.assertContains(portfolio_page, '

Domains

') self.assertContains( - portfolio_page, reverse("portfolio-domains", kwargs={"portfolio_id": self.portfolio.pk}) + portfolio_page, reverse("domains") ) self.assertContains( - portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk}) + portfolio_page, reverse("domain-requests") ) # removing non-basic portfolio role, which should remove domains @@ -261,10 +261,10 @@ class TestPortfolio(WebTest): self.assertContains(portfolio_page, "

Organization

") self.assertNotContains(portfolio_page, '

Domains

') self.assertNotContains( - portfolio_page, reverse("portfolio-domains", kwargs={"portfolio_id": self.portfolio.pk}) + portfolio_page, reverse("domains") ) self.assertNotContains( - portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk}) + portfolio_page, reverse("domain-requests") ) @less_console_noise_decorator diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 4b7374d81..63ebbaa01 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -48,6 +48,7 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): def get_context_data(self, **kwargs): """Add additional context data to the template.""" context = super().get_context_data(**kwargs) + context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission() return context def get_object(self, queryset=None): From 94cda46baf174ab91aa982395b04d1836df81bc9 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 2 Aug 2024 13:53:17 -0400 Subject: [PATCH 32/34] solidify domains permissions --- .../templates/includes/domains_table.html | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 348ab21d7..64eddec41 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -1,15 +1,15 @@ {% load static %} -
-
- {% if portfolio is None %} +
+
+ {% if not has_domains_portfolio_permission %}

Domains

{% else %} {% endif %} -
+
-
+
- {% if portfolio %} + {% if has_domains_portfolio_permission %}
Filter by
@@ -150,7 +150,7 @@ Domain name Expires Status - {% if portfolio %} + {% if has_domains_portfolio_permission %} Suborganization {% endif %} Date: Fri, 2 Aug 2024 13:55:14 -0400 Subject: [PATCH 33/34] solidify requests table perms --- src/registrar/templates/includes/domain_requests_table.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index ad91699ef..30c206741 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -2,7 +2,7 @@
- {% if portfolio is None %} + {% if not has_domain_requests_portfolio_permission %}

Domain requests

From 85f51ef142178e5d35ce4b96f24c41a07521a5f3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 2 Aug 2024 14:11:21 -0400 Subject: [PATCH 34/34] Fix unit tests --- src/registrar/tests/test_views_domain.py | 3 ++- src/registrar/tests/test_views_portfolio.py | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index e24085b30..00fcc8fe0 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -7,6 +7,7 @@ from django.contrib.auth import get_user_model from api.tests.common import less_console_noise_decorator from registrar.models.portfolio import Portfolio +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from .common import MockEppLib, MockSESClient, create_user # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore @@ -326,7 +327,7 @@ class TestDomainDetail(TestDomainOverview): phone="8003111234", title="test title", portfolio=portfolio, - portfolio_roles=[User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], ) domain, _ = Domain.objects.get_or_create(name="bogusdomain.gov") DomainInformation.objects.get_or_create(creator=user, domain=domain, portfolio=portfolio) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 49b4db271..f72130d94 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -151,7 +151,7 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.portfolio.city = "Los Angeles" - self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.portfolio.save() self.user.save() self.user.refresh_from_db() @@ -172,8 +172,8 @@ class TestPortfolio(WebTest): self.app.set_user(self.user.username) self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [ - User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] self.portfolio.city = "Los Angeles" self.portfolio.save()