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

Dsl::Compilers::ActiveRecordRelations: fix #find_sole_by, #sole #1007

Merged
merged 1 commit into from
Jun 27, 2022
Merged

Dsl::Compilers::ActiveRecordRelations: fix #find_sole_by, #sole #1007

merged 1 commit into from
Jun 27, 2022

Conversation

kivikakk
Copy link
Contributor

Motivation

The specs introduced for sole and find_sole_by in 5f90df1 are not quite right.

See https://github.com/kivikakk/rails/blob/9e1cdf27ebaff31005f3d2bf02d97dfb116a3707/activerecord/lib/active_record/relation/finder_methods.rb#L107-L131.

Implementation

Added logic for each of these to generated the correct signatures.

Tests

Tests adjusted.

@ghost ghost added the cla-needed label Jun 21, 2022
@ghost ghost removed the cla-needed label Jun 21, 2022
@KaanOzkan KaanOzkan requested a review from a team June 21, 2022 19:35
Copy link
Collaborator

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

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

Thanks for spotting and fixing this!

@paracycle paracycle merged commit edca765 into Shopify:main Jun 27, 2022
@kivikakk kivikakk deleted the sole-adjustments branch June 28, 2022 00:59
@kivikakk
Copy link
Contributor Author

Thanks so much! 😊

paracycle added a commit that referenced this pull request Jul 1, 2022
Dsl::Compilers::ActiveRecordRelations: fix #find_sole_by, #sole
@paracycle paracycle added the backported Backported to stable branch label Jul 1, 2022
@shopify-shipit shopify-shipit bot temporarily deployed to production July 7, 2022 17:53 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to 0-9-stable August 19, 2022 20:37 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Backported to stable branch bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants