Skip to content
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

feat: user list using user_nodes query #2934

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Conversation

yomybaby
Copy link
Member

@yomybaby yomybaby commented Dec 11, 2024

Changes:
Migrates the user management interface from Web Components to React, introducing a new UserNodeList component that provides enhanced user management capabilities. The new implementation includes:

  • Active/Inactive user filtering
  • Advanced property filtering for email, username, role, and description
  • Table-based user listing with sorting and pagination
  • User information viewing and settings modification
  • User deletion functionality with confirmation dialog
  • Refresh capabilities for real-time updates

Rationale:
This change modernizes the user management interface by leveraging React's component model and state management, while maintaining feature parity with the previous implementation. The migration removes duplicate code and consolidates user management logic into a single, maintainable component.

Copy link

graphite-app bot commented Dec 11, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Member Author

yomybaby commented Dec 11, 2024


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

@ironAiken2 ironAiken2 left a comment

Choose a reason for hiding this comment

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

Made some code modifications for the user-generated modal. LGTM now!

Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

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

  1. I can't find the description even though the filtering supports the description.
  2. In the role column, the superadmin is described as 'superadmin' but it is described as 'super admin' in filtering. Let's unify the expressions.
  3. Please remove the USER | CREDENTIALS tab of the Credentials tab.

image.png

image.png

react/src/pages/UserCredentialsPage.tsx Show resolved Hide resolved
react/src/components/UserNodeList.tsx Show resolved Hide resolved
react/src/components/UserNodeList.tsx Outdated Show resolved Hide resolved
@yomybaby yomybaby force-pushed the feat/user-list-page branch from 59b79b0 to 682b18e Compare December 12, 2024 09:05
@yomybaby yomybaby force-pushed the feature/user-node-list branch from 261b005 to 47f2a54 Compare December 12, 2024 09:05
Copy link
Member Author

yomybaby commented Dec 12, 2024

I'll check 1 & 2. The 3 is handled in #2758

@yomybaby yomybaby force-pushed the feat/user-list-page branch from 682b18e to 0559656 Compare December 13, 2024 03:09
@yomybaby yomybaby force-pushed the feature/user-node-list branch from 47f2a54 to 9c14f4e Compare December 13, 2024 03:09
@yomybaby yomybaby requested a review from agatha197 December 13, 2024 03:10
Copy link
Member Author

Please check again. I resolve 1 and 2. Thank you

Copy link
Member Author

I'll remove the USER | CREDENTIALS tab you mentioned so that the current PR can be merged first.

Copy link
Contributor

I think 2 is not resolved yet.

Copy link
Member Author

Sorry I don't know why it is reverted. I'll push again soon.

Copy link
Contributor

And please check the unresolved threads too :)

@yomybaby yomybaby force-pushed the feature/user-node-list branch from 9c14f4e to dbeccac Compare December 13, 2024 03:27
@yomybaby yomybaby marked this pull request as draft December 13, 2024 05:55
@yomybaby yomybaby force-pushed the feature/user-node-list branch from dbeccac to baa2e6e Compare December 13, 2024 09:36
@github-actions github-actions bot added area:ux UI / UX issue. area:i18n Localization labels Dec 13, 2024
@yomybaby yomybaby marked this pull request as ready for review December 13, 2024 09:36
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

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

I suggested color changes like other components.

react/src/components/UserNodeList.tsx Outdated Show resolved Hide resolved
react/src/components/UserNodeList.tsx Outdated Show resolved Hide resolved
react/src/components/UserNodeList.tsx Outdated Show resolved Hide resolved
react/src/components/UserNodeList.tsx Outdated Show resolved Hide resolved
react/src/components/UserNodeList.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ironAiken2 ironAiken2 force-pushed the feature/user-node-list branch from 66d86df to a29c647 Compare December 16, 2024 02:03
Copy link

graphite-app bot commented Dec 16, 2024

Merge activity

**Changes:**
Migrates the user management interface from Web Components to React, introducing a new `UserNodeList` component that provides enhanced user management capabilities. The new implementation includes:

- Active/Inactive user filtering
- Advanced property filtering for email, username, role, and description
- Table-based user listing with sorting and pagination
- User information viewing and settings modification
- User deletion functionality with confirmation dialog
- Refresh capabilities for real-time updates

**Rationale:**
This change modernizes the user management interface by leveraging React's component model and state management, while maintaining feature parity with the previous implementation. The migration removes duplicate code and consolidates user management logic into a single, maintainable component.
@yomybaby yomybaby force-pushed the feat/user-list-page branch from de17251 to 60dedfb Compare December 16, 2024 02:07
@yomybaby yomybaby force-pushed the feature/user-node-list branch from a29c647 to cc2f9a1 Compare December 16, 2024 02:08
Base automatically changed from feat/user-list-page to main December 16, 2024 02:10
@graphite-app graphite-app bot merged commit cc2f9a1 into main Dec 16, 2024
4 checks passed
@graphite-app graphite-app bot deleted the feature/user-node-list branch December 16, 2024 02:11
@yomybaby yomybaby linked an issue Dec 16, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:i18n Localization area:ux UI / UX issue. size:XL 500~ LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate User List to React React migration: User List Page for large datasets
3 participants