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.

Sign up for GitHub

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

Jump to bottom

Admin: Update admin page titles #2410

Merged
merged 9 commits into from
Sep 30, 2024
Merged

Admin: Update admin page titles #2410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9b5d3aa
feat(admin): set site title, index title for Dashboard
machikoyasuda Sep 26, 2024
0ff6b05
refactor(admin): use default title and variables
machikoyasuda Sep 26, 2024
7269d26
refactor(admin): use site-header for Administrator
machikoyasuda Sep 26, 2024
ebcec56
feat(admin): set title for in-person flow/error pages
machikoyasuda Sep 26, 2024
3b70c8a
feat(admin): set title for dashboard
machikoyasuda Sep 26, 2024
05f79d9
feat(admin): use site_title for logged out
machikoyasuda Sep 26, 2024
dcb37ca
feat(admin): add titles to renenrollment/server/system error, success…
machikoyasuda Sep 30, 2024
e4c68f3
test(admin): update fixtures
machikoyasuda Sep 30, 2024
becff48
fix(admin): add title to in-person enrollment success
machikoyasuda Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions benefits/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

class BenefitsAdminSite(admin.AdminSite):

site_title = "Cal-ITP Benefits Administrator"
site_header = "Administrator"
index_title = "Dashboard"
Comment on lines +11 to +13
Copy link
Member

Choose a reason for hiding this comment

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

Just wanted to note here: @lalver1 and I had thought about setting these back when he was working on #2313; we chose not to because we didn't want to affect the superuser view.

However, now that we have the spec from product for the page titles and can see more places that need this copy, I think it makes sense to go ahead and set/use these attributes.

So the note here is that the superuser view will now use this copy as well, and I think that is perfectly fine. (We'll probably make the superuser UI and transit agency staff UI look more similar at some point anyways.)

dev-benefits this branch
image image

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for documenting this!


def index(self, request, extra_context=None):
"""
Display the main admin index page if the user is a superuser or a "staff_group" user.
Expand Down Expand Up @@ -42,6 +46,7 @@ def index(self, request, extra_context=None):
{
"has_permission_for_in_person": has_permission_for_in_person,
"transit_processor_portal_url": transit_processor_portal_url,
"title": f"{agency.long_name} | {self.index_title} | {self.site_title}",
}
)

Expand Down
4 changes: 4 additions & 0 deletions benefits/in_person/templates/error-base.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{% extends "admin/agency-base.html" %}
{% load static %}

{% block title %}
{{ title }}
{% endblock title %}

{% block content %}
<div class="row justify-content-center">
<div class="col-lg-6">
Expand Down
2 changes: 1 addition & 1 deletion benefits/in_person/templates/in_person/eligibility.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{% extends "admin/agency-base.html" %}

{% block title %}
Agency dashboard: In-person enrollment
{{ title }}
{% endblock title %}

{% block content %}
Expand Down
4 changes: 4 additions & 0 deletions benefits/in_person/templates/in_person/enrollment.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
{% extends "admin/agency-base.html" %}

{% block title %}
{{ title }}
{% endblock title %}

{% block content %}

<div class="row justify-content-center">
Expand Down
4 changes: 4 additions & 0 deletions benefits/in_person/templates/in_person/enrollment/success.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{% extends "admin/agency-base.html" %}
{% load static %}

{% block title %}
{{ title }}
{% endblock title %}

{% block content %}
<div class="row justify-content-center">
<div class="col-lg-6">
Expand Down
38 changes: 32 additions & 6 deletions benefits/in_person/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ def eligibility(request):
"""View handler for the in-person eligibility flow selection form."""

agency = session.agency(request)
context = {**admin_site.each_context(request), "form": forms.InPersonEligibilityForm(agency=agency)}
context = {
**admin_site.each_context(request),
"form": forms.InPersonEligibilityForm(agency=agency),
"title": f"{agency.long_name} | In-person enrollment | {admin_site.site_title}",
}

if request.method == "POST":
form = forms.InPersonEligibilityForm(data=request.POST, agency=agency)
Expand Down Expand Up @@ -130,14 +134,20 @@ def enrollment(request):
"form_server_error": tokenize_server_error_form.id,
"form_success": tokenize_success_form.id,
"form_system_error": tokenize_system_error_form.id,
"title": f"{agency.long_name} | In-person enrollment | {admin_site.site_title}",
machikoyasuda marked this conversation as resolved.
Show resolved Hide resolved
}

return TemplateResponse(request, "in_person/enrollment.html", context)


def reenrollment_error(request):
"""View handler for a re-enrollment attempt that is not yet within the re-enrollment window."""
context = {**admin_site.each_context(request)}

agency = session.agency(request)
context = {
**admin_site.each_context(request),
"title": f"{agency.long_name} | In-person enrollment | {admin_site.site_title}",
}

flow = session.flow(request)
context["flow_label"] = flow.label
Expand All @@ -147,27 +157,43 @@ def reenrollment_error(request):

def retry(request):
"""View handler for card verification failure."""
context = {**admin_site.each_context(request)}
agency = session.agency(request)
context = {
**admin_site.each_context(request),
"title": f"{agency.long_name} | In-person enrollment | {admin_site.site_title}",
}

return TemplateResponse(request, "in_person/enrollment/retry.html", context)


def system_error(request):
"""View handler for an enrollment system error."""
context = {**admin_site.each_context(request)}
agency = session.agency(request)
context = {
**admin_site.each_context(request),
"title": f"{agency.long_name} | In-person enrollment | {admin_site.site_title}",
}

return TemplateResponse(request, "in_person/enrollment/system_error.html", context)


def server_error(request):
"""View handler for errors caused by a misconfiguration or bad request."""
context = {**admin_site.each_context(request)}
agency = session.agency(request)
context = {
**admin_site.each_context(request),
"title": f"{agency.long_name} | In-person enrollment | {admin_site.site_title}",
}

return TemplateResponse(request, "in_person/enrollment/server_error.html", context)


def success(request):
"""View handler for the final success page."""
context = {**admin_site.each_context(request)}
agency = session.agency(request)
context = {
**admin_site.each_context(request),
"title": f"{agency.long_name} | In-person enrollment | {admin_site.site_title}",
}

return TemplateResponse(request, "in_person/enrollment/success.html", context)
5 changes: 1 addition & 4 deletions benefits/templates/admin/agency-base.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@
<link href="{% static "css/admin/styles.css" %}" rel="stylesheet">
{% endblock extrastyle %}

{% block title %}
{% endblock title %}

{% block branding %}
<div id="header-container" class="navbar navbar-expand-sm navbar-dark justify-content-between">
<div class="container">
Expand All @@ -28,7 +25,7 @@
<img class="lg d-none d-lg-block" src="{% static "img/logo-lg.svg" %}" width="220" height="50" alt="California Integrated Travel Project: Benefits logo (large)" />
</span>
</div>
<h1 class="p-0 m-0 text-white">Administrator</h1>
<h1 class="p-0 m-0 text-white">{{ site_header }}</h1>
</div>
{% endblock branding %}

Expand Down
2 changes: 1 addition & 1 deletion benefits/templates/admin/agency-dashboard-index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{% load i18n static %}

{% block title %}
Agency dashboard
{{ title }}
{% endblock title %}

{% block content %}
Expand Down
6 changes: 1 addition & 5 deletions benefits/templates/admin/login.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
{% block dark-mode-vars %}
{% endblock dark-mode-vars %}

{% block title %}
Log in | Cal-ITP Benefits Administrator
{% endblock title %}

Comment on lines -7 to -10
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not necessary here, because the {% extends "admin/login.html" %} default Django login page's title uses Log in | site_title.

{% block extrastyle %}
{% comment %} Overriding instead of extending agency-base here to remove jQuery declaration, which admin/login.html includes on its own {% endcomment %}
<link href="{% static "img/favicon.ico" %}" rel="icon" type="image/x-icon" />
Expand All @@ -24,6 +20,6 @@
</div>

<div id="site-name">
<h1 class="text-center text-white fs-3 py-3 m-0">Administrator</h1>
<h1 class="text-center text-white fs-3 py-3 m-0">{{ site_header }}</h1>
</div>
{% endblock branding %}
4 changes: 2 additions & 2 deletions benefits/templates/registration/logged_out.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{% load static %}

{% block title %}
Logged out | Cal-ITP Benefits Administrator
Logged out | {{ site_title }}
{% endblock title %}
Comment on lines 4 to 6
Copy link
Member Author

Choose a reason for hiding this comment

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

This logged_out page extends from agency-base, and not the default Django admin log out template, so it's necessary to explicitly spell out Logged out

Copy link
Member

Choose a reason for hiding this comment

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

(can defer this suggestion)

Since we're now setting site_title, etc. on BenefitsAdminSite, I wonder if it'd make sense to make use of the title logic in base_site.html:

{% block title %}{% if subtitle %}{{ subtitle }} | {% endif %}{{ title }} | {{ site_title|default:_('Django site admin') }}{% endblock %}

The idea is we would make agency-base.html extend base_site.html instead of base.html. We'd then set subtitle and title in the context dict, which better aligns with how the default Django admin app configures its page titles, e.g. login.

The way you've implemented setting page titles in this PR works, and I know we're trying to finish this out soon, so maybe we defer using this approach for the future, if ever.


{% block extrastyle %}
Expand All @@ -20,7 +20,7 @@
</div>

<div id="site-name">
<h1 class="text-center text-white fs-3 py-3 m-0">Administrator</h1>
<h1 class="text-center text-white fs-3 py-3 m-0">{{ site_header }}</h1>
</div>
{% endblock branding %}

Expand Down
10 changes: 9 additions & 1 deletion tests/pytest/in_person/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def test_enrollment_post_valid_form_reenrollment_error(mocker, admin_client, car


@pytest.mark.django_db
@pytest.mark.usefixtures("mocked_session_flow")
@pytest.mark.usefixtures("mocked_session_flow", "mocked_session_agency")
def test_reenrollment_error(admin_client):
path = reverse(routes.IN_PERSON_ENROLLMENT_REENROLLMENT_ERROR)

Expand All @@ -327,6 +327,8 @@ def test_reenrollment_error(admin_client):
assert response.template_name == "in_person/enrollment/reenrollment_error.html"


@pytest.mark.django_db
@pytest.mark.usefixtures("mocked_session_flow", "mocked_session_agency")
def test_retry(admin_client):
path = reverse(routes.IN_PERSON_ENROLLMENT_RETRY)

Expand All @@ -336,6 +338,8 @@ def test_retry(admin_client):
assert response.template_name == "in_person/enrollment/retry.html"


@pytest.mark.django_db
@pytest.mark.usefixtures("mocked_session_flow", "mocked_session_agency")
def test_system_error(admin_client):
path = reverse(routes.IN_PERSON_ENROLLMENT_SYSTEM_ERROR)

Expand All @@ -345,6 +349,8 @@ def test_system_error(admin_client):
assert response.template_name == "in_person/enrollment/system_error.html"


@pytest.mark.django_db
@pytest.mark.usefixtures("mocked_session_flow", "mocked_session_agency")
def test_server_error(admin_client):
path = reverse(routes.IN_PERSON_SERVER_ERROR)

Expand All @@ -354,6 +360,8 @@ def test_server_error(admin_client):
assert response.template_name == "in_person/enrollment/server_error.html"


@pytest.mark.django_db
@pytest.mark.usefixtures("mocked_session_flow", "mocked_session_agency")
def test_success(admin_client):
path = reverse(routes.IN_PERSON_ENROLLMENT_SUCCESS)

Expand Down
Loading