chris/feat(#381): Implement User Role Access #458
Conversation
|
Gridiron Survivor Application
Project name: Gridiron Survivor Application
Only deployments on the production branch are activated automatically. If you'd like to activate this deployment, navigate to your deployments. Learn more about Appwrite Function deployments.
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
@chris-nowicki Can you show me an example of this working in action?
context/AuthContextProvider.tsx
Outdated
| @@ -40,6 +41,7 @@ export const AuthContextProvider = ({ | |||
| children: React.ReactNode; | |||
| }): JSX.Element => { | |||
| const [isSignedIn, setIsSignedIn] = useState<boolean>(false); | |||
| const [isSuperAdmin, setIsSuperAdmin] = useState<boolean>(false); | |||
There was a problem hiding this comment.
I don't believe this needs to be a useState. It's going to cause a rehydration of the entire application that may be alreayd handled from other auth functions.
lib/adminRoutes.ts
Outdated
| // Copyright (c) Gridiron Survivor. | ||
| // Licensed under the MIT License. | ||
|
|
||
| export const adminRoutes: string[] = [ |
There was a problem hiding this comment.
Why can't this be a wildcard? admin/*? Then, we don't need to track every page.
context/AuthContextProvider.tsx
Outdated
| useMemo(() => { | ||
| if (isSignedIn) { | ||
| if (adminRoutes.includes(pathname)) { | ||
| !isSuperAdmin && router.push('/league/all'); |
There was a problem hiding this comment.
The homepage should redirect them to league/all already. Once this is set to the real homepage, we won't need to modify this again.
| !isSuperAdmin && router.push('/league/all'); | |
| !isSuperAdmin && router.push('/'); |
There was a problem hiding this comment.
@shashilo tried this out and nope I am not redirected automatically.
chris-nowicki
dismissed stale reviews from choir241 and ryandotfurrer
via
7bbd79a
chris-nowicki
changed the title
| if (pathname.startsWith('/admin')) { | ||
| !user.labels.includes('admin') && router.push('/'); | ||
| } | ||
| }, [user, pathname]); | ||
|
|
There was a problem hiding this comment.
Should we write out other edge cases such as
- if user.labels is undefined or not yet populated
- if the logic has multiple stated updates (i.e. user or pathname changing frequently causing multiple redirects (to prevent adding to the history stack as a user can go forward and backward)
What are your thoughts?
There was a problem hiding this comment.
- The way the useEffect is written user will be populated if it hits this point
if (user.id === '' || user.email === '') {
getUser();
return;
}
```
2. This is a question I'd like to defer to @shashilo ... right now this is the only way I was able to get it to work due when we change pathname it loads the data again (using cached data) but we still need to wait for it to load.
There was a problem hiding this comment.
@vmaineng brings up a great point. If this is a case in our app, we should write it out by making a unit test for it.
There was a problem hiding this comment.
✅ pathname helper function added to utils with a unit test to test pathname changing.
| if (pathname.startsWith('/admin')) { | ||
| !user.labels.includes('admin') && router.push('/'); | ||
| } | ||
| }, [user, pathname]); | ||
|
|
There was a problem hiding this comment.
@vmaineng brings up a great point. If this is a case in our app, we should write it out by making a unit test for it.
| @@ -89,9 +93,8 @@ export const AuthContextProvider = ({ | |||
|
|
|||
| try { | |||
| const user = await account.get(); | |||
| const userData: IUser = await getCurrentUser(user.$id); | |||
| updateUser(userData.id, userData.email, userData.leagues); | |||
| return userData; | |||
There was a problem hiding this comment.
Why is the return removed?
There was a problem hiding this comment.
✅ it is back in :). Not sure why it was removed.
pnpm-lock.yaml
Outdated
There was a problem hiding this comment.
Why is the lock file updated?
fixes #381
Added access for SUPER ADMINS only. This has to be a field added in the database manually under the user > labels data field. This should only apply to GIS Developers.
VIDEO
CleanShot.2024-10-16.at.23.58.17.mp4