Re: [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system

[Leo Nunner <l.nunner@proxmox.com>]

On 2023-04-11 11:52, Thomas Lamprecht wrote:
> For now some higher level review inline from my side, not sure if I get to
> a more in-depth for each patch soonish.
>
> Am 07/04/2023 um 15:42 schrieb Leo Nunner:
>> This series introduces two new features for the PMG rule system: object
>> negation, and match groups. Negation allows the user to negate objects
>> inside a rule, meaning that they will evaluate to true if their
>> condition is NOT fulfilled.
> Do we really don't have any test system for the rule system, if that'd be some
> technical debt to address probably rather sooner than later.
We have regression tests for PMG, which should already cover this AFAIK.
>> The second feature, match groups, allows objects to be chained together.
>> A match group functions as a container for multiple other objects; it
>> will only evaluate to true if all its members evaluate to true. Match groups
>> are connected via logical 'or' to all other objects inside the rule;
>> take the following rule system:
>>
>> - Rule
>>     - Match Group
>> 	- Object 1
>> 	- Object 2
>>     - Object 3
>>
>> It will match if either (Object 1 && Object 2), or if Object 3
>> evaluate to true.
> Why not allow or matches? Most mail filter tools (like e.g., the x-sieve
> fronted of open-xchange) allow to configure if all or any of a group's matching
> rules must trigger for the whole group to be true.
>
> I mean, OR matching can be workarounded by duplicated rules but if we add
> groups with AND combination, then OR is something that I'd think is also
> expected to be there.

Not sure if I follow…

- Rule
    - Who Objects
        - Object 1
    - When Objects
        - Object 2

You would want something like this to match if either Who *or* When
produce a match? IIRC those are connected via AND right now.

>> To properly achieve match groups inside the GUI, the rule overview was
>> reworked as a tree, instead of a simple list. It can now be
>> collapsed/expanded, and each object type (except actions) has a single
>> subfolder called 'Match Group'.
>>
>> In combination, these features should cover quite a few use cases, and
>> make it possible to model more complex rule systems.
>>
>> pmg-api:
>>
>> Leo Nunner (8):
> can we please drop the weird feature prefix, doesn't really add anything and is
> relatively hard to parse as the subsystem is missing – PMG is somewhat small
> enough in scope, so one can guess that; but I'd rather have that explicit.
>
> Also, order and separation is a bit odd as some things that are implementing a
> single semantic work are split, and others like the api expand+add are merged,
> also making the whole thing work only after exposing it via the API just feels
> plain wrong and is actually risky if someone overlooks this and applies the
> first few patches already.
>
> So, order/separation could be IMO better if:
>
> commit 2 ("parse negation value into objects"), and 4 ("implement matching
> logic") are squashed into a "rule system: implement match negation" as they
> implement the internal/backend functionallity, and then exposing that via the
> api could be done in the next commit. The one adding the field to the rule db
> schema could be also squashed into the commit 2 & 4, but less hard feelings
> there, albeit I'd note in the commit message that it's a preparation for the
> next commits.
>
> […]
I'll change the commits around accordingly (and keep the formatting in
mind for future patches), thanks!