Skip to content

Commit

Permalink
Revert changes to hide the 'Manage Customers' button
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 committed Feb 16, 2024
1 parent a956062 commit 34116ee
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 30 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

0 comments on commit 34116ee

Please sign in to comment.