Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modification can now be disabled #528

Merged
merged 17 commits into from
Sep 18, 2024
Merged

Modification can now be disabled #528

merged 17 commits into from
Sep 18, 2024

Conversation

klesaulnier
Copy link
Contributor

No description provided.

LE SAULNIER Kevin added 5 commits August 30, 2024 10:33
Copy link
Contributor

@antoinebhs antoinebhs left a comment

Choose a reason for hiding this comment

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

Some first comments, I will further test with the study-server PR

Copy link
Contributor

@souissimai souissimai left a comment

Choose a reason for hiding this comment

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

set active status for all modifications, example ModificationCreation.java

@@ -242,6 +242,16 @@ public ResponseEntity<Void> stashNetworkModifications(
return ResponseEntity.ok().build();
}

@PutMapping(value = "/network-modifications", produces = MediaType.APPLICATION_JSON_VALUE, params = "active")
@Operation(summary = "enable or disable network modifications")
@ApiResponse(responseCode = "200", description = "The network modifications were updated successfully")
Copy link
Contributor

@souissimai souissimai Sep 10, 2024

Choose a reason for hiding this comment

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

To clarify that only the activation status was updated and not the modification itself, you could rephrase the description:
Suggestion:
"The activation status related to the network modification was successfully updated"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

public ResponseEntity<Void> updateNetworkModificationsActivation(
@Parameter(description = "Network modification UUIDs") @RequestParam("uuids") List<UUID> networkModificationUuids,
@Parameter(description = "enable or disable network modifications") @RequestParam(name = "active") Boolean active) {
networkModificationService.updateNetworkModificationActivation(networkModificationUuids, active);
Copy link
Contributor

Choose a reason for hiding this comment

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

updateNetworkModificationActivationStatus?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -155,6 +155,32 @@ public void testCreate() throws Exception {
testCreationModificationMessage(createdModificationWithOnlyMetadata);
}

@Test
Copy link
Contributor

Choose a reason for hiding this comment

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

in my opinion :
When creating a modification, it is set to active by default, so there’s no need to explicitly approve the opposite.

@souissimai souissimai self-requested a review September 13, 2024 09:40
public ResponseEntity<Void> updateNetworkModificationsActivationStatus(
@Parameter(description = "Network modification UUIDs") @RequestParam("uuids") List<UUID> networkModificationUuids,
@Parameter(description = "enable or disable network modifications") @RequestParam(name = "active") Boolean active) {
networkModificationService.updateNetworkModificationActivation(networkModificationUuids, active);
Copy link
Contributor

Choose a reason for hiding this comment

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

activated ?

@@ -100,12 +100,17 @@ public class ModificationInfos {
@Schema(description = "Message values")
private String messageValues;

@Schema(description = "Modification enabled")
Copy link
Contributor

Choose a reason for hiding this comment

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

activated ?

@@ -242,6 +242,16 @@ public ResponseEntity<Void> stashNetworkModifications(
return ResponseEntity.ok().build();
}

@PutMapping(value = "/network-modifications", produces = MediaType.APPLICATION_JSON_VALUE, params = "active")
@Operation(summary = "enable or disable network modifications")
Copy link
Contributor

Choose a reason for hiding this comment

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

Change network modification activation status ?

@ApiResponse(responseCode = "200", description = "The activation status related to the network modification was successfully updated")
public ResponseEntity<Void> updateNetworkModificationsActivationStatus(
@Parameter(description = "Network modification UUIDs") @RequestParam("uuids") List<UUID> networkModificationUuids,
@Parameter(description = "enable or disable network modifications") @RequestParam(name = "active") Boolean active) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Network modification activation status ?

@@ -242,6 +242,16 @@ public ResponseEntity<Void> stashNetworkModifications(
return ResponseEntity.ok().build();
}

@PutMapping(value = "/network-modifications", produces = MediaType.APPLICATION_JSON_VALUE, params = "active")
Copy link
Contributor

Choose a reason for hiding this comment

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

activated ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed all renamining only for comments, changing all variable names would impact changesets, other services and frontend
after discussion, we've decided not to change them

.findById(modificationUuid)
.orElseThrow(() -> new NetworkModificationException(MODIFICATION_NOT_FOUND, String.format(MODIFICATION_NOT_FOUND_MESSAGE, modificationUuid)));
modificationEntity.setActive(active);
this.modificationRepository.save(modificationEntity);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove save

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@SlimaneAmar
Copy link
Contributor

Change requested just for only save

Signed-off-by: LE SAULNIER Kevin <[email protected]>
LE SAULNIER Kevin added 2 commits September 16, 2024 15:11
Signed-off-by: LE SAULNIER Kevin <[email protected]>
Signed-off-by: LE SAULNIER Kevin <[email protected]>
Copy link

@klesaulnier klesaulnier merged commit 433fad1 into main Sep 18, 2024
3 checks passed
@klesaulnier klesaulnier deleted the desactivate_modification branch September 18, 2024 08:14
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