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 #24097: Skipped directive does not show in node compliance tree #5630

Open
wants to merge 1 commit into
base: branches/rudder/8.0
Choose a base branch
from

Conversation

clarktsiory
Copy link
Contributor

@clarktsiory clarktsiory commented Apr 25, 2024

https://issues.rudder.io/issues/24097

Essentially the same as #5600, and with some refactoring :

  • move the Elm model for SkippedDirective to the Elm Compliance.Datatypes module
  • compute the skipped directives in a method which takes the conversion DirectiveComplianceOverride => T as a param, either ByRuleDirectiveCompliance or ByRuleDirectiveCompliance (this PR)

Screenshot from 2024-04-25 17-48-22

@clarktsiory clarktsiory requested a review from fanf April 25, 2024 15:48
@fanf
Copy link
Member

fanf commented Apr 30, 2024

So, the code looks good and we now have skipped in node, but I still think there is something broken: in the case where the rule has only one directive and that directive is skipped, then the rule doesn't appear at all in the node.

Step to reproduce: in your screenshot example, remove the directive "File download (HTTP)" from rule "Install package skip"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants