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: keypair info/setting modal in credential page #2940

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

ironAiken2
Copy link
Contributor

@ironAiken2 ironAiken2 commented Dec 11, 2024

This PR resolves #2937 issue

Changes:

  • Since UserSettingPage is using the KeypairInfoModal, I renamed the component to MyKeypairInfoModal.
  • Added KeypairInfoModal and KeypairSettingModal for Credential Page.
  • The KeypairSettingModal is used to create or modify a key pair.
  • ordering/filtering keypair_list query by user_id is out of scope for this PR because it does not yet provide ordering, filtering on user_id.

How to test:

  • Access the user credentials & policy page with an account with admin or higher permissions.
  • Verify that the KeypairInfoModal shows the correct information for the selected keypair.
  • Verify that the KeypairSettingModal allows you to create and modify keypair information.

Checklist: (if applicable)

  • Mention to the original issue
  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

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
Contributor Author

ironAiken2 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.

@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from 485ce28 to c2f4135 Compare December 11, 2024 10:00
@github-actions github-actions bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels Dec 11, 2024
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from c2f4135 to b330220 Compare December 12, 2024 07:04
@github-actions github-actions bot added area:ux UI / UX issue. area:i18n Localization size:XL 500~ LoC and removed size:L 100~500 LoC labels Dec 12, 2024
@ironAiken2 ironAiken2 requested review from agatha197 and yomybaby and removed request for agatha197 December 12, 2024 07:12
@ironAiken2 ironAiken2 marked this pull request as ready for review December 12, 2024 07:12
@ironAiken2 ironAiken2 force-pushed the feat/user-credential-list branch from b19dfdb to 7cc2e99 Compare December 12, 2024 08:06
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from b330220 to 74c2cf5 Compare December 12, 2024 08:06
@ironAiken2 ironAiken2 requested a review from agatha197 December 12, 2024 08:19
@yomybaby yomybaby force-pushed the feat/user-credential-list branch from 7cc2e99 to 8106932 Compare December 12, 2024 09:05
@ironAiken2 ironAiken2 marked this pull request as draft December 12, 2024 09:13
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from 74c2cf5 to 39cfc44 Compare December 12, 2024 09:29
@yomybaby yomybaby force-pushed the feat/user-credential-list branch from 8106932 to 73d7b5f Compare December 13, 2024 03:10
@yomybaby yomybaby force-pushed the feat/keypair-create-and-modify-modal branch from 39cfc44 to 7d3792f Compare December 13, 2024 03:10
@ironAiken2 ironAiken2 force-pushed the feat/user-credential-list branch from 73d7b5f to 665742b Compare December 16, 2024 01:38
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from 092b2a2 to 986255b Compare December 18, 2024 04:39
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.

.

react/src/components/KeypairSettingModal.tsx Show resolved Hide resolved
react/src/components/KeypairSettingModal.tsx Show resolved Hide resolved
react/src/components/MyKeypairInfoModal.tsx Outdated Show resolved Hide resolved
react/src/components/MyKeypairInfoModal.tsx Outdated Show resolved Hide resolved
@ironAiken2 ironAiken2 force-pushed the feat/user-credential-list branch from a35f9a8 to 8b8138c Compare December 18, 2024 06:29
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from 986255b to ac98042 Compare December 18, 2024 06:29
@ironAiken2 ironAiken2 force-pushed the feat/user-credential-list branch from 8b8138c to ad3c66f Compare December 18, 2024 06:31
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from ac98042 to 2479854 Compare December 18, 2024 06:31
@ironAiken2 ironAiken2 force-pushed the feat/user-credential-list branch from ad3c66f to 6405a37 Compare December 18, 2024 07:22
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from 2479854 to 9d61c02 Compare December 18, 2024 07:22
@ironAiken2 ironAiken2 requested a review from agatha197 December 18, 2024 07:23
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.

image.png

Modify modal doesn't close automatically after updating.

@ironAiken2 ironAiken2 force-pushed the feat/user-credential-list branch from 6405a37 to 642664f Compare December 18, 2024 08:42
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch 2 times, most recently from 53a7626 to 2be9bf0 Compare December 18, 2024 08:49
@ironAiken2 ironAiken2 requested a review from agatha197 December 18, 2024 08:49
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 feat/keypair-create-and-modify-modal branch from 2be9bf0 to 6ac31c3 Compare December 23, 2024 02:21
@ironAiken2 ironAiken2 requested a review from agatha197 December 23, 2024 02:21
@ironAiken2 ironAiken2 force-pushed the feat/user-credential-list branch from 642664f to 5490140 Compare December 23, 2024 06:43
@ironAiken2 ironAiken2 force-pushed the feat/keypair-create-and-modify-modal branch from 6ac31c3 to 8d3b258 Compare December 23, 2024 06:43
@yomybaby yomybaby force-pushed the feat/keypair-create-and-modify-modal branch from 8d3b258 to 1f4dc18 Compare December 23, 2024 08:06
Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

graphite-app bot commented Dec 23, 2024

Merge activity

<!--
Please precisely, concisely, and concretely describe what this PR changes, the rationale behind codes,
and how it affects the users and other developers.
-->

### This PR resolves [#2937](#2937) issue

**Changes:**
- Since `UserSettingPage` is using the KeypairInfoModal, I renamed the component to MyKeypairInfoModal.
- Added `KeypairInfoModal` and `KeypairSettingModal` for `Credential Page`.
- The `KeypairSettingModal` is used to create or modify a key pair.
- ordering/filtering keypair_list query by user_id is out of scope for this PR because it does not yet provide ordering, filtering  on user_id.

**How to test:**
- Access the user credentials & policy page with an account with admin or higher permissions.
- Verify that the `KeypairInfoModal` shows the correct information for the selected keypair.
- Verify that the `KeypairSettingModal` allows you to create and modify keypair information.

**Checklist:** (if applicable)

- [ ] Mention to the original issue
- [ ] Documentation
- [ ] Minium required manager version
- [ ] Specific setting for review (eg., KB link, endpoint or how to setup)
- [ ] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after
@yomybaby yomybaby force-pushed the feat/user-credential-list branch from 5490140 to e07e4bc Compare December 23, 2024 08:09
@yomybaby yomybaby force-pushed the feat/keypair-create-and-modify-modal branch from 1f4dc18 to ec8d643 Compare December 23, 2024 08:09
Base automatically changed from feat/user-credential-list to main December 23, 2024 08:11
@graphite-app graphite-app bot merged commit ec8d643 into main Dec 23, 2024
4 checks passed
@graphite-app graphite-app bot deleted the feat/keypair-create-and-modify-modal branch December 23, 2024 08:12
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 credential list to React
3 participants