-
Notifications
You must be signed in to change notification settings - Fork 24
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
1653/user invite flow frontend #1857
Conversation
✔️ Deploy Preview for dev-partners-bloom ready! 🔨 Explore the source changes: f9d3483 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-partners-bloom/deploys/6153390bf84b640007b521d6 😎 Browse the preview: https://deploy-preview-1857--dev-partners-bloom.netlify.app |
✔️ Deploy Preview for dev-bloom ready! 🔨 Explore the source changes: f9d3483 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-bloom/deploys/6153390b59e7770007975860 😎 Browse the preview: https://deploy-preview-1857--dev-bloom.netlify.app |
✔️ Deploy Preview for dev-storybook-bloom ready! 🔨 Explore the source changes: f9d3483 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-storybook-bloom/deploys/6153390b95d44b0007b31ef1 😎 Browse the preview: https://deploy-preview-1857--dev-storybook-bloom.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small typo in the e-mail To complete your account creation, please the link below:
just need the word click
.
The e-mail subject should be Welcome to the Partners Portal
and the content is a little different, but it's in the issue :)
When I click "Confirm my Account" from the email I'm taken to the partners sign-in page but I don't have a password yet so I'm not sure what the flow there should be. Maybe the "create a password" flow is a separate issue? |
/* Fetch user list */ | ||
const { data: userList } = useUserList({ | ||
page: currentPage, | ||
limit: itemsPerPage, | ||
}) | ||
|
||
/* Fetch listings */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seanmalbert Might eventually be worth a new query that only retrieves name, jurisdiction, and ID
lastName, | ||
email, | ||
roles, | ||
leasingAgentInListings: leasingAgentInListings, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NB but don't need the colons here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're automagically added by linter ;D
I'm curious about my two comments above still - the content and subject of the email is not quite right and the fact that I am redirected to sign-in without having set up a password (which is maybe a later ticket?) |
@emilyjablonski Flow for leasing agents to update their password is in a separate issue, #1654 Email copy from issues re-pasted below for reference :) Email Copy From: no-reply@housingbayarea.org Hello Name, Welcome to the the Partners Portal on [URL]. You will now be able to manage listings and applications that you are a part of from one centralized location. To complete your account creation, please click the link below: Confirm my account Thank you, |
@emilyjablonski I updated translations and resolved conflict in the |
inviteMessage: | ||
"Thank you for setting up your account on %{appUrl}. It will now be easier for you to start, save, and submit online applications for listings that appear on the site.", | ||
toCompleteAccountCreation: "To complete your account creation, please the link below:", | ||
"Welcome to the the Partners Portal on %{appUrl}. You will now be able to manage listings and applications that you are a part of from one centralized location.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small typo here the the
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed! :)
I get a 400 if I try to add a user who already exists which I think is probably okay, but it means if anyone has ever created an account on the public site (and maybe even parters?) then we can't add them as a user on the partners site with the same email. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM apart from the existing user piece and it sounds like that's a separate PR :)
@pbn4 , see Emily's comment above, can you fix the issue causing the 400 bad request to be thrown when a user who already exists in the system gets added. A more useful response is probably all that's needed, something like |
* Update field props * Update translation * Fix alert style * Update hook to support 'all' option * Create user add form * Make user model dob field optional * Update changelog * Drop NOT NULL constraint in user model DOB column * Fix code style issues with Prettier * Fix type issue * Fix typos in emails * Update translations * Fix typo Co-authored-by: Michal Plebanski <michalp@airnauts.com> Co-authored-by: Lint Action <lint-action@samuelmeuli.com>
Pull Request Template
Issue
Addresses #1653
Description
Adds the "Add user" drawer with form.
Type of change
How Can This Be Tested/Reviewed?
Use the newly created form to add a new user, then check if a user exists in the user table.
You can also test behavior if an email already exists (it should show a proper alert with an error message).
Checklist:
yarn generate:client
if I made backend changes