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

use the email address from the create test user response #2791

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

johnduffell
Copy link
Member

Traditionally test users have used the first name field to determine whether someone is a test user or not.

However this is less reliable than using email address as the email address field is passed around everywhere whereas there has to be special logic to get test users' first name around to the right places (as for a "real" user, first name would be extra PII)

In some other PRs, changes are proposed to embed the generated token into email addresses. This means that the email address is generated on the fly, so anything using the create test user endpoint, needs to look in the response to find out what the generated email address is.

--

This PR updates gateway to get the email address out of the response, rather than relying on what was passed in to be correct. See https://github.com/guardian/identity/pull/2528 for the PR that makes the changes that this PR depends on.

@johnduffell johnduffell requested a review from a team as a code owner July 3, 2024 20:37
@@ -253,7 +253,7 @@ export const createTestUser = ({
})
.then((res) => {
return cy.wrap({
emailAddress: finalEmail,
emailAddress: res.body.emailAddress as string,
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@AshCorr AshCorr left a comment

Choose a reason for hiding this comment

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

Cypress seems to be failing now!

@johnduffell
Copy link
Member Author

Cypress seems to be failing now!

thanks yes once the PR this depends on is merged, the tests should be able to pass
See https://github.com/guardian/identity/pull/2528 for the PR that makes the changes that this PR depends on.

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.

2 participants