adds a name to the composite modifications + updates the composite creation endpoint#762
adds a name to the composite modifications + updates the composite creation endpoint#762Mathieu-Deharbe wants to merge 10 commits intomainfrom
Conversation
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Meklo
left a comment
Meklo
left a comment
There was a problem hiding this comment.
Evolving ModificationInfos to include composite subModification and its name in the message values will be done later ?
Additionally I've noticed when using INSERT_COMPOSITE it create two new entries in composite_modification table which corresponds to two modifications, one with the targetted group_uuid and one with a blank group_uuid, is this intended or a mishap on my side ?
| MOVE, | ||
| COPY, | ||
| SPLIT_COMPOSITE, // the network modifications contained into the composite modifications are extracted and inserted one by one | ||
| INSERT_COMPOSITE // the composite modifications are fully inserted as composote modifications |
| <changeSet author="deharbemat (generated)" id="1770384121049-15"> | ||
| <addColumn tableName="composite_modification"> | ||
| <column name="composite_name" type="varchar(255)"/> | ||
| </addColumn> |
There was a problem hiding this comment.
There's no issue if it hasn't a default name for preexisting composite modifications ?
There was a problem hiding this comment.
I don't see how it could happen but I can add one just in case. Like "My composite" ?
The thing is that this name is useless for now and can be useful ony if the composite is inserted into gridstudy, and then it gets a name. How I see it : the front interface will use the name of the composite in grid-explore (its element name) as a default value and then call the endpoint with it.
But you are right : better to add one just in case.
| @Parameter(description = "the modification Uuid to move before (MOVE option, empty means moving at the end)") @RequestParam(value = "before", required = false) UUID beforeModificationUuid, | ||
| @Parameter(description = "origin group UUID, where modifications are copied or cut") @RequestParam(value = "originGroupUuid", required = false) UUID originGroupUuid, | ||
| @Parameter(description = "modifications can be applied (default is true)") @RequestParam(value = "build", required = false, defaultValue = "true") Boolean canApply, | ||
| @Parameter(description = "composite modifications names") @RequestParam(value = "compositeNames", required = false) List<String> compositeNames, |
There was a problem hiding this comment.
Shouldn't it be added to modificationContextInfos ? Like do you think it would be possible to replace List from the Pair to a list of records/dto containing UUID and their coresponding compositeName ?
compositeName would be optional to keep the system working for copying non composite modifications
There was a problem hiding this comment.
I don't know. I don't think any of those ways is satisfying because it will be useful only for INSERT_COMPOSITE. At least here the other action types can completely ignore that parameter, no structure has to be changed. But I don't like it either.
What do you think @SlimaneAmar ?
| @NonNull List<UUID> compositeModificationsUuids, | ||
| @NonNull List<String> compositeNames) { | ||
| List<CompositeModificationEntity> newCompositeModifications = new ArrayList<>(); | ||
| for (int i = 0; i < Math.min(compositeNames.size(), compositeModificationsUuids.size()); i++) { |
There was a problem hiding this comment.
There can be a discrepancy between UUIDs and names amount ?
There was a problem hiding this comment.
There shouldn't be. I am being a bit paranoid because the front part is not done yet and I am not 100% sure how it will be done.
But maybe I should check that compositeNames.size() == compositeModificationsUuids.size() and return an error if it ever happens, instead of this.
| @NonNull List<String> compositeNames) { | ||
| List<CompositeModificationEntity> newCompositeModifications = new ArrayList<>(); | ||
| for (int i = 0; i < Math.min(compositeNames.size(), compositeModificationsUuids.size()); i++) { | ||
| UUID newCompositeModificationUuid = cloneCompositeModification(compositeModificationsUuids.get(i), compositeNames.get(i)); // ici c'est fait orderonné par modification order plutôt que UUID |
There was a problem hiding this comment.
better translate the comment in english
There was a problem hiding this comment.
Sorry I had forgotten to push the cleaning. Removed : 1d4877a
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
I didn't think of that. Edit after discussion => not really the scope, will probably be added to the visualisation ticket, to be checked with the POs
I just checked : you seem to be right ! Sorry, I will look into it. => Edit : corrected |
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
2 participants
needs : gridsuite/network-modification#156