Skip to content

MONGOID-4998 deprecate :id_sort and implement limit positional argument on #first/#last #5358

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 28 commits into from
Jul 12, 2022
Merged

MONGOID-4998 deprecate :id_sort and implement limit positional argument on #first/#last #5358

merged 28 commits into from
Jul 12, 2022

Conversation

neilshweky
Copy link
Contributor

@neilshweky neilshweky commented Jul 7, 2022 •
edited
Loading

PLEASE READ THIS before reviewing this PR. There's a lot going on here, and a lot still left to do.

First, this change is actually a few different changes wrapped in one (besides all of the #take stuff under it):

  1. remove support for the id_sort option
  2. add support for the limit option
  3. add support for passing options in all of our first/last methods, so that users don't get errors when they pass in options to different first/lasts. Note that our syntax differs from Ruby's. We use keyword args (well actually, a hash) whereas Ruby uses an optional arg to pass the limit. This is annoying, but I fear it would be too complicated to completely do away with the hash in favor of just a limit integer.

Things still left to do:

  • Release notes about changes to first/last
  • Deprecate the id_none option in 7.5 and add a release note

Note that this PR should not be merged before that of MONGOID-4547, since this deprecation/removal of the id_none option only works once we have take. take provides the functionality that the id_none option did in first/last.

Future directives:

  • first! method
  • last! method
  • other take-like methods: second, third etc.

neilshweky
neilshweky commented Jul 7, 2022
View reviewed changes
@neilshweky neilshweky changed the title MONGOID-4998 drop support for :id_sort and implement :limit option on #first/#last MONGOID-4998 deprecate :id_sort and implement limit positional argument on #first/#last Jul 7, 2022
@neilshweky neilshweky mentioned this pull request Jul 7, 2022
Merged
@neilshweky neilshweky requested review from p-mongo and comandeo July 7, 2022 23:49
neilshweky added 19 commits July 8, 2022 14:19
@neilshweky
MONGOID-4998 add limit to #first
c0bd5d8
@neilshweky
MOGNOID-4998 add none implementation
da0f01a
@neilshweky
MONGOID-4998 add caching and limit to last, fix first tests
d3381d8
@neilshweky
MONGOID-4998 pass options through on memory, none, findable
ddb599d
@neilshweky
MONGOID-4998 add last findable tests
0efb7bf
@neilshweky
Apply suggestions from code review
0d1f1fe
@neilshweky
MONGOID-4998 add mongoid warning module
5260338
@neilshweky
MONGOID-4998 make limit a positional argument
7528b1a
@neilshweky
MONGOID-4998 update last signatures
6760951
@neilshweky
MONGOID-4998 fix last sig
fa9aa35
@neilshweky
MONGOID-4998 fix mongo tests
3482f1b
@neilshweky
MONGOID-4998 fix tests
f2e557f
@neilshweky
MONGOID-4998 fix enumerable test
14c229f
@neilshweky
MONGOID-4998 add release note
73e65da
@neilshweky
MONGOID-4998 refactor numbered_cache
aa0e750
@neilshweky
MONGOID-4998 add back id_sort functionality
050828b
@neilshweky
MONGOID-4998 update queries.txt
c513520
@neilshweky
MONGOID-4998 np docs
a14a15f
@neilshweky
MONGOID-4998 fix test
d67d4a5
p-mongo
p-mongo reviewed Jul 9, 2022
View reviewed changes
p-mongo
p-mongo reviewed Jul 12, 2022
View reviewed changes
lib/mongoid/contextual/memory.rb Outdated
def first(*args)
eager_load([documents.first]).first
def first(limit_or_opts = nil)
if !limit_or_opts || limit_or_opts.is_a?(Hash)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

lib/mongoid/contextual/none.rb Outdated
# @return [ nil ] Always nil.
def last; nil; end
def last(limit_or_opts = nil)
if limit_or_opts && !limit_or_opts.is_a?(Hash)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@neilshweky @p-mongo
Update docs/reference/queries.txt
563716b 
Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>
neilshweky and others added 8 commits July 12, 2022 12:46
@neilshweky @p-mongo
Update docs/reference/queries.txt
20054a7 
Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>
@neilshweky @p-mongo
Update docs/release-notes/mongoid-7.5.txt
92de300 
Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>
@neilshweky @p-mongo
Apply suggestions from code review
0dc96b6 
Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>
@neilshweky @p-mongo
Update docs/release-notes/mongoid-7.5.txt
f44cdbd 
Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>
@neilshweky @p-mongo
Update lib/mongoid/warnings.rb
ddfc475 
Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>
@neilshweky
MONGOID-4998 fix up suggestions
c9e993d
@neilshweky
MONGOID-4998 use nil?
0ce5813
@neilshweky
MONGOID-4998 use nil? on last
f2d6de4
@neilshweky neilshweky requested a review from p-mongo July 12, 2022 16:54
@neilshweky neilshweky merged commit cb8418c into mongodb:master Jul 12, 2022
neilshweky added a commit that referenced this pull request Jul 12, 2022
@neilshweky @p-mongo
MONGOID-4998 deprecate :id_sort and implement limit positional argume…
467c228 
…nt on #first/#last (#5358)

* MONGOID-4998 add limit to #first

* MOGNOID-4998 add none implementation

* MONGOID-4998 add caching and limit to last, fix first tests

* MONGOID-4998 pass options through on memory, none, findable

* MONGOID-4998 add last findable tests

* Apply suggestions from code review

* MONGOID-4998 add mongoid warning module

* MONGOID-4998 make limit a positional argument

* MONGOID-4998 update last signatures

* MONGOID-4998 fix last sig

* MONGOID-4998 fix mongo tests

* MONGOID-4998 fix tests

* MONGOID-4998 fix enumerable test

* MONGOID-4998 add release note

* MONGOID-4998 refactor numbered_cache

* MONGOID-4998 add back id_sort functionality

* MONGOID-4998 update queries.txt

* MONGOID-4998 np docs

* MONGOID-4998 fix test

* Update docs/reference/queries.txt

Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>

* Update docs/reference/queries.txt

Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>

* Update docs/release-notes/mongoid-7.5.txt

Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>

* Update docs/release-notes/mongoid-7.5.txt

Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>

* Update lib/mongoid/warnings.rb

Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>

* MONGOID-4998 fix up suggestions

* MONGOID-4998 use nil?

* MONGOID-4998 use nil? on last

Co-authored-by: Oleg Pudeyev <39304720+p-mongo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p-mongo p-mongo p-mongo approved these changes

@comandeo comandeo Awaiting requested review from comandeo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@neilshweky @p-mongo