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

Update sign in page error handling #978

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

matthew-robinson-ons
Copy link
Contributor

@matthew-robinson-ons matthew-robinson-ons commented Feb 7, 2025

What and why?

This PR updates the error handling on the sign in page. It was identified in the DAC report that the error summary displayed a misleading please try again message. This has been updated with links to the specific errors.
The error summary was also being displayed as an info summary which has been updated.

How to test?

Navigate to the sign in page and generate a sign in error. Check the resulting error summary and components meets the requirements on the DAC report.

Check all other sign in page functionality remains unchanged.

Jira

RAS-1364

@matthew-robinson-ons matthew-robinson-ons requested a review from a team as a code owner February 7, 2025 12:27
@@ -42,47 +60,7 @@
{% endwith %}
<form method="post" class="form">
{{ form.csrf_token }}
{% if form.errors %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't find how this was used

Copy link
Contributor

@arroyoAle arroyoAle left a comment

Choose a reason for hiding this comment

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

Looks fine for this page, I see the comment on the card but I don't know if there has been any resolution about this being just for the sign in page

<h2>Sign in</h2>
{% if failed_authentication or form.username.errors | length > 0 %}
{% set error = { "text": "Email address is required" } %}
{% set errorUN = { "text": "Email Address is required", "id": 'username_error' } %}
Copy link
Contributor

Choose a reason for hiding this comment

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

At first glance this variable confused me, maybe something like error_username?

{% if failed_authentication or form.password.errors | length > 0 %}
{% set error = { "text": "Password is required" } %}
{% set errorPW = { "text": "Password is required", "id": 'password_error' } %}
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

@matthew-robinson-ons
Copy link
Contributor Author

Yeah have checked and this is just the sign in page.

Looks fine for this page, I see the comment on the card but I don't know if there has been any resolution about this being just for the sign in page

Copy link
Contributor

@arroyoAle arroyoAle left a comment

Choose a reason for hiding this comment

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

Happy with this now

@anwilkie
Copy link
Contributor

/deploy wilkia

@ras-rm-pr-bot
Copy link
Collaborator

Deploying to dev cluster with following parameters:

  • namespace: wilkia

  • tag: pr-978

  • configBranch: main

  • paramKey: ``

  • paramValue: ``

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants