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

fix: Unable to revoke shared folder permission and refactor the modify permissions modal. #2644

Merged

Conversation

agatha197
Copy link
Contributor

@agatha197 agatha197 commented Aug 19, 2024

resolves #2545
related PR: lablup/backend.ai#2731, lablup/backend.ai#2740

TL;DR

Fix a bug that changes the permissions of other people in the shared list.

What changed?

  • Updated the translation key for renaming a folder from 'data.folders.RenameAFolder' to 'data.explorer.RenameAFolder'.
  • Refactored the permission modification logic to improve type safety and readability.
  • Fixed a syntax error in the "Kick Out" list item by removing an extra quotation mark.

How to test?

  1. check out core branch to feat: Add new vfolder API to update sharing status backend.ai#2740, and cherry pick fix: vfolder id doesn't match in list_shared_vfolders backend.ai#2731
  2. Navigate to the storage list component.
  3. Attempt to rename a folder and verify the correct translation is displayed.
  4. Modify permissions for shared folders and ensure the changes are applied correctly.
  5. Check that the "Kick Out" option in the permission dropdown works as expected.
  6. Check the permission select has valid value just after accepting the invitation.

Why make this change?

These changes improve the user experience by ensuring correct translations are used and enhance the reliability of permission management. The refactored code also improves maintainability and reduces potential bugs related to type mismatches.

Screenshot

image image image

Checklist: (if applicable)

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

@github-actions github-actions bot added type:fix Fix features that are not working urgency:4 As soon as feasible, implementation is essential. urgency:blocker IT SHOULD BE RESOLVED BEFORE DISTRIBUTING area:lib Library and SDK related issue. size:M 30~100 LoC labels Aug 19, 2024
Copy link
Contributor Author

agatha197 commented Aug 19, 2024

@agatha197 agatha197 requested review from lizable and yomybaby August 19, 2024 16:03
This was referenced Aug 19, 2024
@yomybaby yomybaby force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch from 38435c3 to 01d329e Compare August 20, 2024 03:03
@yomybaby yomybaby changed the base branch from main to fix/share-button-disapear August 20, 2024 03:03
@agatha197 agatha197 changed the base branch from fix/share-button-disapear to graphite-base/2644 August 20, 2024 03:36
@agatha197 agatha197 force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch from 01d329e to 23a061c Compare August 20, 2024 03:37
@agatha197 agatha197 changed the base branch from graphite-base/2644 to main August 20, 2024 03:38
@agatha197 agatha197 force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch 3 times, most recently from 59d4070 to d84f743 Compare August 20, 2024 05:37
@agatha197 agatha197 requested a review from yomybaby August 20, 2024 05:41
@agatha197 agatha197 marked this pull request as draft August 20, 2024 07:00
@agatha197 agatha197 force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch 2 times, most recently from b6052e3 to e923a63 Compare August 20, 2024 14:57
@github-actions github-actions bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels Aug 20, 2024
Copy link

github-actions bot commented Aug 20, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
3.68% (-0.03% 🔻)
215/5849
🔴 Branches
4.04% (-0.01% 🔻)
159/3940
🔴 Functions
2.22% (-0.02% 🔻)
43/1941
🔴 Lines
3.56% (-0.03% 🔻)
204/5730
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🔴
... / InviteFolderPermissionSettingModal.tsx
0% 0% 0% 0%

Test suite run success

71 tests passing in 7 suites.

Report generated by 🧪jest coverage report action from 7a1f8a2

@agatha197 agatha197 force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch from e923a63 to 3a13f63 Compare August 20, 2024 15:10
@agatha197 agatha197 force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch from 3a13f63 to 4a653b2 Compare August 21, 2024 02:48
@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 Aug 21, 2024
@agatha197 agatha197 force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch 3 times, most recently from 7c4b7a7 to 8e79e39 Compare August 21, 2024 06:44
@agatha197 agatha197 marked this pull request as ready for review August 21, 2024 06:46
@agatha197 agatha197 changed the title fix: revoke shared folder permission doesn't work fix/refactor: fix revoking shared folder permission doesn't work Aug 21, 2024
@agatha197 agatha197 changed the title fix/refactor: fix revoking shared folder permission doesn't work fix: Unable to revoke shared folder permission and refactor the modify permissions modal. Aug 21, 2024
@agatha197 agatha197 force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch from 8e79e39 to cae264b Compare August 21, 2024 06:56
@agatha197 agatha197 requested a review from gahyuun August 21, 2024 06:59
@yomybaby yomybaby force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch from cae264b to 6e742c0 Compare August 23, 2024 02:57
@yomybaby yomybaby mentioned this pull request Aug 23, 2024
6 tasks
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 Aug 23, 2024

Merge activity

…y permissions modal. (#2644)

resolves #2545
related PR: lablup/backend.ai#2731, lablup/backend.ai#2740

### TL;DR

Fix a bug that changes the permissions of other people in the shared list.

### What changed?

- Updated the translation key for renaming a folder from 'data.folders.RenameAFolder' to 'data.explorer.RenameAFolder'.
- Refactored the permission modification logic to improve type safety and readability.
- Fixed a syntax error in the "Kick Out" list item by removing an extra quotation mark.

### How to test?
1. check out core branch to lablup/backend.ai#2740, and cherry pick lablup/backend.ai#2731
2. Navigate to the storage list component.
3. Attempt to rename a folder and verify the correct translation is displayed.
4. Modify permissions for shared folders and ensure the changes are applied correctly.
5. Check that the "Kick Out" option in the permission dropdown works as expected.
6. Check the permission select has valid value just after accepting the invitation.

### Why make this change?

These changes improve the user experience by ensuring correct translations are used and enhance the reliability of permission management. The refactored code also improves maintainability and reduces potential bugs related to type mismatches.

### Screenshot
<img width="542" alt="image" src="https://github.com/user-attachments/assets/17b8cf7e-fc7a-4975-ab9d-baebe479bbd0">
<img width="537" alt="image" src="https://github.com/user-attachments/assets/22e16773-322b-4686-83e3-b284aaf654db">
<img width="543" alt="image" src="https://github.com/user-attachments/assets/4aab0493-c31f-4178-8504-2d0f10293070">

---

**Checklist:** (if applicable)

- [x] Mention to the original issue
- [ ] Documentation
- [x] Minium required manager version: 23.09
- [x] Specific setting for review (eg., KB link, endpoint or how to setup): refer [#2545](#2545)
- [x] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after
@yomybaby yomybaby force-pushed the bugfix/revoke-shared-folder-permission-does-not-work branch from 6e742c0 to 7a1f8a2 Compare August 23, 2024 02:58
@graphite-app graphite-app bot merged commit 7a1f8a2 into main Aug 23, 2024
7 checks passed
@graphite-app graphite-app bot deleted the bugfix/revoke-shared-folder-permission-does-not-work branch August 23, 2024 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:i18n Localization area:lib Library and SDK related issue. area:ux UI / UX issue. size:XL 500~ LoC type:fix Fix features that are not working urgency:blocker IT SHOULD BE RESOLVED BEFORE DISTRIBUTING urgency:4 As soon as feasible, implementation is essential.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revoke shared folder permission doesn't working properly
3 participants