-
Notifications
You must be signed in to change notification settings - Fork 16
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
API Token use cases integration for account page #531
Conversation
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.
Good job @ChengShi-1 !
I've left some comments to improve things, you will need to make a refactor to avoid creating a new repository for api token info and instead use the user repository, that will need changes in several places.
Also try to add some more stories, after that I will check how they look, thanks!
src/users/infrastructure/repositories/ApiTokenInfoJSDataverseRepository.tsx
Outdated
Show resolved
Hide resolved
src/sections/account/api-token-section/ApiTokenSectionSkeleton.tsx
Outdated
Show resolved
Hide resolved
src/sections/account/api-token-section/CreateUserRepositoryFactory.tsx
Outdated
Show resolved
Hide resolved
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.
Approving! nice work!! 🚀
Hello @ChengShi-1, Had an observation on this ticket. Copy to Clipboard isn't working when I Recreate token. Can be reproduced in storybook and local. Steps to reproduce:
Here is where the issue lies, the recreated token isn't getting copied. |
Thank you, Omer! one variable was wrong in the copy function, I have a new commit to fix it. Also, it may need to be reviewed and approved again @g-saracca Thanks!! |
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.
Nice catch @ofahimIQSS, new changes approved 👍
Thanks for the fix @ChengShi-1 looks good now. Merging PR. |
What this PR does / why we need
This PR integrates the API Token account page UI with the new API token management use cases, ensuring seamless interaction between the UI and backend functionality.
Which issue(s) this PR closes:
Special notes for your reviewer:
waiting until this PR is reviewed.
Suggestions on how to test this:
Step 1: Run the Development Environment
Step 2: Test the feature
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: