Skip to content

User Management Page#771

Merged
KCarretto merged 1 commit intomainfrom
users
Feb 1, 2025
Merged

User Management Page#771
KCarretto merged 1 commit intomainfrom
users

Conversation

@jabbate19
Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds a User Management page. Instead of needing to use the GraphQL Playground, you can approve/promote users in the website.

Which issue(s) this PR fixes:

N/A

@jabbate19
Copy link
Copy Markdown
Collaborator Author

image

@jabbate19 jabbate19 requested a review from cmp5987 May 5, 2024 23:10
Copy link
Copy Markdown
Collaborator

@cmp5987 cmp5987 left a comment

Choose a reason for hiding this comment

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

Just a few comments based on what you currently have written.

@jabbate19
Copy link
Copy Markdown
Collaborator Author

Updated photos

Admin UI

image

If a user attempts to access /admin while not admin

image

Non-Admin Users still do not see Admin in navbar

image

@jabbate19 jabbate19 requested a review from cmp5987 December 29, 2024 00:27
const [sidebarOpen, setSidebarOpen] = useState(false)
const {data, isLoading, error} = useContext(AuthorizationContext);

if(isLoading){
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be good to pull this logic outside of PageWrapper so the PageWrapper is only accessed after AuthorizationContext is loaded.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I kept this here because it needs to be used by AccessGate and Navigation. Is there an easy way to use it in both places without using context twice?

}
`;

const handleError = (error: NetworkError | GraphQLErrors) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When are these errors displayed, I saw the submit used by the table.

Additionally, How are they reset to false for if a user tries to re-trigger the action on a failure?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Wdym by this?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When an error occurs as a result of a user demoting/promoting a user. We should be displaying these errors to the user. This could occur via Toast or other banner message. It should also be dismissible.

<span>This portal is for managing users allowed to access Tavern. You can also promote users to Administrator, who will be able to access this page.</span>
</div>
</div>
<Tab.Group>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any plans for what this will expand to include, right now we have only 1 tab so its a bit odd to include a tab/tab panel.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Since we are naming it an "Admin" panel, thought it would be good to add tabs if other admin features need to get added later

@jabbate19
Copy link
Copy Markdown
Collaborator Author

jabbate19 commented Jan 25, 2025

image image

New red button styles

image

Fixed text location

@jabbate19
Copy link
Copy Markdown
Collaborator Author

image

Without tabs

@jabbate19
Copy link
Copy Markdown
Collaborator Author

Solves #838

@KCarretto
Copy link
Copy Markdown
Collaborator

Will leave this review to @cmp5987 as well

@jabbate19 jabbate19 added this to the v0.2.0 milestone Jan 26, 2025
@cmp5987
Copy link
Copy Markdown
Collaborator

cmp5987 commented Jan 27, 2025

Nits:

  • Mind adding 'buttonVariant="ghost" to the action buttons.
  • Add feedback handling for success or error cases when users click 'actions. Think Toast or Inline banners that are dismissible.
  • Don't forget to squash before merging but this looks almost ready.
  • Add loading states to buttons and prevent interaction when a user's action is being processed. In scenarios where backend is slow we don't really want to have things double clicked or users to not understand that their action is getting processed.

Task Output vs Task Finished
I think the task output vs task finished really depends. The ideal for the statistic would be tasks with meaningful information available. Unfortunately, 'finished' doesn't mean success/not failed. Output can also be filled with errors depending on the task V_V. Let's stick with whichever you prefer and iterate based on 2025 ISTS feedback.

@jabbate19
Copy link
Copy Markdown
Collaborator Author

Add feedback handling for success or error cases when users click 'actions. Think Toast or Inline banners that are dismissible.

Is there an example in the codebase I can use?

@jabbate19
Copy link
Copy Markdown
Collaborator Author

Mind adding 'buttonVariant="ghost" to the action buttons.

image

Your call, but imo I think this looks off unless we add a background color to the button or outline

@cmp5987
Copy link
Copy Markdown
Collaborator

cmp5987 commented Jan 27, 2025

Add feedback handling for success or error cases when users click 'actions. Think Toast or Inline banners that are dismissible.

Is there an example in the codebase I can use?

For an Inline banner example we have 'AlertError' component that was used in our Tome repo setup.
https://github.com/spellshift/realm/blob/main/tavern/internal/www/src/pages/tomes/components/StepCreateRepository.tsx#L32

This is a simple guide on using Chakra-UI Toasts for user feedback.
https://medium.com/@santhoshs19032003/easy-toast-messages-in-react-with-chakra-ui-a-beginners-guide-8833807d9e12

@jabbate19
Copy link
Copy Markdown
Collaborator Author

  • Changed button to outline, imo looks better with the outline contract than white on white
  • Added toasts
image image image image

cmp5987
cmp5987 previously approved these changes Feb 1, 2025
Copy link
Copy Markdown
Collaborator

@cmp5987 cmp5987 left a comment

Choose a reason for hiding this comment

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

Awesome work,
Don't forget to squash, rebase and rebuild the UI.

~ I think we can squeeze this in for next weeks release.

@KCarretto KCarretto merged commit 12a3bf4 into main Feb 1, 2025
@KCarretto KCarretto deleted the users branch February 1, 2025 22:28
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.

3 participants