Skip to content

feat: Create user page #1197

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

Merged
merged 49 commits into from
Apr 28, 2022
Merged

feat: Create user page #1197

merged 49 commits into from
Apr 28, 2022

Conversation

presleyp
Copy link
Contributor

@presleyp presleyp commented Apr 27, 2022

Blocked by #1184
Fixes #734

  • Adds New User button to the Users page
  • Adds Create User page
  • XState determines if it's a field error or a generic error
  • I'm pretending users come with org ids in preparation for Bruno's PR
  • I use a selector to get the org id out of the authXService without making the CreateUserPage re-render for every change to authXService
  • Note that the CreateUserForm stories have a flash of a validation error message. That appears to be a Storybook problem where it thinks the field is touched momentarily, maybe a lag from looking at a related story.
  • Note that in order to see the field error message under test in the FormErrors story, you have to touch the email field.
  • I gave up on testing for redirect after success because it was taking too long. We should e2e test redirects after the deadline.
  • I got rid of creation mode in the XService because it expanded the surface area for bugs by requiring routing and the XService to stay in sync. So I'm embedding navigate in an action like @BrunoQuaresma suggested. I prefer this over embedding it in context because in the latter solution, the XService has to know our routes.
  • I had to resort to a jest mock for the generic error because other ways of mocking a non-response error either caused a crash or caused MSW to fall back to its happy path handler.

@codecov
Copy link

codecov bot commented Apr 27, 2022

Codecov Report

Merging #1197 (10c765c) into main (816441e) will increase coverage by 0.29%.
The diff coverage is 91.54%.

@@            Coverage Diff             @@
##             main    #1197      +/-   ##
==========================================
+ Coverage   66.00%   66.30%   +0.29%     
==========================================
  Files         265      268       +3     
  Lines       16751    16817      +66     
  Branches      157      162       +5     
==========================================
+ Hits        11057    11151      +94     
+ Misses       4533     4508      -25     
+ Partials     1161     1158       -3     
Flag Coverage Δ
unittest-go-macos-latest 53.44% <ø> (-0.01%) ⬇️
unittest-go-postgres- 65.44% <ø> (+0.19%) ⬆️
unittest-go-ubuntu-latest 55.95% <ø> (-0.08%) ⬇️
unittest-go-windows-2022 53.06% <ø> (ø)
unittest-js 68.85% <91.54%> (+2.35%) ⬆️
Impacted Files Coverage Δ
site/src/AppRouter.tsx 0.00% <0.00%> (ø)
site/src/api/errors.ts 85.71% <ø> (ø)
site/src/testHelpers/entities.ts 100.00% <ø> (ø)
site/src/api/index.ts 65.30% <66.66%> (+0.08%) ⬆️
site/src/xServices/users/usersXService.ts 81.25% <77.77%> (-4.47%) ⬇️
site/src/xServices/StateContext.tsx 92.30% <80.00%> (-7.70%) ⬇️
site/src/pages/UsersPage/UsersPage.tsx 82.35% <87.50%> (-0.99%) ⬇️
...e/src/components/CreateUserForm/CreateUserForm.tsx 100.00% <100.00%> (ø)
site/src/components/FormFooter/FormFooter.tsx 91.66% <100.00%> (+91.66%) ⬆️
.../pages/UsersPage/CreateUserPage/CreateUserPage.tsx 100.00% <100.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 816441e...10c765c. Read the comment docs.

@presleyp presleyp marked this pull request as ready for review April 28, 2022 15:36
@presleyp presleyp requested a review from a team as a code owner April 28, 2022 15:36

export const CreateUserPage: React.FC = () => {
const xServices = useContext(XServiceContext)
const myOrgId = useSelector(xServices.authXService, selectOrgId)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing this instead of getting authState using useActor means that this component will only re-render in response to changes in the org id, not all authXService changes. I haven't bothered with this approach in other places yet because so far it's been components that have a lot to do with the XService they're accessing and there probably won't be many irrelevant updates, but we should do this for cases where we only care about a little piece of some global state.

email: "",
password: "",
username: "",
organization_id: myOrgId,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just hard-coded the org id in here; in EE we'll need a drop-down field for it.

target: "idle",
actions: ["assignCreateUserError"],
},
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works like a switch statement - if the cond is true on the first case, do that, otherwise try the next case.

@presleyp presleyp self-assigned this Apr 28, 2022
Copy link
Contributor

@greyscaled greyscaled left a comment

Choose a reason for hiding this comment

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

Nicely done 🎉

Comment on lines +76 to +86
onError: [
{
target: "idle",
cond: "isFormError",
actions: ["assignCreateUserFormErrors"],
},
{
target: "idle",
actions: ["assignCreateUserError"],
},
],
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Very nice!

Co-authored-by: G r e y <grey@coder.com>
@presleyp presleyp merged commit c16f105 into main Apr 28, 2022
@presleyp presleyp deleted the create-user/presleyp/734 branch April 28, 2022 20:32
@misskniss misskniss added this to the V2 Beta milestone May 15, 2022
kylecarbs pushed a commit that referenced this pull request Jun 10, 2022
* Add button and route

* Hook up api

* Lint

* Add basic form

* Get users on page mount

* Make cancel work

* Creating -> idle bc users page refetches

* Import as TypesGen

* Handle api errors

* Lint

* Add handler

* Add FormFooter

* Add FullPageForm

* Lint

* Better form, error, stories

bug in formErrors story

* Make detail optional

* Use Language

* Remove detail prop

* Add back autoFocus

* Remove displayError, use displaySuccess

* Lint, export Language

* Tests - wip

* Fix cancel tests

* Switch back to mock

* Add navigate to xservice

Doesn't work in test

* Move error type predicate to xservice

* Lint

* Switch to using creation mode in XState

still problems in tests

* Lint

* Lint

* Lint

* Revert "Switch to using creation mode in XState"

This reverts commit cf8442f.

* Give XService a navigate action

* Add missing validation messages

* Fix XState warning

* Fix tests

IRL is broken bc I need to send org id

* Pretend user has org id and make it work

* Format

* Lint

* Switch to org ids array

* Skip lines between tests

Co-authored-by: G r e y <grey@coder.com>

* Punctuate notification messages

Co-authored-by: G r e y <grey@coder.com>
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.

Frontend User Create
4 participants