isomerpages / isomercms-backend

A static website builder and host for the Singapore Government
5 stars 1 forks source link

feat(e2e): create new e2e email users #782

Closed seaerchin closed 1 year ago

seaerchin commented 1 year ago

Problem

Our e2e user account at present is only valid for github login; hence, we have no way of testing email logins

Closes IS-206

Solution

  1. piggyback on the verification bypass in AuthenticationMiddlewareService

notes

seaerchin commented 1 year ago

Hey, I have a concern regarding the implementation of this :( At the moment, if we want to add more e2e users to test more sophisticated flows, we would need to keep adding more env vars. Newer devs would also have to be aware that when they run e2e, they would need to create the new e2e users and manually update the env var for this.

How do we feel about naming the users intentionally, eg e2eEmailFlowAdmin1, e2eEmailFlowCollab1 and then performing the findOrCreate() from our UsersService to get the user ID? that way we dont have to touch the env vars, and we could leverage on this to create multiple users for our e2e test suite in the future

this is a good point! however, i'm not inclined to make the change here - this is because i'm not exactly sure how we'll get + use the isomerUserId, which makes it difficult to change. however, i'll first change it from returning a fixed object (in extractE2eUserInfo) to using a function to get the desired shape so that we can more easily change this in the future if required. will file this as an issue

seaerchin commented 1 year ago

filed here

seaerchin commented 1 year ago

this is a good point! however, i'm not inclined to make the change here - this is because i'm not exactly sure how we'll get + use the isomerUserId, which makes it difficult to change. however, i'll first change it from returning a fixed object (in extractE2eUserInfo) to using a function to get the desired shape so that we can more easily change this in the future if required. will file this as an issue

just checking in, this PR already addresses this right? since now got no more env var for this + there is code to find or create here?

yeap! i wanted to write a few specs out first to see whether there was any complications w/ user creation etc. should've marked it as draft maybe and that was why i didn't request a review but now it uses the db cos the user management is still done thru db ultimately