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

ATO-925: Fix double email entry bug #841

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

Ryan-Andrews99
Copy link
Contributor

@Ryan-Andrews99 Ryan-Andrews99 commented Jul 23, 2024

Onboarding Feature Deployment

Warning

Pull requests merged to main will be released to production, please ensure the checklist below is complete

Before any work can be merged to main in must meet the definition of done and be ready to deploy. While many of these tasks will be automated, the reviewers must take the responsibility of confirming the checklist below has been completed before this ticket can be merged.

Checklist

  • this pull request meets the acceptance criteria of the ticket

  • this branch is up-to-date with the main branch

    git fetch --all && git rebase origin/main

  • these changes are backwards compatible (no breaking changes)

    • all methods signatures and return values are the same
    • any replaced methods are marked as @deprecated
  • tests have been written to cover any new or updated functionality

  • new configuration parameters have been deployed to all environments, see configuration management.

  • all external infrastructure dependencies have been updated in all environments

Changes

[ please list the changes this pull request is making ]

Added for new features

Changed for changes in existing functionality

Deprecated for soon-to-be removed features

Removed for now removed features

Fixed for any bug fixes

  • Fixes the double email entry bug that occurs when registering with no previous session
  • Makes the acceptance tests easier to run locally as this deals with some of the flakiness

Security in case of vulnerabilities

@Ryan-Andrews99 Ryan-Andrews99 requested a review from a team as a code owner July 23, 2024 09:38
@Ryan-Andrews99 Ryan-Andrews99 requested review from a team July 23, 2024 09:38
@Ryan-Andrews99 Ryan-Andrews99 force-pushed the ATO-925-fix-double-email-entry-bug branch from a12d816 to b2bf246 Compare July 23, 2024 09:39
@kalpaitch kalpaitch force-pushed the ATO-925-fix-double-email-entry-bug branch from b2bf246 to 91b231b Compare August 1, 2024 08:58
We will need to assert session.save is called so adding this to our mock
request object. I have also narrowed the type as object is the incorrect
type here as this includes arrays.
The next controller in the user flow, showCheckEmailForm relies on the
session containing the email address. However we redirect when submitting
the email address, which may happen faster than our session updating,
leading to the user being redirected back to the enter-email page. The
docs for express-session say "There are some cases where it is useful to
call this method (`session.save()`), for example, redirects..."[1] so
adding this session.save call in to ensure the session is updated before
the redirect occurs.
[deploy]

[1]- https://expressjs.com/en/resources/middleware/session.html
@Ryan-Andrews99 Ryan-Andrews99 force-pushed the ATO-925-fix-double-email-entry-bug branch from 91b231b to 3664077 Compare August 6, 2024 09:39
The AWS sdk has a vunerability in fast-xml parser[1]. This just bumps the
underlying dependency.

[1]- https://github.com/govuk-one-login/onboarding-self-service-experience/security/dependabot/67
@kalpaitch kalpaitch enabled auto-merge August 6, 2024 09:51
@kalpaitch kalpaitch added this pull request to the merge queue Aug 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 6, 2024
@Ryan-Andrews99 Ryan-Andrews99 added this pull request to the merge queue Aug 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 6, 2024
@kalpaitch kalpaitch added this pull request to the merge queue Aug 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 7, 2024
@kalpaitch kalpaitch added this pull request to the merge queue Aug 7, 2024
Merged via the queue into main with commit 741f73c Aug 7, 2024
15 checks passed
@kalpaitch kalpaitch deleted the ATO-925-fix-double-email-entry-bug branch August 7, 2024 14:54
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.

3 participants