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

Fix BPNL Group validation operators logic #1673

Merged

Conversation

bmg13
Copy link
Contributor

@bmg13 bmg13 commented Nov 11, 2024

WHAT

Fixes some of the existing BPN Group validation operators' scope and proposes the removal of potentially redundant ones.

WHY

Too keep the expected behaviour of some operators, specially the isAllOf since was not validating properly (switched the assigned and allowed groups in the validation).

FURTHER NOTES

The suggestions are based on expected behaviour (isAllOf should check if all the allowed groups are present in the assigned groups) and potential improvements (eq e neq could maybe be removed since they do not seem to add much value when isAllOf, isAnyOf and isNoneOf exist).

Changed the BpnGroupHolder properties to Sets simply to easily avoid repeated entries.

Tests updated accordingly.

Closes #1665

Copy link
Contributor

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale label Nov 21, 2024
@ndr-brt ndr-brt self-requested a review November 21, 2024 08:25
Copy link
Contributor

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

my suggestion would be to just deprecate the eq and neq operators with a warn log printed, to be removed in the next versions.

@bmg13 bmg13 requested a review from ndr-brt November 21, 2024 12:33
@github-actions github-actions bot removed the stale label Nov 22, 2024
@bmg13 bmg13 requested a review from ndr-brt November 22, 2024 11:50
Copy link
Contributor

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

almost there

@bmg13 bmg13 requested a review from ndr-brt November 22, 2024 14:51
Copy link
Contributor

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

LGTM

@ndr-brt
Copy link
Contributor

ndr-brt commented Nov 22, 2024

@bmg13 please fix dependencies check and we'll be ready to go

@bmg13
Copy link
Contributor Author

bmg13 commented Nov 25, 2024

@ndr-brt took a few tries, but is done :) thank you!

@ndr-brt ndr-brt merged commit ef8189a into eclipse-tractusx:main Nov 26, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

Fix BPNL Group Validation Operator's Logic
2 participants