Skip to content

feat: multiple conditions on query rules#1171

Merged
nunomaduro merged 1 commit intomasterfrom
feat/adds-multiple-conditions-on-rules
Jul 13, 2020
Merged

feat: multiple conditions on query rules#1171
nunomaduro merged 1 commit intomasterfrom
feat/adds-multiple-conditions-on-rules

Conversation

@nunomaduro
Copy link
Copy Markdown

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

Closes #1170.

@nunomaduro nunomaduro requested review from Haroenv and aseure June 25, 2020 13:32
/**
* Conditions of the rule, expressed using the following variables: pattern, anchoring, context.
*/
readonly conditions?: readonly Condition[];
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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