Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
WalkthroughThis change adds an 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| t, | ||
| type | ||
| }) | ||
| const setType = async newType => { |
There was a problem hiding this comment.
I think if (newType === type) part can be optimized (we hideMenu anyway, the only condition is to trigger the try/catch or no).
Besides, it seems to me we have a part of the action here, and in the makeActions part 🤔 it's not very clear. Do you think we can optimize a bit to gather action logic in only one place?
There was a problem hiding this comment.
I refactored it a bit to be simplier. What do you mean by we have a part of the action here, and in the makeActions part 🤔 ?
There was a problem hiding this comment.
I understand. Currently setReadOnlySharedPermission and setReadWriteSharedPermission are generic and are used to set permissions for members, groups and links. They only carry the UI of the action as you said.
| * @param {number} group - The index of the group in the sharing groups array | ||
| * @param {string} newType - 'one-way' for read-only, 'two-way' for read-write | ||
| */ | ||
| updateSharingGroupType = async (sharingId, groupIndex, newType) => { |
There was a problem hiding this comment.
is it ok to disptach something... and then if it doesn't work, we dispatch a rollback? 🤔 sounds strange...
There was a problem hiding this comment.
It is usual optimistic update? I took the logic from the update member logic for the optimistic update here.
Summary by CodeRabbit