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 order of line item based report rows #13154

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Feb 14, 2025

What? Why?

The orders_and_fulfillment_spec would sometimes fail when the database would return line items in a different order than they were created. Without specific order clause the order of rows can be random.

The additional sorting may lead to more server load but also ensures more consistent results for users.

Related CI fail:

What should we test?

  • Reports are well tested but we should test the performance impact.
  • Observer loading times for the orders and fulfillment reports before and after staging.

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

The orders_and_fulfillment_spec would sometimes fail when the database
would return line items in a different order than they were created.
Without specific `order` clause the order of rows can be random.

The additional sorting may lead to more server load but also ensures
more consistent results for users.
@mkllnk mkllnk added the user facing changes Thes pull requests affect the user experience label Feb 14, 2025
@mkllnk mkllnk self-assigned this Feb 14, 2025
@mkllnk mkllnk marked this pull request as ready for review February 14, 2025 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user facing changes Thes pull requests affect the user experience
Projects
Status: Code review 🔎
Development

Successfully merging this pull request may close these issues.

[Flaky] ./spec/system/admin/reports/orders_and_fulfillment_spec.rb
1 participant