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

fix attribute sorting across Ruby versions #3153

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

robinboening
Copy link
Contributor

@robinboening robinboening commented Jan 21, 2025

Chaining sort_by methods caused issues because each subsequent sort_by effectively overrides the previous sort order by re-sorting the entire array based solely on the new condition. This can disrupt the prioritization of earlier sorting criteria.

Interestingly, chaining multiple sort_by calls has been working as expected in Ruby versions prior to 3.3.1, as the underlying behavior of Array#sort_by appears to have been consistent in preserving the prioritization of earlier steps.

Switching to a single sort_by call ensures compatibility across Ruby versions, including 3.3.1 and beyond.

Fixes #3152

@robinboening robinboening requested a review from a team as a code owner January 21, 2025 14:03
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Nice. Thank you

@tvdeyen tvdeyen added backport-to-7.1-stable Needs to be backported to 7.1-stable backport-to-7.2-stable Needs to be backported to 7.2-stable backport-to-7.3-stable Needs to be backported to 7.3-stable backport-to-7.4-stable Needs to be backported to 7.4-stable labels Jan 22, 2025
Copy link

codecov bot commented Jan 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.58%. Comparing base (32d212b) to head (fe62c1f).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3153   +/-   ##
=======================================
  Coverage   96.58%   96.58%           
=======================================
  Files         236      236           
  Lines        6360     6360           
=======================================
  Hits         6143     6143           
  Misses        217      217           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@robinboening robinboening requested a review from tvdeyen January 23, 2025 00:10
@tvdeyen
Copy link
Member

tvdeyen commented Jan 23, 2025

@robinboening a rebase should fix Rails 7.0 tests. Can you squash the "fix specs" commit into the previous one, while you're at it?

@robinboening
Copy link
Contributor Author

@robinboening a rebase should fix Rails 7.0 tests. Can you squash the "fix specs" commit into the previous one, while you're at it?

absolutely!

@robinboening robinboening force-pushed the fix_sorted_attributes branch from 0c60342 to a51b2d1 Compare January 23, 2025 22:32
Chaining sort_by methods caused issues because each subsequent sort_by effectively overrides the previous sort order by re-sorting the entire array based solely on the new condition. This can disrupt the prioritization of earlier sorting criteria.

Interestingly, chaining multiple sort_by calls has been working as expected in Ruby versions prior to 3.3.1, as the underlying behavior of Array#sort_by appears to have been consistent in preserving the prioritization of earlier steps.

Switching to a single sort_by call ensures the logic remains robust and compatible across Ruby versions, including 3.3.1.

Fixes AlchemyCMS#3152
@robinboening robinboening force-pushed the fix_sorted_attributes branch from a51b2d1 to fe62c1f Compare January 23, 2025 22:37
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thank you.

@alchemycms-bot
Copy link
Collaborator

💚 All backports created successfully

Status Branch Result
7.1-stable
7.2-stable
7.3-stable
7.4-stable

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-7.1-stable Needs to be backported to 7.1-stable backport-to-7.2-stable Needs to be backported to 7.2-stable backport-to-7.3-stable Needs to be backported to 7.3-stable backport-to-7.4-stable Needs to be backported to 7.4-stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resource#sorted_attributes behaves differently across Ruby versions
3 participants