Skip to content

fix: keep modification activation status unchanged on metadata update#819

Open
flomillot wants to merge 1 commit into
mainfrom
activated-default-on-creation
Open

fix: keep modification activation status unchanged on metadata update#819
flomillot wants to merge 1 commit into
mainfrom
activated-default-on-creation

Conversation

@flomillot
Copy link
Copy Markdown
Contributor

The activated flag was defaulted to true in the ModificationInfos DTO, so after deserialization it was never null when omitted from a request. As a result, the null guard in updateNetworkModificationMetadata was always satisfied, and a metadata update that did not provide the flag silently reactivated the modification.

This was visible when renaming a deactivated composite modification in Grid Explorer: the rename reactivated the composite and its content.

Changes

  • The true default is now applied at creation only, in the ModificationEntity(ModificationInfos) constructor. When the flag is not provided on a metadata update, the activation status is left unchanged.
  • Added a test verifying that metadata fields (name, description, activated) not provided on an update keep their previous value.

This change depends on the matching DTO change: gridsuite/network-modification#190


Important

Penser à mettre à jour la version de network-modification : le pom.xml pointe actuellement sur 0.84.0-SNAPSHOT. Une fois la PR network-modification#190 fusionnée et publiée, il faudra remplacer cette version par la version released avant de fusionner cette PR.

The activated flag was defaulted in the ModificationInfos DTO, so it was
never null after deserialization. A metadata update that omitted the flag
therefore always reactivated the modification (visible when renaming a
deactivated composite modification).

The flag is now defaulted to true at creation only. When it is not
provided on a metadata update, the activation status is left unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR introduces null-safety handling for the activated field in ModificationEntity and upgrades the network-modification dependency. The entity constructor now defaults activated to true when null is received from the DTO, preventing null propagation. A new integration test validates that composite modification metadata remains unchanged when updates provide empty payloads.

Changes

Activation field null-safety and dependency update

Layer / File(s) Summary
Dependency version upgrade
pom.xml
Network-modification dependency version updated from 0.83.0 to 0.84.0-SNAPSHOT.
Null-safe activation field initialization with validation
src/main/java/org/gridsuite/modification/server/entities/ModificationEntity.java, src/test/java/org/gridsuite/modification/server/ModificationControllerTest.java
ModificationEntity constructor defaults activated field to true when the incoming ModificationInfos activation flag is null. New integration test verifies that updating composite modification metadata with empty fields does not alter previously stored metadata values (name, description, activated).

Suggested reviewers

  • basseche
  • Mathieu-Deharbe
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main fix: preventing unintended activation of modifications during metadata updates.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the root cause, the fix, test additions, and dependency information.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pom.xml`:
- Line 56: Update the Maven property network-modification.version in the pom.xml
to the released version instead of the snapshot; replace the value
"0.84.0-SNAPSHOT" with the final released version string (e.g., "0.84.0") after
the dependency PR is merged and published so builds use the released artifact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5923cda7-1301-45ed-b558-c64abf1234df

📥 Commits

Reviewing files that changed from the base of the PR and between 6def173 and f2c6f9d.

📒 Files selected for processing (3)
  • pom.xml
  • src/main/java/org/gridsuite/modification/server/entities/ModificationEntity.java
  • src/test/java/org/gridsuite/modification/server/ModificationControllerTest.java

Comment thread pom.xml
<sonar.projectKey>org.gridsuite:network-modification-server</sonar.projectKey>
<!-- TODO network-modification.version remove when included in gridsuite-dependencies -->
<network-modification.version>0.83.0</network-modification.version>
<network-modification.version>0.84.0-SNAPSHOT</network-modification.version>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update to released version before merging.

The dependency currently points to 0.84.0-SNAPSHOT. As noted in the PR description, this should be updated to the released version after the dependency PR is merged and published.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pom.xml` at line 56, Update the Maven property network-modification.version
in the pom.xml to the released version instead of the snapshot; replace the
value "0.84.0-SNAPSHOT" with the final released version string (e.g., "0.84.0")
after the dependency PR is merged and published so builds use the released
artifact.

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.

1 participant