Skip to content

Commit

Permalink
Revert changes to hide the 'Manage Customers' button (#1251)
Browse files Browse the repository at this point in the history
Hiding the button broke formatting on the app list page and could
lead to confusion. Instead I have improved the messaging displayed
to users if they try to manage customers without either creating
an Auth0 client or enabling authentication
  • Loading branch information
michaeljcollinsuk authored Feb 16, 2024
1 parent a956062 commit 5dcc0be
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 32 deletions.
13 changes: 4 additions & 9 deletions controlpanel/api/models/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@
from django_extensions.db.models import TimeStampedModel

# First-party/Local
from controlpanel.api import auth0, cluster
from controlpanel.api import auth0, cluster, tasks
from controlpanel.api.models import IPAllowlist
from controlpanel.utils import github_repository_name, s3_slugify, webapp_release_name
from controlpanel.api import tasks


class App(TimeStampedModel):
Expand Down Expand Up @@ -83,12 +82,6 @@ def release_name(self):
def iam_role_arn(self):
return cluster.iam_arn(f"role/{self.iam_role_name}")

@property
def can_manage_customers(self):
if not self.app_conf:
return False
return bool(self.app_conf.get(self.KEY_WORD_FOR_AUTH_SETTINGS))

def get_group_id(self, env_name):
return self.get_auth_client(env_name).get("group_id")

Expand Down Expand Up @@ -320,7 +313,8 @@ class DeleteCustomerError(Exception):
App.DeleteCustomerError = DeleteCustomerError


from django.db.models.signals import post_save, post_delete
# Third-party
from django.db.models.signals import post_delete, post_save
from django.dispatch import receiver


Expand All @@ -342,6 +336,7 @@ def trigger_app_create_related_messages(sender, instance, created, **kwargs):

@receiver(post_delete, sender=App)
def remove_app_related_tasks(sender, instance, **kwargs):
# First-party/Local
from controlpanel.api.models import Task
related_app_tasks = Task.objects.filter(entity_class="App", entity_id=instance.id)
for task in related_app_tasks:
Expand Down
2 changes: 1 addition & 1 deletion controlpanel/frontend/jinja2/customers-list.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ <h2 class="govuk-heading-m">

{% if not groups_dict %}
<h2 class="govuk-heading-s">
No need to manage the customers of the app on Control panel as it does not require authentication
Customer management is disabled. To manage customers, create an Auth0 client and enable authentication on the <a href="{{ url("manage-app", kwargs={ "pk": app.id}) }}"> manage app page.</a>
</h2>
{% else %}
<section>
Expand Down
9 changes: 3 additions & 6 deletions controlpanel/frontend/jinja2/includes/app-list.html
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,9 @@
<td class="govuk-table__cell">
{{ yes_no(user.is_app_admin(app.id)) }}
</td>
{% if app.can_manage_customers %}
<td class="govuk-table__cell">
<a href="{{ url("appcustomers-page", kwargs={"pk": app.id, "page_no": "1"}) }}"
class="govuk-button govuk-button--secondary right" >Manage customers</a>
</td>
{% endif %}
<td class="govuk-table__cell">
<a href="{{ url("appcustomers-page", kwargs={"pk": app.id, "page_no": "1"}) }}" class="govuk-button govuk-button--secondary right">Manage customers</a>
</td>
<td class="govuk-table__cell">
<a class="govuk-button govuk-button--secondary right {%- if not request.user.has_perm('api.retrieve_app', app) %} govuk-visually-hidden{% endif %}"
href="{{ url("manage-app", kwargs={ "pk": app.id}) }}">
Expand Down
14 changes: 0 additions & 14 deletions tests/api/models/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,17 +209,3 @@ def test_app_allowed_ip_ranges():
def test_iam_role_arn():
app = App(slug="example-app")
assert app.iam_role_arn == f"arn:aws:iam::{settings.AWS_DATA_ACCOUNT_ID}:role/test_app_example-app"


@pytest.mark.parametrize("conf, expected", [
(None, False),
({}, False),
({"some_setting_key": []}, False),
({App.KEY_WORD_FOR_AUTH_SETTINGS: None}, False),
({App.KEY_WORD_FOR_AUTH_SETTINGS: {}}, False),
({App.KEY_WORD_FOR_AUTH_SETTINGS: {"dev": {}}}, True),
])
def test_can_manage_customers(conf, expected):
app = App()
app.app_conf = conf
assert app.can_manage_customers is expected
4 changes: 2 additions & 2 deletions tests/api/views/test_customer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
from unittest.mock import patch

# Third-party
import pytest
from auth0.rest import Auth0Error
from bs4 import BeautifulSoup
from model_mommy import mommy
import pytest
from rest_framework import status
from rest_framework.reverse import reverse

Expand Down Expand Up @@ -221,7 +221,7 @@ def test_no_exist_auth0_clients_on_customers_page(client, app, users, ExtendedAu
def test_no_auth0_customers_page(client, app, users, ExtendedAuth0):
app.app_conf = None
app.save()
message = "No need to manage the customers of the app on Control pane"
message = "Customer management is disabled."
client.force_login(users["superuser"])
response = client.get(reverse("appcustomers-page", args=(app.id, 1)))

Expand Down

0 comments on commit 5dcc0be

Please sign in to comment.