[MS] Batch workspace sharing #8900
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Max-7
requested changes
Nov 12, 2024
mmmarcos
reviewed
Nov 13, 2024
Ironicbay
force-pushed
the
ms-batch-workspace-sharing
branch
3 times, most recently
from
3646795
to
c0d0261
Compare
Max-7
requested changes
Nov 18, 2024
| } | ||
| } | ||
|
|
||
| if (!changesMade) { |
There was a problem hiding this comment.
Error handling is insufficient here.
You can test it in web mode by updating the mocks for shareWorkspace and add (as an example)
return { ok: false, error: { tag: ClientShareWorkspaceErrorTag.Offline, error: 'Offline' } };
The error I get is Selected members were already Contributor., which is false. I'm not even sure that this error is possible, if you try to assign the same role to a user, I think it'll just work.
Handling multiple errors is always a pain. The best thing to do imo is to count the number of successes, the number of errors, and store the first error (if any). You can then compare it to the number of operations:
successesCount === selectedUsers.value.lengthmeans all gooderrorsCount === selectedUsers.value.lengthmeans all failed, probably a very general case like being offline, being revoked, having insufficient permission (you can check using the error you stored and assume it's the same for all)- some errors is a complicated case, but a generic error like "some failed" is probably enough
Ironicbay
force-pushed
the
ms-batch-workspace-sharing
branch
from
c0d0261
to
521002b
Compare
Ironicbay
force-pushed
the
ms-batch-workspace-sharing
branch
from
521002b
to
69aa5a7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #8326
Enregistrement.de.l.ecran.2024-11-18.a.10.57.40.mov