Skip to content

Add an endpoint sharing a composite modification of a group - #874

Open
flomillot wants to merge 4 commits into
mainfrom
feat/share-composite-modification
Open

Add an endpoint sharing a composite modification of a group#874
flomillot wants to merge 4 commits into
mainfrom
feat/share-composite-modification

Conversation

@flomillot

Copy link
Copy Markdown
Contributor

Adds POST /v1/network-composite-modifications/{uuid}/share?groupUuid=&name=, used when a composite modification of a study node becomes a shared modification stored in GridExplore.

In a single transaction, the composite modification is taken out of its group and a MODIFICATION_REFERENCE to it takes its place, at the very same order. The composite keeps its own uuid — it is moved, not duplicated — so the caller can store it in the directory server under that uuid, and it is renamed when a name is given.

Sharing a modification which is not a composite one is rejected with the new MODIFICATION_BAD_TYPE business error, and sharing one which is not owned by the given group with MODIFICATION_NOT_FOUND.

The composite modification is taken out of its group, so that it can be
stored as an element of the directory server, and a reference to it takes
its place - and its order - in the group. It keeps its own uuid, and is
renamed when a name is given.

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

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

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: Pro Plus

Run ID: d790c54e-6281-469b-9706-3747bab2f57b


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.

Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
if (!groupUuid.equals(modificationEntity.getContainerUuid())) {
throw new NetworkModificationServerException(MODIFICATION_NOT_FOUND,
String.format("Modification %s is not owned by group %s", modificationUuid, groupUuid),
Map.of(MODIFICATION_ID, modificationUuid + " (group = " + groupUuid + ")"));

@ghazwarhili ghazwarhili Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Map.of(MODIFICATION_ID, modificationUuid, "groupId", groupUuid)) ?
(Jackson serializes UUID as a string anyway)

@flomillot flomillot Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took same as existing

Map.of("modificationId", referenceUuid + " (container = " + getId() + ")"));

}
if (!groupUuid.equals(modificationEntity.getContainerUuid())) {
throw new NetworkModificationServerException(MODIFICATION_NOT_FOUND,
String.format("Modification %s is not owned by group %s", modificationUuid, groupUuid),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String.format(MODIFICATION_NOT_FOUND.messageTemplate(), modificationUuid) ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope it's a different one.

}

@PostMapping(value = "/{uuid}/share")
@Operation(summary = "Extract a composite modification from its group, replacing it by a reference to it")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract the composite modification from the group and replace it with a reference ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I don't see the difference and I prefer the original one

}

modificationRepository.save(referenceEntity);
modificationRepository.save(compositeEntity);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modificationRepository.save(referenceEntity);
modificationRepository.save(compositeEntity);

are you sure about those save ? they seem a little weird to me do you really need to perform the save operation after addModification? (the modifications are already committed)

@flomillot flomillot Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good catch and actually technically it works without. However, it totally depends on how other entities are annotated.

cascade = {CascadeType.PERSIST, CascadeType.MERGE, CascadeType.REFRESH, CascadeType.DETACH})

It's tricky for me and keeping the save is safer even if it's useless right now.

Moreover, there is already a similar case keeping the save.

Maybe we should ask a tech lead about it.

The composite modification keeps its own uuid when extracted, so the caller
already knows it.

Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
@flomillot
flomillot requested a review from ghazwarhili August 20, 2026 15:37
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants