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

Fixes #22525: Directives applied twice don't show in rule details (they should be skipped) #5600

Conversation

clarktsiory
Copy link
Contributor

@clarktsiory clarktsiory commented Apr 11, 2024

https://issues.rudder.io/issues/22525

We need to add all the details of a skipped directive in the compliance API (here only for the Rules compliance API)
Here we are reusing the logic used in the server-rendered compliance which is now added to the API in a new JSON field :

"skippedDetails": { "overridingRuleId": ..., "overridingRuleName": ... }

Result :

Screenshot from 2024-04-11 18-14-59

@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

@fanf
Copy link
Member

fanf commented Apr 17, 2024

LGTM. I'm not sure if we want to use the "skipped" tag when a rule has all its directives skipped.

image

I think it's a rare enought case to not matter a lot, and that version is clear.

I'm merging that one, and if we want to do the skipped tag for rule too, we will open an other ticker.

@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/5600
-- Your faithful QA
Kant merge: "Morality is not the doctrine of how we may make ourselves happy, but how we may make ourselves worthy of happiness."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/83385/console)

@fanf
Copy link
Member

fanf commented Apr 17, 2024

OK, squash merging this PR

@fanf fanf force-pushed the bug_22525/directives_applied_twice_don_t_show_in_rule_details_they_should_be_skipped branch from 0ff1417 to c99e311 Compare April 17, 2024 21:03
@fanf fanf merged commit c99e311 into Normation:branches/rudder/7.3 Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants