Can add multiple (duplicate or conflicting) conditions

[sssbox]

https://github.com/adamchainz/gargoyle/blob/9d699ae94deccedd35b90b0ce93f48fcb503f9dc/gargoyle/models.py#L141

It seems like this line of code is supposed to prevent adding conditions that already exist, however because self.value[namespace][field_name] is a list of tuples with the second item as the condition, the condition not in will always be True.

Seems like there are two solutions, either just prevent duplicates:

condition_tuple = (exclude and EXCLUDE or INCLUDE, condition)
if condition_tuple not in self.value[namespace][field_name]:
    self.value[namespace][field_name].append(condition_tuple)

or prevent adding conflicting or duplicate conditions (seems like this would probably make more sense, however probably shouldn't just silently fail, should probably let the user know to remove the other condition first):

if condition not in (c for _, c in self.value[namespace][field_name]):

I might be able to find some time to work on a patch if nobody else wants to quickly throw something together, but would like some feedback first.

[adamchainz]

Thanks for your in-depth analysis. It looks right to me. It also seems you are half-way to a patch so it would be easiest for you to do it, if you can find the time.

[sssbox]

Thanks Adam.

Do you think it would be worth preventing conflicting conditions or should I just prevent duplicates? Preventing conflicting conditions would require more changes to let users know why the new condition wasn't added. Also, there might be some edge cases where it might be useful to have (at least temporarily) conflicting conditions that I've not considered.

[adamchainz]

Conflicting would be more useful, but perhaps preventing duplicates would be a good first step.

[adamchainz]

Fixed in #212