Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Enum should allow the conventionally case-sensitive operators #434

Merged
merged 2 commits into from
Mar 18, 2024
Merged

Enum should allow the conventionally case-sensitive operators #434

merged 2 commits into from
Mar 18, 2024

Commits on Mar 18, 2024

  1. Enum should allow the conventionally case-sensitive operators 

    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 polymorphic_has_* 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 polymorphic_has_* 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 polymorphic_has_* helper.

**Warning** no tests yet. This PR primarily for discussion.
    @jasonkarns @jkeen
    jasonkarns authored and jkeen committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    4afe29b View commit details
    Browse the repository at this point in the history

  2. Add some tests for string_enum:eql and string_enum:not_eql

    @jkeen
    jkeen committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    14618c5 View commit details
    Browse the repository at this point in the history