Skip to content

Conversation

@Mathieu-Deharbe
Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe commented Nov 21, 2025

PR Summary

  • reorganises the logs of the limit sets modification (transfo and lines)
  • refacto on the AbstractBranchModification
  • correction of a bad display of the modificatin of the name
  • corrections of a bug : when all the limit sets from a brnch are deleted, it doesn't work

Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Copy link
Contributor

@EtienneLt EtienneLt left a comment

Choose a reason for hiding this comment

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

some code suggestions

if (applicableOnSide1() && modifiedBranch.getOperationalLimitsGroup1(olgId).isEmpty() ||
applicableOnSide2() && modifiedBranch.getOperationalLimitsGroup2(olgId).isEmpty()) {
throw new PowsyblException(
"Cannot delete operational limit group " + olgId + " which has not been found in equipment on " + applicabilityToString(olgModifInfos.getApplicability()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Cannot delete operational limit group " + olgId + " which has not been found in equipment on " + applicabilityToString(olgModifInfos.getApplicability()));
"Cannot delete operational limit group " + olgId + " because it was not found in equipment on " + applicabilityToString(olgModifInfos.getApplicability()));

Copy link
Contributor

Choose a reason for hiding this comment

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

could add equipmentId to know which one is concerned ?

@EtienneLt
Copy link
Contributor

missing detailed logs when creating

Signed-off-by: Mathieu DEHARBE <[email protected]>
Copy link
Contributor

@EtienneLt EtienneLt left a comment

Choose a reason for hiding this comment

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

code ok I need to test

* @param side which side of the branch receives the limits
*/
public void setCurrentLimitsOnASide(ReportNode reportNode, List<OperationalLimitsGroupInfos> opLimitGroups, Branch<?> branch, TwoSides side) {
public void setCurrentLimitsOnASide(ReportNode reportNode, OperationalLimitsGroupInfos opLimitsGroup, Branch<?> branch, TwoSides side) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public void setCurrentLimitsOnASide(ReportNode reportNode, OperationalLimitsGroupInfos opLimitsGroup, Branch<?> branch, TwoSides side) {
public void setCurrentLimits(ReportNode reportNode, OperationalLimitsGroupInfos opLimitsGroup, Branch<?> branch, TwoSides side) {

Signed-off-by: Mathieu DEHARBE <[email protected]>
protected void applyModificationToOperationalLimitsGroup() {
switch (olgModifInfos.getModificationType()) {
case OperationalLimitsGroupModificationType.MODIFY_OR_ADD:
switch (olgModifInfos.getApplicability()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i think you should extract the content of this switch in a function

Comment on lines +84 to +88
if (modifiedOperationalLimitsGroup2() == null) {
addOlg(SIDE2);
} else {
modifyOLG(SIDE2);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this code almost duplicate with the one above you should make a function as it can also be useful below

Comment on lines +259 to +260
} else {
if (currentLimits != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

merge if with else

Comment on lines +301 to +308
OperationalLimitsGroup modifiedOlg = modifiedOperationalLimitsGroup1();
if (modifiedOlg != null) {
modifiedOlg.removeCurrentLimits();
removeAllProperties(modifiedOlg);
}
OperationalLimitsGroup newOperationalLimitsGroup = modifiedBranch.newOperationalLimitsGroup1(olgModifInfos.getId());
modifyCurrentLimits(newOperationalLimitsGroup.newCurrentLimits(), null, SIDE1);
addProperties(newOperationalLimitsGroup, SIDE1);
Copy link
Contributor

Choose a reason for hiding this comment

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

this code could be extracted into a function

@sonarqubecloud
Copy link

Copy link
Contributor

@EtienneLt EtienneLt left a comment

Choose a reason for hiding this comment

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

Test Ok code Ok

@Mathieu-Deharbe Mathieu-Deharbe merged commit 9e3304d into main Nov 27, 2025
4 checks passed
@Mathieu-Deharbe Mathieu-Deharbe deleted the limits-modification-logs branch November 27, 2025 10:09
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.

4 participants