-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Uncomment role checks in dbAuth's auth.js template #4476
Conversation
@dthyresson did you mean to comment these when you were resolving the TS errors over here 20cb526? |
I don’t think so, they should be there. But will check for typing. |
Oh, I see:
Maybe with the new Tutorial examples, roles will exist? @cannikin |
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.
For DbAuth, since the User model doesn't yet have roles, the role check is commented out and the developer is informed to re-enabled them as needed
Maybe with the new Tutorial examples, roles will exist? @cannikin
That's why they were commented out.
Well, we won't have roles in part 1, we add them in part 2. But, any calls to |
How's this looking, could we merge? I heard a rumor @thedavidprice was thinking of doing a release and it'd be nice if this was in it, otherwise people going through Part 2 of the tutorial are gonna be very confused! |
I need to check that the types don’t give a warning. And see if it did if adding empty roles to currentUser (even empty) is ok. |
So the real role checking only kicks in if you make a call to But, this is now the same as the non-dbAuth |
Ok. Approved |
On latest test run @thedavidprice
😬 |
I went ahead and merged this, but after a couple runs the Windows Node 16 test still fails with out-of-memory errors. |
…test * 'main' of github.com:redwoodjs/redwood: (23 commits) Netlify client getToken fix when GoTrue client refreshes JWT (redwoodjs#4539) Update dependency @supabase/supabase-js to v1.30.4 (redwoodjs#4536) Envelop: Don't use useImmediateIntrospection as it causes auth bug (redwoodjs#4538) Update dependency react-hook-form to v7.27.1 (redwoodjs#4521) try increasing timeout for flaky test (redwoodjs#4526) Update dependency stacktracey to v2.1.8 (redwoodjs#4519) Update dependency msw to v0.38.1 (redwoodjs#4525) Update dependency eslint-config-prettier to v8.4.0 (redwoodjs#4522) Provide a Revised Runtime Error Page (redwoodjs#4167) update yarn.lock v0.46.0 Update dependency esbuild to v0.14.23 (redwoodjs#4518) Fix Storybook build args (redwoodjs#4455) Update dependency react-helmet-async to v1.2.3 (redwoodjs#4502) Bump url-parse in /__fixtures__/example-todo-main-with-errors (redwoodjs#4511) Bump url-parse from 1.5.1 to 1.5.7 in /__fixtures__/example-todo-main (redwoodjs#4512) Update dependency fastify to v3.27.2 (redwoodjs#4516) Uncomment role checks (redwoodjs#4476) Update dependency zx to v5.1.0 (redwoodjs#4505) Update dependency firebase to v9.6.7 (redwoodjs#4514) ...
Not sure why I would have commented these out, I thought I just copied the standard
auth.js
template and make a couple of changes forgetCurrentUser()
unique to dbAuth.