Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fixed filter by enum filter when label <> value #2971

Closed
wants to merge 6 commits into from
Closed

Fixed filter by enum filter when label <> value #2971

wants to merge 6 commits into from

Conversation

mshytikov
Copy link
Contributor

@mshytikov mshytikov commented Dec 24, 2017

Refactored code to remove unnecessary calls to parse_value.
Which were affecting filter by enum (rails >= 5.0):
If enum defined like

enum size: { L: 'l', S: 's' }

Due to multiple calls, the chain when filtering by size is following:

parse_value("l") => "L"
parse_value("L") => nil

Which leads to non working filtering.
NOTE: parse_value returns correct result in both cases above

@mshibuya
Copy link
Member

Could you add a test case for that issue?

@mshytikov
Copy link
Contributor Author

Sure, I will add tests.
(need to find some time, should be ready in few days. )

Refactored code to remove unnecessary calls to `parse_value`.
Which were affecting filter by enum (rails >= 5.0):
If enum defined like
```
enum size: { L: 'l', S: 's' }
```
Due to multiple calls, the chain when filtering by size is following:
```
parse_value("l") => "L"
parse_value("L") => null
```
Which leads to non working filtering.
NOTE: `parse_value` returns correct result in both cases above
@mshytikov
Copy link
Contributor Author

work in progress.

@mshytikov
Copy link
Contributor Author

Turns out that fix provided in #2952 for active_record_enum.rb is also required to pass the build for some older rails versions

@mshytikov
Copy link
Contributor Author

@mshibuya I have added tests. And have rebased them, so it is possible to see in travis logs that without fix they are failing.
I was quite unlucky with this PR that I also faced pg update see the last commit message.
Could you please review PR ?

mshibuya added a commit that referenced this pull request Feb 17, 2018
Fixed filter by enum filter when label <> value
@mshibuya
Copy link
Member

Merged in 8576961, thanks so much!

@mshytikov
Copy link
Contributor Author

👍 thank you.

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.

2 participants