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

feat: multiple conditions on query rules #1171

Merged
merged 1 commit into from
Jul 13, 2020

Conversation

nunomaduro
Copy link
Contributor

This pull request adds the possibility of passing multiple conditions on query rules.

Closes #1170.

/**
* Conditions of the rule, expressed using the following variables: pattern, anchoring, context.
*/
readonly conditions?: readonly Condition[];
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be a disjunct union: { condition } | { conditions } with required in either case. That way you can’t have an object with both condition & conditions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Haroenv None of them are actually required.

However, we normally don't go that far with types. What do you think @Ant-hem @aseure?

Copy link
Contributor

Choose a reason for hiding this comment

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

What does a query rule without condition mean? One of them is required, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Haroenv No. You can the only required files are: objectID, and consequence. This is quite often used by people that want to apply a query rule all the time.

If you agree, I would to prefer to keep the rule typing just simple like this. We usually don't go that far with types, like possible combinations of key in rules, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with @nunomaduro on this: we don't go that far as enforcing the mutual exclusion here since the engine does not either.

/**
* Conditions of the rule, expressed using the following variables: pattern, anchoring, context.
*/
readonly conditions?: readonly Condition[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with @nunomaduro on this: we don't go that far as enforcing the mutual exclusion here since the engine does not either.

@nunomaduro nunomaduro merged commit 0985834 into master Jul 13, 2020
@nunomaduro nunomaduro deleted the feat/adds-multiple-conditions-on-rules branch July 13, 2020 13:04
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.

Multi-Condition Rules feature
3 participants