Skip to content

Give the modifications an applicability for the tag of a new root network - #940

Open
flomillot wants to merge 17 commits into
mainfrom
feat/new-root-network-applicability
Open

flomillot wants to merge 17 commits into
mainfrom
feat/new-root-network-applicability

Conversation

@flomillot

@flomillot flomillot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Replaces #918.

Adds POST /network-modifications/root-network-tag?groupUuids&newTag: the modifications of the groups, and the shared ones they point to, get an applicability for the new tag unless they already have one. It is false when one of their applicabilities, for any tag, is false, and true otherwise.

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

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

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

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

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

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3f9d1f15-af26-4924-8b21-fca602ed0ab7
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

SELECT m, :tag, CASE WHEN EXISTS (SELECT 1 FROM ModificationRootNetworkApplicabilityEntity a
WHERE a.modification = m AND a.applicable = false) THEN false ELSE true END
FROM ModificationEntity m
WHERE m.id IN (:ids) AND TYPE(m) <> ModificationReferenceEntity

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.

All modification UUIDs go into a single IN (:ids) ?
we don't have sql limit here otherwise it will be rejected by sql? maybe we could filter by groupuuid ?
@etiennehomer what do you think ?

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.

With postgres, the limit is 65 535 parameters in IN clause. Should be ok

AND NOT EXISTS (SELECT 1 FROM ModificationRootNetworkApplicabilityEntity a
WHERE a.modification = m AND a.rootNetworkTag = :tag)
""")
void addRootNetworkApplicabilityIfMissing(@Param("ids") Collection<UUID> ids, @Param("tag") String tag);

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.

it is too complicated query, it should be reviewed by @etiennehomer or @SlimaneAmar

@etiennehomer etiennehomer Oct 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.

To me, we should avoid as much as we can to put intelligence in the SQL. Most of the complexity should be done in java with understandable code.
If an optimization is needed, it should only be an INSERT with a list of modification uuids and a list of applicabilities
Same as https://github.com/gridsuite/network-modification-server/pull/932/changes#r4324384990

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.

Yes I will remove it

@Query("""
INSERT INTO ModificationRootNetworkApplicabilityEntity (modification, rootNetworkTag, applicable)
SELECT m, :tag, CASE WHEN EXISTS (SELECT 1 FROM ModificationRootNetworkApplicabilityEntity a
WHERE a.modification = m AND a.applicable = false) THEN false ELSE true END

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.

including tags that are not in the study anymore ?

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.

Yes, whatever the study

if (ownedUuids.isEmpty()) {
return;
}
Set<UUID> sharedModificationTreesUuids = getSharedModificationTrees(ownedUuids).allUuids();

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.

these first lines are the same as in renameRootNetworkTag
=> create small private function to avoid fixing this in two places lates

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.

Can you suggest something because I don't see how to make it without adding more code ?

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.

you can create record hold both result of what you called ownedUuids and sharedTreesUuids ...
never mind it can be done next time

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.

Yes but it add more codes, create a record used only once, and don't make it clearer IMO, unless I haven't understand

Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
@flomillot
flomillot requested a review from EtienneLt as a code owner October 21, 2026 14:36
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
…pplicabilities' into HEAD

Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
@flomillot
flomillot changed the base branch from main to refactor/explicit-root-network-applicabilities October 21, 2026 15:05
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
@flomillot
flomillot requested a review from ghazwarhili October 21, 2026 15:54
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
…pplicabilities' into feat/new-root-network-applicability

Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Base automatically changed from refactor/explicit-root-network-applicabilities to main October 23, 2026 14:34
…-applicability

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

# Conflicts:
#	src/main/java/org/gridsuite/modification/server/entities/ModificationEntity.java
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>

# Conflicts:
#	src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java
#	src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
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.

3 participants