feat: expose the write permission on shared modifications by flomillot · Pull Request #1261 · gridsuite/commons-ui

flomillot · 2026-07-27T13:36:23Z

PR Summary

In GridStudy, a 'shared modification' is a MODIFICATION_REFERENCE carrying a referenceId, which is also the elementUuid of the composite in directory-server. use-shared-modifications-permissions resolves the write
permission of every shared modification in a single getAccessibleElements call, caches the answers and
invalidates them on UPDATE_DIRECTORY notifications. A permission that hasn't resolved yet reads as denied, and a resolution that failed is asked for again on the next trigger.

Every row unfolded from a reference carries ancestorSharedModificationUuids, the chain of the shared
modifications it sits inside: it is locked as soon as ONE of them can't be written into. Two rules build on that chain:

isInLockedSharedModification — what only the content of a shared modification forbids: opening a form, and
the toolbar's delete, cut, paste and assemble.
isModificationEditLocked — the above, plus the reference row itself, for what targets the shared
modification: renaming, saving a description, and toggling the applicability globally or per root network.

Actions on the shared modification taken as a whole stay allowed: moving it, deleting it, assembling it.

Drag and drop reasons on the same chain: nothing can be taken out of a shared modification the user can't
write into, moved around inside it, nor dropped into it, while the shared modification taken as a whole stays
movable.

Requires gridsuite/network-modification-server#858: without referenceId in the metadata nothing locks, so that
one has to be merged first.

Requires gridsuite/explore-server#209 and gridsuite/directory-server#267, which provide the batch permission
endpoint this branch calls. hasElementPermission is rebuilt on top of it and keeps its signature, so its
other callers are unaffected.

Limitation left as TODO in the code: a permission granted through a group is not invalidated —
user-admin-server emits no notification on group composition change.

A shared modification references a composite whose uuid is also the uuid of the shared element in the directory, so the permission of the current user can be resolved from it. useSharedModificationsPermissions resolves that permission for the shared modifications of a node, and the table derives from it everything nested inside the ones the user can't write into. Both are published through the table meta, so the cells deny the actions targeting them: applicability, renaming, description saving and drag and drop. Acting on a shared modification as a whole stays allowed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

coderabbitai · 2026-07-27T13:38:51Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 68c4f69d-de42-427c-b66b-fc0a6e6fba60

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…-permissions' into florent/poc-shared-modifications-permissions

…difications-permissions Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com> # Conflicts: # src/features/network-modification-table/renderers/cell-renderers.tsx # src/features/network-modification-table/utils.ts

Dragging anything in or out of a referenced modification is already forbidden, whatever the permissions, by the temporary guard of GRD-4785. The permission check on top of it was dead code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

…difications-permissions Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

The trigger only fires on a composite that has just been assembled, which can never be a shared modification nor sit inside one, so the permission check was dead. Clicking the label stays blocked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

The hook needed the UPDATE_DIRECTORY type, which only existed in gridstudy. Moved to commons-ui next to NotificationsUrlKeys so both sides read the same constant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

gridexplore uses these types as types, not only as values, hence the const assertion and the derived union. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

The sets hold the uuids of the rows sitting in the study, which are reference modifications, while the shared modification is the one element in the directory they all point at. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

Without the write permission and without an existing description, the button only opened a dialog whose validation was denied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

An enum is both a value and a type, which is what the consumers need, so the const assertion and the derived union go away. It also matches NotificationsUrlKeys right above it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

ghazwarhili · 2026-08-19T14:44:22Z

    const defaultCompositeName: string = useMemo(() => intl.formatMessage({ id: 'CompositeModification' }), [intl]);

    // triggers composite name editing from outside the component
+    // i.e., when a composite is being created


Unnecessary comment !
revert

I don't think it is. The only purpose of this use effect is when we create a new composite which was not mentioned.

ghazwarhili · 2026-08-19T14:55:10Z

        setOpenDescModificationDialog(true);
    }, []);

+    // Nothing to read and nothing to write


Unnecessary comment !
remove it

Returning null from a component is not something that is common and should be explained. At least it's legit.

I update the comments to make it clearer.

ghazwarhili · 2026-08-20T08:16:45Z

+    DELETE_DIRECTORY = 'DELETE_DIRECTORY',
+    ADD_DIRECTORY = 'ADD_DIRECTORY',
+    UPDATE_DIRECTORY = 'UPDATE_DIRECTORY',
+}


This isn't the right file for this enum !

I agree the file does not fit totally, but I didn't find an accurate file. Where is it ?

…nd save is disabled Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

ghazwarhili · 2026-08-20T15:02:14Z

    messageType: string;
    messageValues: string;
+    // MODIFICATION_REFERENCE only: uuid of the referenced composite modification
+    referenceId?: UUID;


referenceId is already declared in ReferenceModificationInfos could that cause conflicts?

Nope because it's the same field in the back. However ReferenceModificationInfos is bad typed because it should not be optional.

However, I added this line before the one you mention exists, so I can remove the other one.

ghazwarhili · 2026-08-25T08:38:06Z

Here’s my code review of this #1313

handleRowSelected read the permission set through a ref and was not recreated when it changed, while the effect re-emitting the selection only depends on the selection and the row model. Permissions resolve asynchronously, so a selection emitted before the answer arrived kept reporting that nothing was locked, and delete, cut, paste and assemble stayed enabled on locked content. The set is referentially stable already - replaceIfChanged keeps the previous one when the content is unchanged - so it can simply be a dependency, and the ref goes away. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

The set was only published once every permission had an answer, so nothing was locked while the first fetch was in flight: forms opened and the activation switch and description dialog were live on content the user may not write into. It is now published before the fetch starts and refined by the cache update it triggers, buildReadOnlySharedModificationUuids already keeping the ids the cache has no answer for. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

useSharedModificationsPermissions was given the node's flat modification list, so only top-level references were submitted to hasElementPermission. A reference nested inside a composite was never queried, and the uuid stamped into its children's ancestorSharedModificationUuids could not be in the resulting set: its content stayed editable however the permissions on it. The table now calls the hook itself, over the reference modifications of the whole loaded tree, so a reference is resolved as soon as the composite holding it is unfolded. The readOnlySharedModificationUuids prop is gone, and so is the hook from the public API - nothing outside the table needs it. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

…difications-permissions

hasElementPermission was called once per shared modification, which grew with the number of references loaded in the tree. getAccessibleElements resolves them all in one request to explore-server, and hasElementPermission is now built on top of it, keeping its signature. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

…difications-permissions

ghazwarhili · 2026-08-27T13:10:19Z

    }, []);

+    // As the description is empty and we can't update it, we don't want to render the cell and its button
+    if (empty && isSaveDisabled) {


and if openDescModificationDialog = true
(component is already mounted)

I don't understand what you want to change exactly. Can you suggest the code here ?

ghazwarhili · 2026-08-27T13:35:11Z

+            .catch((error) => {
+                console.error('Failed to resolve the permissions on the shared modifications', error);
+                // Denying them all is what the cache already reports while they are unresolved
+                return [];


why return [] ? (should be return;)
even if the error is about fetch failure ? (it will be treaten like a real deny)

Actually, it was to avoid allowing permission if we don't know if the user has the rights.
But it's not necessary anymore and it removes them from missing IDs which is wrong.
I change it.

ghazwarhili · 2026-08-27T13:36:24Z

+                return [];
+            })
+            .then((accessibleIds) => {
+                if (aborted) {


if (aborted || accessibleIds == null) return;

I switched the order of then and catch, then I don't need this.

ghazwarhili · 2026-08-27T13:38:59Z

+    useEffect(() => {
+        let aborted = false;
+
+        const referenceIds = getReferenceIds(modifications.filter(isReferenceModification));


use useMemo for referenceIds (outside the useEffect)

Use memo is useful when there is a heavy compute or when you don't want to trigger re-render.
It's not the case here.

…difications-permissions Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> # Conflicts: # src/features/network-modification-table/utils.ts

Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

Caching a denial on failure dropped the id from the ones left to resolve, turning a transient error into a lock lasting until the cache is dropped. Leaving the cache untouched keeps them denied, since an id it doesn't answer for already counts as read-only, and asks for them again on the next trigger. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

…difications-permissions Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com> # Conflicts: # src/features/network-modification-table/network-modifications-table.tsx # src/features/network-modification-table/renderers/cell-renderers.tsx # src/features/network-modification-table/renderers/root-network-chip-cell.tsx # src/features/network-modification-table/utils.ts # src/utils/types/network-modification-metadata.ts

explore-server now exposes it as /explore/elements/accessible. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

sonarqubecloud · 2026-09-09T10:16:09Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

flomillot and others added 2 commits July 24, 2026 18:13

flomillot mentioned this pull request Jul 27, 2026

feat: deny editing the modifications held by a read-only shared modification gridsuite/gridstudy-app#4112

Open

flomillot and others added 7 commits July 29, 2026 10:16

Merge remote-tracking branch 'origin/florent/poc-shared-modifications…

03b3ddc

…-permissions' into florent/poc-shared-modifications-permissions

Merge remote-tracking branch 'origin/main' into florent/poc-shared-mo…

9ea99ed

…difications-permissions Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

flomillot mentioned this pull request Aug 14, 2026

Read the directory notification types from commons-ui gridsuite/gridexplore-app#956

Open

flomillot and others added 4 commits August 14, 2026 14:49

Clarify comment on replacing hook with Redux for permissions handling

4c072c4

Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

flomillot marked this pull request as ready for review August 17, 2026 09:17

flomillot requested review from TheMaskedTurtle, antoinebhs, ayolab and klesaulnier as code owners August 17, 2026 09:17

ghazwarhili reviewed Aug 20, 2026

View reviewed changes

clarify: update description cell logic to skip rendering when empty a…

b1197fb

…nd save is disabled Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

flomillot requested a review from ghazwarhili August 20, 2026 14:02

ghazwarhili reviewed Aug 25, 2026

View reviewed changes

ghazwarhili and others added 4 commits August 26, 2026 12:04

Code review shared modifications permission (#1313)

b62a185

flomillot added 2 commits August 26, 2026 13:16

Merge remote-tracking branch 'origin/main' into florent/poc-shared-mo…

ad4ca5a

…difications-permissions

flomillot mentioned this pull request Aug 27, 2026

Replace the element permission check with a batch endpoint gridsuite/explore-server#209

Open

Merge remote-tracking branch 'origin/main' into florent/poc-shared-mo…

f16d063

…difications-permissions

ghazwarhili reviewed Aug 27, 2026

View reviewed changes

flomillot added 3 commits September 3, 2026 14:19

Merge remote-tracking branch 'origin/main' into florent/poc-shared-mo…

f264c34

…difications-permissions Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> # Conflicts: # src/features/network-modification-table/utils.ts

Lock drag-and-drop interactions for read-only shared modifications

d947377

Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

flomillot requested a review from ghazwarhili September 4, 2026 11:17

flomillot and others added 2 commits September 7, 2026 11:32

Follow the batch permission endpoint rename

fe59ef9

explore-server now exposes it as /explore/elements/accessible. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

Uh oh!

Conversation

flomillot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

coderabbitai Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flomillot Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghazwarhili commented Aug 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghazwarhili Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Sep 9, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

flomillot commented Jul 27, 2026 •

edited

Loading

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

flomillot Aug 26, 2026 •

edited

Loading

ghazwarhili Aug 27, 2026 •

edited

Loading