Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(on getgov-bob) Ticket #1948: Remove draft domain and websites for analysts #1981

Merged
merged 14 commits into from
Apr 12, 2024
Merged
85 changes: 84 additions & 1 deletion src/registrar/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,46 @@ class WebsiteAdmin(ListHeaderAdmin):
]
search_help_text = "Search by website."

def get_model_perms(self, request):
"""
Return empty perms dict thus hiding the model from admin index.
"""
superuser_perm = request.user.has_perm("registrar.full_access_permission")
analyst_perm = request.user.has_perm("registrar.analyst_access_permission")
if analyst_perm and not superuser_perm:
return {}
return super().get_model_perms(request)

def has_change_permission(self, request, obj=None):
"""
Allow analysts to access the change form directly via URL.
"""
superuser_perm = request.user.has_perm("registrar.full_access_permission")
analyst_perm = request.user.has_perm("registrar.analyst_access_permission")
if analyst_perm and not superuser_perm:
return True
return super().has_change_permission(request, obj)

def response_change(self, request, obj):
"""
Override to redirect users back to the previous page after saving.
"""
superuser_perm = request.user.has_perm("registrar.full_access_permission")
analyst_perm = request.user.has_perm("registrar.analyst_access_permission")
return_path = request.GET.get("return_path")

# First, call the super method to perform the standard operations and capture the response
response = super().response_change(request, obj)

# Don't redirect to the website page on save if the user is an analyst.
# Rather, just redirect back to the originating page.
if (analyst_perm and not superuser_perm) and return_path:
# Redirect to the return path if it exists
return HttpResponseRedirect(return_path)

# If no redirection is needed, return the original response
return response


class UserDomainRoleAdmin(ListHeaderAdmin):
"""Custom user domain role admin class."""
Expand Down Expand Up @@ -1466,7 +1506,10 @@ class DomainInformationInline(admin.StackedInline):
def has_change_permission(self, request, obj=None):
"""Custom has_change_permission override so that we can specify that
analysts can edit this through this inline, but not through the model normally"""
if request.user.has_perm("registrar.analyst_access_permission"):

superuser_perm = request.user.has_perm("registrar.full_access_permission")
analyst_perm = request.user.has_perm("registrar.analyst_access_permission")
if analyst_perm and not superuser_perm:
return True
return super().has_change_permission(request, obj)

Expand Down Expand Up @@ -1896,6 +1939,46 @@ class DraftDomainAdmin(ListHeaderAdmin):
# in autocomplete_fields for user
ordering = ["name"]

def get_model_perms(self, request):
"""
Return empty perms dict thus hiding the model from admin index.
"""
superuser_perm = request.user.has_perm("registrar.full_access_permission")
analyst_perm = request.user.has_perm("registrar.analyst_access_permission")
if analyst_perm and not superuser_perm:
return {}
return super().get_model_perms(request)

def has_change_permission(self, request, obj=None):
"""
Allow analysts to access the change form directly via URL.
"""
superuser_perm = request.user.has_perm("registrar.full_access_permission")
analyst_perm = request.user.has_perm("registrar.analyst_access_permission")
if analyst_perm and not superuser_perm:
return True
return super().has_change_permission(request, obj)

def response_change(self, request, obj):
"""
Override to redirect users back to the previous page after saving.
"""
superuser_perm = request.user.has_perm("registrar.full_access_permission")
analyst_perm = request.user.has_perm("registrar.analyst_access_permission")
return_path = request.GET.get("return_path")

# First, call the super method to perform the standard operations and capture the response
response = super().response_change(request, obj)

# Don't redirect to the website page on save if the user is an analyst.
# Rather, just redirect back to the originating page.
if (analyst_perm and not superuser_perm) and return_path:
# Redirect to the return path if it exists
return HttpResponseRedirect(return_path)

# If no redirection is needed, return the original response
return response


class PublicContactAdmin(ListHeaderAdmin):
"""Custom PublicContact admin class."""
Expand Down
37 changes: 37 additions & 0 deletions src/registrar/migrations/0084_create_groups_v11.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# This migration creates the create_full_access_group and create_cisa_analyst_group groups
# It is dependent on 0079 (which populates federal agencies)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non blocking comment: is this right? It seems like it's really dependent on 0083

Copy link
Contributor Author

@zandercymatics zandercymatics Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find!! That is a typo

# If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS
# in the user_group model then:
# [NOT RECOMMENDED]
# step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions
# step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups
# step 3: fake run the latest migration in the migrations list
# [RECOMMENDED]
# Alternatively:
# step 1: duplicate the migration that loads data
# step 2: docker-compose exec app ./manage.py migrate

from django.db import migrations
from registrar.models import UserGroup
from typing import Any


# For linting: RunPython expects a function reference,
# so let's give it one
def create_groups(apps, schema_editor) -> Any:
UserGroup.create_cisa_analyst_group(apps, schema_editor)
UserGroup.create_full_access_group(apps, schema_editor)


class Migration(migrations.Migration):
dependencies = [
("registrar", "0083_alter_contact_email_alter_publiccontact_email"),
]

operations = [
migrations.RunPython(
create_groups,
reverse_code=migrations.RunPython.noop,
atomic=True,
),
]
10 changes: 0 additions & 10 deletions src/registrar/models/user_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ def create_cisa_analyst_group(apps, schema_editor):
"model": "domain",
"permissions": ["view_domain"],
},
{
"app_label": "registrar",
"model": "draftdomain",
"permissions": ["change_draftdomain"],
},
{
"app_label": "registrar",
"model": "user",
Expand All @@ -56,11 +51,6 @@ def create_cisa_analyst_group(apps, schema_editor):
"model": "domaininvitation",
"permissions": ["add_domaininvitation", "view_domaininvitation"],
},
{
"app_label": "registrar",
"model": "website",
"permissions": ["change_website"],
},
{
"app_label": "registrar",
"model": "userdomainrole",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
</dl>
</div>
{% endif %}
{% elif field.field.name == "requested_domain" %}
{% with current_path=request.get_full_path %}
<a class="margin-top-05 padding-top-05" href="{% url 'admin:registrar_draftdomain_change' original.requested_domain.id %}?{{ 'return_path='|add:current_path }}">{{ original.requested_domain }}</a>
{% endwith%}
{% elif field.field.name == "current_websites" %}
{% comment %}
The "website" model is essentially just a text field.
Expand All @@ -49,9 +53,11 @@
</div>
{% elif field.field.name == "alternative_domains" %}
<div class="readonly">
{% with current_path=request.get_full_path %}
{% for alt_domain in original.alternative_domains.all %}
<a href="{% url 'admin:registrar_website_change' alt_domain.id %}">{{ alt_domain }}</a>{% if not forloop.last %}, {% endif %}
<a href="{% url 'admin:registrar_website_change' alt_domain.id %}?{{ 'return_path='|add:current_path }}">{{ alt_domain }}</a>{% if not forloop.last %}, {% endif %}
{% endfor %}
{% endwith %}
</div>
{% else %}
<div class="readonly">{{ field.contents }}</div>
Expand Down
131 changes: 130 additions & 1 deletion src/registrar/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,16 @@
UserDomainRoleAdmin,
VerifiedByStaffAdmin,
)
from registrar.models import Domain, DomainRequest, DomainInformation, User, DomainInvitation, Contact, Website
from registrar.models import (
Domain,
DomainRequest,
DomainInformation,
User,
DomainInvitation,
Contact,
Website,
DraftDomain,
)
from registrar.models.user_domain_role import UserDomainRole
from registrar.models.verified_by_staff import VerifiedByStaff
from .common import (
Expand Down Expand Up @@ -703,6 +712,126 @@ def setUp(self):
)
self.mock_client = MockSESClient()

@less_console_noise_decorator
def test_analyst_can_see_and_edit_alternative_domain(self):
"""Tests if an analyst can still see and edit the alternative domain field"""

# Create fake creator
_creator = User.objects.create(
username="MrMeoward",
first_name="Meoward",
last_name="Jones",
)

# Create a fake domain request
_domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=_creator)

fake_website = Website.objects.create(website="thisisatest.gov")
_domain_request.alternative_domains.add(fake_website)
_domain_request.save()

p = "userpass"
self.client.login(username="staffuser", password=p)
response = self.client.get(
"/admin/registrar/domainrequest/{}/change/".format(_domain_request.pk),
follow=True,
)

# Make sure the page loaded, and that we're on the right page
self.assertEqual(response.status_code, 200)
self.assertContains(response, _domain_request.requested_domain.name)

# Test if the page has the alternative domain
self.assertContains(response, "thisisatest.gov")

# Check that the page contains the url we expect
expected_href = reverse("admin:registrar_website_change", args=[fake_website.id])
self.assertContains(response, expected_href)

# Navigate to the website to ensure that we can still edit it
response = self.client.get(
"/admin/registrar/website/{}/change/".format(fake_website.pk),
follow=True,
)

# Make sure the page loaded, and that we're on the right page
self.assertEqual(response.status_code, 200)
self.assertContains(response, "thisisatest.gov")

@less_console_noise_decorator
def test_analyst_can_see_and_edit_requested_domain(self):
"""Tests if an analyst can still see and edit the requested domain field"""

# Create fake creator
_creator = User.objects.create(
username="MrMeoward",
first_name="Meoward",
last_name="Jones",
)

# Create a fake domain request
_domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=_creator)

p = "userpass"
self.client.login(username="staffuser", password=p)
response = self.client.get(
"/admin/registrar/domainrequest/{}/change/".format(_domain_request.pk),
follow=True,
)

# Filter to get the latest from the DB (rather than direct assignment)
requested_domain = DraftDomain.objects.filter(name=_domain_request.requested_domain.name).get()

# Make sure the page loaded, and that we're on the right page
self.assertEqual(response.status_code, 200)
self.assertContains(response, requested_domain.name)

# Check that the page contains the url we expect
expected_href = reverse("admin:registrar_draftdomain_change", args=[requested_domain.id])
self.assertContains(response, expected_href)

# Navigate to the website to ensure that we can still edit it
response = self.client.get(
"/admin/registrar/draftdomain/{}/change/".format(requested_domain.pk),
follow=True,
)

# Make sure the page loaded, and that we're on the right page
self.assertEqual(response.status_code, 200)
self.assertContains(response, "city.gov")

@less_console_noise_decorator
def test_analyst_can_see_current_websites(self):
"""Tests if an analyst can still see current website field"""

# Create fake creator
_creator = User.objects.create(
username="MrMeoward",
first_name="Meoward",
last_name="Jones",
)

# Create a fake domain request
_domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=_creator)

fake_website = Website.objects.create(website="thisisatest.gov")
_domain_request.current_websites.add(fake_website)
_domain_request.save()

p = "userpass"
self.client.login(username="staffuser", password=p)
response = self.client.get(
"/admin/registrar/domainrequest/{}/change/".format(_domain_request.pk),
follow=True,
)

# Make sure the page loaded, and that we're on the right page
self.assertEqual(response.status_code, 200)
self.assertContains(response, _domain_request.requested_domain.name)

# Test if the page has the current website
self.assertContains(response, "thisisatest.gov")

def test_domain_sortable(self):
"""Tests if the DomainRequest sorts by domain correctly"""
with less_console_noise():
Expand Down
2 changes: 0 additions & 2 deletions src/registrar/tests/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def test_groups_created(self):
"add_domaininvitation",
"view_domaininvitation",
"change_domainrequest",
"change_draftdomain",
"add_federalagency",
"change_federalagency",
"delete_federalagency",
Expand All @@ -48,7 +47,6 @@ def test_groups_created(self):
"add_verifiedbystaff",
"change_verifiedbystaff",
"delete_verifiedbystaff",
"change_website",
]

# Get the codenames of actual permissions associated with the group
Expand Down
Loading