Skip to content

Commit

Permalink
Display error banners when saving duplicate template category and ema…
Browse files Browse the repository at this point in the history
…il branding entities (#2094)

* Display banners when saving duplicates

- Banners will now be displayed when saving duplicate template categories or email branding
- Added error message FR translations

* Fix punctuation in translations

* Add cypress tests for new template category errors

- Refactored how the template category tests work. These tests now use dynamically generated TC ids to ensure that when the tests are run in parallel with different cypress users there will not be any collisions or deletions of TCs being used by other tests.

* Update package.json and lock

* Add error banner UI tests for email branding

- Set up scaffolding for the email branding list and management page
- Added more data-testid attributes to email branding markup

* Fix slow brain mistakes

* Preserve branding form details when update fails

* Preserve form data when updating existing Template Category

* Preserve form data when creating TCs or email branding fails
  • Loading branch information
whabanks authored Mar 3, 2025
1 parent 2737a18 commit 8c0ffdd
Show file tree
Hide file tree
Showing 16 changed files with 540 additions and 145 deletions.
69 changes: 47 additions & 22 deletions app/main/views/email_branding.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
)
from flask_babel import _
from flask_login import current_user
from notifications_python_client.errors import HTTPError
from notifications_utils.template import HTMLEmailTemplate

from app import current_service, email_branding_client, organisations_client
Expand Down Expand Up @@ -89,17 +90,22 @@ def update_email_branding(branding_id, logo=None):

updated_logo_name = permanent_email_logo_name(logo, session["user_id"]) if logo else None

email_branding_client.update_email_branding(
branding_id=branding_id,
logo=updated_logo_name,
name=form.name.data,
text=form.text.data,
colour=form.colour.data,
brand_type=form.brand_type.data,
organisation_id=form.organisation.data if form.organisation.data != "-1" else None,
alt_text_en=form.alt_text_en.data,
alt_text_fr=form.alt_text_fr.data,
)
try:
email_branding_client.update_email_branding(
branding_id=branding_id,
logo=updated_logo_name,
name=form.name.data,
text=form.text.data,
colour=form.colour.data,
brand_type=form.brand_type.data,
organisation_id=form.organisation.data if form.organisation.data != "-1" else None,
alt_text_en=form.alt_text_en.data,
alt_text_fr=form.alt_text_fr.data,
)
except HTTPError as e:
if e.status_code == 400 and "already exists" in e.message:
flash(_(e.message), "error")
return redirect(url_for(".update_email_branding", branding_id=branding_id))

if logo:
persist_logo(logo, updated_logo_name)
Expand All @@ -122,7 +128,11 @@ def update_email_branding(branding_id, logo=None):
@user_is_platform_admin
def create_email_branding(logo=None):
all_organisations = organisations_client.get_organisations()
form = ServiceUpdateEmailBranding(brand_type="custom_logo", organisation="-1")
form = (
ServiceUpdateEmailBranding(**session["form_data"])
if session.get("form.data")
else ServiceUpdateEmailBranding(brand_type="custom_logo", organisation="-1")
)

form.organisation.choices = [(org["id"], org["name"]) for org in all_organisations]
# add the option for no org
Expand All @@ -144,16 +154,31 @@ def create_email_branding(logo=None):

updated_logo_name = permanent_email_logo_name(logo, session["user_id"]) if logo else None

email_branding_client.create_email_branding(
logo=updated_logo_name,
name=form.name.data,
text=form.text.data,
colour=form.colour.data,
brand_type=form.brand_type.data,
organisation_id=None if form.organisation.data == "-1" else form.organisation.data,
alt_text_en=form.alt_text_en.data,
alt_text_fr=form.alt_text_fr.data,
)
try:
email_branding_client.create_email_branding(
logo=updated_logo_name,
name=form.name.data,
text=form.text.data,
colour=form.colour.data,
brand_type=form.brand_type.data,
organisation_id=None if form.organisation.data == "-1" else form.organisation.data,
alt_text_en=form.alt_text_en.data,
alt_text_fr=form.alt_text_fr.data,
)
except HTTPError as e:
if e.status_code == 400 and "already exists" in e.message:
session["form_data"] = {
"logo": updated_logo_name,
"name": form.name.data,
"text": form.text.data,
"colour": form.colour.data,
"brand_type": form.brand_type.data,
"organisation": form.organisation.data,
"alt_text_en": form.alt_text_en.data,
"alt_text_fr": form.alt_text_fr.data,
}
flash(_(e.message), "error")
return redirect(url_for(".create_email_branding"))

if logo:
persist_logo(logo, updated_logo_name)
Expand Down
64 changes: 42 additions & 22 deletions app/main/views/templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -1380,19 +1380,34 @@ def delete_template_category(template_category_id):
@main.route("/template-category/add", methods=["GET", "POST"])
@user_is_platform_admin
def add_template_category():
form = TemplateCategoryForm()
form = TemplateCategoryForm(**session["form_data"]) if session.get("form_data") else TemplateCategoryForm()

if form.validate_on_submit():
template_category_api_client.create_template_category(
name_en=form.data["name_en"],
name_fr=form.data["name_fr"],
description_en=form.data["description_en"],
description_fr=form.data["description_fr"],
hidden=form.data["hidden"],
email_process_type=form.data["email_process_type"],
sms_process_type=form.data["sms_process_type"],
sms_sending_vehicle=form.data["sms_sending_vehicle"],
)
try:
template_category_api_client.create_template_category(
name_en=form.data["name_en"],
name_fr=form.data["name_fr"],
description_en=form.data["description_en"],
description_fr=form.data["description_fr"],
hidden=form.data["hidden"],
email_process_type=form.data["email_process_type"],
sms_process_type=form.data["sms_process_type"],
sms_sending_vehicle=form.data["sms_sending_vehicle"],
)
except HTTPError as e:
if e.status_code == 400 and "already exists" in e.message:
session["form_data"] = {
"name_en": form.data["name_en"],
"name_fr": form.data["name_fr"],
"description_en": form.data["description_en"],
"description_fr": form.data["description_fr"],
"hidden": form.data["hidden"],
"email_process_type": form.data["email_process_type"],
"sms_process_type": form.data["sms_process_type"],
"sms_sending_vehicle": form.data["sms_sending_vehicle"],
}
flash(_(e.message), "error")
return redirect(url_for(".add_template_category"))
flash(
_("Template category '{}' added.").format(
form.data["name_en"] if session["userlang"] == "en" else form.data["name_fr"]
Expand Down Expand Up @@ -1420,17 +1435,22 @@ def template_category(template_category_id):
)

if form.validate_on_submit():
template_category_api_client.update_template_category(
template_category_id,
name_en=form.data["name_en"],
name_fr=form.data["name_fr"],
description_en=form.data["description_en"],
description_fr=form.data["description_fr"],
hidden=form.data["hidden"],
email_process_type=form.data["email_process_type"],
sms_process_type=form.data["sms_process_type"],
sms_sending_vehicle=form.data["sms_sending_vehicle"],
)
try:
template_category_api_client.update_template_category(
template_category_id,
name_en=form.data["name_en"],
name_fr=form.data["name_fr"],
description_en=form.data["description_en"],
description_fr=form.data["description_fr"],
hidden=form.data["hidden"],
email_process_type=form.data["email_process_type"],
sms_process_type=form.data["sms_process_type"],
sms_sending_vehicle=form.data["sms_sending_vehicle"],
)
except HTTPError as e:
if e.status_code == 400 and "already exists" in e.message:
flash(_(e.message), "error")
return redirect(url_for(".template_category", template_category_id=template_category_id))
flash(
_("Template category '{}' saved.").format(
form.data["name_en"] if session["userlang"] == "en" else form.data["name_fr"]
Expand Down
19 changes: 10 additions & 9 deletions app/templates/components/file-upload.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
alternate_link=None,
alternate_link_text=None,
hint=None,
accept=""

accept="",
testid=None
) %}
<form method="post" enctype="multipart/form-data" {% if action %}action="{{ action }}"{% endif %} data-module="file-upload">

{{ file_upload_field(field=field, action=action, button_text=button_text, hint=hint, accept=accept) }}

{% if alternate_link and alternate_link_text %}
Expand All @@ -18,7 +18,7 @@
</span>
{% endif %}
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<button type="submit" class="file-upload-submit">{{ _('Submit') }}</button>
<button type="submit" class="file-upload-submit" testid="{{ testid }}">{{ _('Submit') }}</button>
</form>
{% endmacro %}

Expand All @@ -29,24 +29,25 @@
button_text=_('Choose file'),
button_class="",
hint=None,
accept=""
accept="",
testid=None
) %}
<div class="file-upload-group {% if field.errors %}form-group-error{% endif %} relative inline-flex flex-col gap-2 items-start">

{% if hint %}
<span class="form-hint">
{{ hint }}
</span>
{% endif %}

{% if field.errors %}
<span class="error-message">
{{ field.errors[0] }}
</span>
{% endif %}

{{ field(**{ 'class': 'file-upload-field', 'accept': accept, 'aria-describedby': 'file-description', 'data-error-msg': field.validators and field.validators[0].message, 'data-testid': field_testid}) }}

<label id="file-upload-button" class="file-upload-button button {{ button_class }}" for="{{ field.name }}" role="link">{{ button_text }}</label>

<div class="file-upload-extra pt-gutterHalf flex flex-col gap-4">
Expand Down
4 changes: 3 additions & 1 deletion app/templates/components/live-search.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
target_selector=None,
show=False,
form=None,
label=None
label=None,
testid=None
) %}
{%- set search_label = label or form.search.label.text %}
{% if show %}
Expand All @@ -16,6 +17,7 @@
label=search_label,
autocomplete="off",
required=false,
testid=testid,
**kwargs
) }}
</div>
Expand Down
17 changes: 9 additions & 8 deletions app/templates/views/email-branding/manage-branding.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,21 @@
<p>
{{ _('Logos should be PNG files, 108px high') }}
</p>
{{ file_upload(form.file, button_text='{} logo'.format('Update' if email_branding else 'Upload')) }}
{{ file_upload(form.file, button_text='{} logo'.format('Update' if email_branding else 'Upload'), testid='upload-branding') }}
{% call form_wrapper() %}
<div class="form-group contain-floats box-border mb-gutterHalf md:mb-gutter">
<div style='margin-top:15px;'>{{textbox(form.name)}}</div>
<div style='margin-top:15px;'>{{textbox(form.text)}}</div>
<div style='margin-top:15px;'>{{textbox(form.alt_text_en)}}</div>
<div style='margin-top:15px;'>{{textbox(form.alt_text_fr)}}</div>
{{ textbox(form.colour, width='w-full md:w-1/4', colour_preview=True) }}
{{ radios(form.brand_type) }}
<div style='margin-top:15px;'>{{textbox(form.name, testid='branding-name')}}</div>
<div style='margin-top:15px;'>{{textbox(form.text, testid='branding-text')}}</div>
<div style='margin-top:15px;'>{{textbox(form.alt_text_en, testid='branding-alt-text-en')}}</div>
<div style='margin-top:15px;'>{{textbox(form.alt_text_fr, testid='branding-alt-text-fr')}}</div>
{{ textbox(form.colour, width='w-full md:w-1/4', colour_preview=True, testid='branding-color') }}
{{ radios(form.brand_type, testid='') }}
{{ radios(form.organisation) }}
{{ page_footer(
_('Save'),
button_name='operation',
button_value='email-branding-details'
button_value='email-branding-details',
testid='save-branding'
) }}
</div>
{% endcall %}
Expand Down
6 changes: 3 additions & 3 deletions app/templates/views/email-branding/select-branding.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@
{% block platform_admin_content %}

{{ page_header(_('Email branding'), id_text='email_branding') }}
{{ live_search(target_selector='.email-brand', show=True, form=search_form) }}
{{ live_search(target_selector='.email-brand', testid='branding-search', show=True, form=search_form) }}
{% for brand in email_brandings %}
<p class="email-brand">
<a href="{{ url_for('.update_email_branding', branding_id=brand.id) }}">
<a href="{{ url_for('.update_email_branding', branding_id=brand.id) }}" data-testid="edit-branding-{{ brand.id }}">
{{ brand.name or 'Unnamed' }}
</a>
</p>
{% endfor %}
<div class="js-stick-at-bottom-when-scrolling">
<a id="create_email_branding" href="{{ url_for('.create_email_branding') }}" class="button button-secondary">{{ _('New branding') }}</a>
<a id="create_email_branding" href="{{ url_for('.create_email_branding') }}" data-testid="create-branding" class="button button-secondary">{{ _('New branding') }}</a>
</div>

{% endblock %}
3 changes: 3 additions & 0 deletions app/translations/csv/fr.csv
Original file line number Diff line number Diff line change
Expand Up @@ -2122,5 +2122,8 @@
"Ask which team members have permission to invite you. If the team is unsure, from a GC Notify account visit the main menu and select “Team members.” That page:","Demandez quels ou quelles membres de l’équipe ont la permission de vous inviter. Si l’équipe n’en est pas certaine, utilisez un compte Notification GC pour visiter le menu principal et rendez-vous dans la section « Votre équipe ». Cette page comprend :"
"Includes an invitation button at the top of the page, if the member is permitted to send invitations.","un bouton d’invitation au haut de la page si le ou la membre de l’équipe a la permission d’envoyer des invitations; et"
"Lists permitted tasks under the name of each member.","sous le nom de chaque membre de l’équipe, une liste des tâches que cette personne a l’autorisation de réaliser."
"Email branding already exists, name must be unique.","L'image de marque du courriel existe déjà. Le nom doit être unique."
"Template category already exists, name_en and name_fr must be unique.","La catégorie de gabarit existe déjà. Les noms en anglais et en français doivent être uniques."
"Entity already exists.","L'entité existe déjà."
"You're no longer on the team for “{}”","Vous ne faites plus partie de l’équipe du service “{}”"
"GC Notify cannot send text messages to some international numbers","Il y a des numéros internationaux auxquels Notification GC ne peut pas envoyer de messages texte"
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
let Components = {
BrandingSearchBox: () => cy.getByTestId('branding-search'),
CreateBrandingButton: () => cy.getByTestId('create-branding'),
}

let Actions = {
SearchForBranding: (name) => {
Components.BrandingSearchBox().type(name);
},
CreateBranding: () => {
Components.CreateBrandingButton().click();
},
EditBrandingByName: (name) => {
cy.get('a').contains(name).click();
},
EditBrandingById: (id) => {
cy.getByTestId(`edit-branding-${id}`)
}
}

let EmailBrandingPage = {
Components,
...Actions
};

export default EmailBrandingPage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
let Components = {
UploadLogoButton: () => cy.getByTestId('upload-logo'),
BrandName: () => cy.getByTestId('branding-name'),
BrandText: () => cy.getByTestId('branding-text'),
BrandAltTextEn: () => cy.getByTestId('branding-alt-text-en'),
BrandAltTextFr: () => cy.getByTestId('branding-alt-text-fr'),
BrandColour: () => cy.getByTestId('branding-colour'),
CreateBrandingButton: () => cy.getByTestId('save-branding'),
}

let BrandingTypes = {
BOTH_ENGLISH: "both_english",
BOTH_FRENCH: "both_french",
CUSTOM_LOGO: "custom_logo",
CUSTOM_LOGO_WITH_BG_COLOUR: "custom_logo_with_background_colour",
NO_BRANDING: "no_branding",
}


let Actions = {
UploadLogo: () => {
// TODO
},
SetBrandName: (name) => {
Components.BrandName().clear().type(name);
},
SetBrandText: (text) => {
Components.BrandText().clear().type(text);
},
SetBrandAltTextEn: (text) => {
Components.BrandAltTextEn().clear().type(text);
},
SetBrandAltTextFr: (text) => {
Components.BrandAltTextFr().clear().type(text);
},
SetBrandColour: (colour) => {
Components.BrandColour().clear().type(colour);
},
SetBrandType: (type) => {
cy.getByTestId(type).check();
},
SetOrganisation: (organisation) => {
// Use no org for the time being until we have a way to fetch a list of
// valid organisation IDs as the testid's for these radios are org ids
// which will change between environments.
cy.get('input[id="organisation-5"]').check();
},
Submit: () => {
Components.CreateBrandingButton().click();
}
}

let ManageEmailBrandingPage = {
Components,
BrandingTypes,
...Actions
}

export default ManageEmailBrandingPage;
Loading

0 comments on commit 8c0ffdd

Please sign in to comment.