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.

Sign up for GitHub

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

Jump to bottom

Fixes #26161: Allow to enable/disable technique from the directive tree scree #6117

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Fixes #26161: Allow to enable/disable technique from the directive tree scree #6117

merged 1 commit into from
Jan 22, 2025
+304 −118

Conversation

fanf
Copy link
Member

@fanf fanf commented Jan 9, 2025 •
edited
Loading

https://issues.rudder.io/issues/26161

Add a button to enable/disable technique from the directive tree screen. A validation pop-up is shown with the impacted rules before actually doing the action.

Nothing hard, just tedious old lift.

Screencast_20250116_213950.webm

@fanf fanf requested a review from clarktsiory January 9, 2025 17:51
clarktsiory
clarktsiory reviewed Jan 9, 2025
View reviewed changes
Copy link
Contributor

@clarktsiory clarktsiory left a comment

Choose a reason for hiding this comment

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

LGTM, it works as expected and it is a quick but great improvement !
The UX could be changed a bit

...-web/src/main/scala/com/normation/rudder/web/snippet/configuration/DirectiveManagement.scala Outdated Show resolved Hide resolved
@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/6117
-- Your faithful QA
Kant merge: "In law a man is guilty when he violates the rights of others. In ethics he is guilty if he only thinks of doing so."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/95633/console)

@fanf
Copy link
Member Author

fanf commented Jan 15, 2025

I think I will add a validation pop-up given the comment.

@fanf fanf marked this pull request as draft January 15, 2025 19:27
@fanf fanf force-pushed the enh_26161/allow_to_enable_disable_technique_from_the_directive_tree_scree branch from 7171146 to 2b2b979 Compare January 16, 2025 20:44
@fanf fanf force-pushed the enh_26161/allow_to_enable_disable_technique_from_the_directive_tree_scree branch from 2b2b979 to 5554d56 Compare January 16, 2025 20:46
@fanf fanf marked this pull request as ready for review January 16, 2025 20:50
clarktsiory
clarktsiory approved these changes Jan 17, 2025
View reviewed changes
Copy link
Contributor

@clarktsiory clarktsiory left a comment

Choose a reason for hiding this comment

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

The popup is nice !
LGTM !

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/6117
-- Your faithful QA
Kant merge: "It is beyond a doubt that all our knowledge begins with experience."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/95931/console)

@fanf
Copy link
Member Author

fanf commented Jan 22, 2025

OK, merging this PR

@fanf fanf merged commit 5554d56 into Normation:branches/rudder/8.1 Jan 22, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clarktsiory clarktsiory clarktsiory approved these changes

Assignees
No one assigned
Labels
qa: Can't merge Ready for merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fanf @Normation-Quality-Assistant @clarktsiory