Id 367 get user registration from sam #4716
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ghost-in-a-Jar
approved these changes
Mar 19, 2024
tlangs
requested changes
Mar 19, 2024
| programLocationCountry: 'testProgramLocationCountry', | ||
| researchArea: 'testResearchArea', | ||
| starredWorkspaces: 'testStarredWorkspaces', | ||
| }; |
tlangs
requested changes
Mar 20, 2024
Shakespeared
commented
Mar 26, 2024
| export interface RexFirstTimestampResponse { | ||
| timestamp: Date; | ||
| } | ||
|
|
||
| export interface SamUserResponse { |
There was a problem hiding this comment.
added the undefined so we can initialize with an empty object in state
tlangs
requested changes
Mar 27, 2024
| const { terraUser, samUser } = userStore.get(); | ||
| // for Sam users who registered before we started tracking registration dates in Sam, | ||
| // registeredAt may be null in the Sam db. In that case, use epoch instead | ||
| const dateJoined = samUser.registeredAt ? samUser.registeredAt.getTime() : new Date('1970-01-01').getTime(); |
There was a problem hiding this comment.
We could also just rely on the createdAt date, since its not nullable in Sam's DB. That might be preferable, just in case there's a real createdAt, but not a registeredAt, for some reason.
There was a problem hiding this comment.
I don't think we want to user createdAt because then a user who is invited, waits a month to register, then comes to Terra would immediately be shown the survey
tlangs
approved these changes
Apr 2, 2024
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Jira Ticket: https://broadworkbench.atlassian.net/browse/ID-367
Summary of changes:
remove use of Rex in the UI. Rex is only used to get the registration date to send to Appcues for the NPS Survey. This change gets the registration date from Sam instead, and removes references to Rex from the UI.
The unit tests in login.test.ts are testing new code areas, so I would appreciate a thorough review of those as well. Since these are some of the first auth.ts tests, I want to ensure we're setting a good example for future tests to work from.
Why
We are retiring Rex because we now have the same data stored in Sam
Testing strategy
Appcues itself isn't activated on lower envs, so I'm actually not sure how to test that. However I did verify that the result returned from Sam is the same as was returned from Rex. For testing the changes generally, logging into Terra should be sufficient as it is called on loadTerraUser which is called upon login.
Manually tested general functionality by running locally and logging in