Drop default ORDER scope when calling a find method on a Sortable model

Glad to see this is finally happening. I think lots of specs will need to be fixed, though. :)

@stanhu I don't think so, because this only affects find and find_* queries, it leave all others (like all and where) alone! I just added a spec to verify this.

mentioned in merge request !8662 (closed)

added 1 commit

d2110db1 - Drop default ORDER scope when calling a find method on a Sortable model

Compare with previous version

assigned to @DouweM

@DouweM I added some small comments, and there are a whole bunch of test failures.

@yorickpeterse Those test failures seemed to just be CI failing for unrelated reasons. Let me see if this new push will do better.

added 1 commit

24cc7c19 - Drop default ORDER scope when calling a find method on a Sortable model

Compare with previous version

assigned to @yorickpeterse

@DouweM Stuff like this definitely seems suspicious:

 6) GlobalMilestone#initialize has all project milestones with the same title
     Failure/Error: original_impl.bind(unordered_relation).call(*args)

     ActiveRecord::RecordNotFound:
       Couldn't find Milestone without an ID
     # ./app/models/concerns/sortable.rb:20:in `call'
     # ./app/models/concerns/sortable.rb:20:in `block (2 levels) in <module:DropDefaultScopeOnFinders>'
     # ./app/models/global_milestone.rb:57:in `initialize'
     # ./spec/models/global_milestone_spec.rb:144:in `new'
     # ./spec/models/global_milestone_spec.rb:144:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:90:in `block (2 levels) in <top (required)>'

assigned to @DouweM

added 1 commit

4d6ee98d - Drop default ORDER scope when calling a find method on a Sortable model

Compare with previous version

@yorickpeterse Ah yeah, I didn't realize Relation#find could also take a block to function as regular old Array#find!

And we went green :D

@rspeicher Would you be interested in merging this?

assigned to @rspeicher

@DouweM Clever! LGTM. 👍

resolved all discussions

added 1 commit

b9ec3f23 - Make changelog more descriptive

Compare with previous version

added 1 commit

b9ec3f23 - Make changelog more descriptive

Compare with previous version

changed milestone to %9.4

approved this merge request

merged

mentioned in commit b07c0003

This merge request introduced a bug which was caught by a failing test on master - https://gitlab.com/gitlab-org/gitlab-ce/issues/34511

This test actually failed in the last pipeline executed for this merge request, but then it was retried and it passed 🤔 See https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/20404223.

mentioned in merge request !12557 (merged)

mentioned in issue #34511 (closed)

mentioned in merge request !12560 (merged)

mentioned in merge request !12633 (merged)

mentioned in issue #23079 (closed)

I don't think this works right 😞 https://gitlab.com/gitlab-org/gitlab-ce/issues/35110

Drop default ORDER scope when calling a find method on a Sortable model

Merge request reports

Activity