graphiti-api / graphiti

Stylish Graph APIs
https://www.graphiti.dev/
MIT License
960 stars 138 forks source link

Enum should allow the conventionally case-sensitive operators #434

Closed jasonkarns closed 3 months ago

jasonkarns commented 1 year ago

By default (ie, for strings), eq is a case-insensitive match and eql is case-sensitive.

string_enum is documented as being "like string but only with case-sensitive eq/not_eq operators".

These two statements feel like contradictions to me. If string_enum is basically like string but always does case-sensitive filtering, then shouldn't it support the existing case-sensitive operator?

Some more rationale why I think this should be the case: polymorphic associations have a discrete list of possible values for the as_type attribute. So to me it's only natural that the as_type attribute be defined as a string_enum. (strict case matching!)

However, using the polymorphichas helpers results in an relationship filter using the eql (case-sensitive) operator. So we have an attribute type that expects to be matched strictly; and a relationship filter that matches strictly. But the two are incompatible! (because string_enum only defines eq operator, and polymorphichas helpers define eql operator; despite both ostensibly wishing to compare strictly case-sensitve.)

So this PR proposes that the enum types also define eql operators (which are already expected to be case-sensitive for strings) essentially as duplicates of the existing eq operators. Doing so will allow polymorphic as_type attributes to be defined as string_enum and still leverage the eql filter applied by the polymorphichas* helper.

Warning no tests yet. This PR primarily for discussion.

jkeen commented 4 months ago

@jasonkarns I just hopped into the ring to get this project back up to speed, and what you said makes sense to me. Would you mind adding some tests for it?

jkeen commented 4 months ago

Related to #403

github-actions[bot] commented 3 months ago

:tada: This PR is included in version 1.5.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: