Fixes #23234 - Simplify parent scope lookup #7289
Conversation
|
Issues: #23234 |
tbrisker
force-pushed
the
23234
branch
2 times, most recently
from
e7da081
to
2f3d5a5
Compare
|
Let's drop Ruby 2.3 for develop first, I'd say... |
|
Should be dropped now, please remove the Ruby 2.3 compatibility code. ✨ |
|
[test katello] |
|
katello test failure maybe related? 🤔 |
|
seems to be consistently failing on the same test, so i assume it is related. will try to dig in. |
tbrisker
added
Waiting on contributor
and removed
Needs testing
Not yet reviewed
labels
Knowing the id format allows us to know which field we should search by, no need to try both id and friendly_id and use conditional order to pick the friendly id first.
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
Hope this will fix the failing katello test, looks like in some cases the id can be nil, added handling for them. |
|
merged, thanks @tbrisker! |
|
|
||
| filtered_scope = base_scope.where(arel_query) | ||
| # The id is an integer - no need to check more complex options, just scope by it | ||
| return base_scope.where(id: resource_id) if resource_id.integer? |
There was a problem hiding this comment.
I think this will miss records with name set to an integer - '123'.
There was a problem hiding this comment.
Good point, opened #7328 to fix this case.
| @@ -189,12 +189,26 @@ def truncate_bytes(truncate_at, omission: "…") | |||
| end | |||
| end | |||
| end | |||
|
|
|||
| def integer? | |||
There was a problem hiding this comment.
Instead of extending core object, we can use
foo.to_i.to_s == foo.to_s
Knowing the id format allows us to know which field we should search by,
no need to try both id and friendly_id and use conditional order to
pick the friendly id first.