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 work order retry sorting and avoids loading dataclips #2581

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

jyeshe
Copy link
Member

@jyeshe jyeshe commented Oct 15, 2024

Description

This PR fixes the sorting of retried work orders. It also prevents from loading dataclips to memory once each of them can have MB magnitude and a retry might enqueue thousands of work orders.

Closes #2582

Validation steps

Run the test case from the error multiple times and check that the code now has a order_by.

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed a self-review of my code.
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

@jyeshe jyeshe force-pushed the fix-sort-workorder branch from 73e196b to 40229bb Compare October 15, 2024 12:09
@jyeshe jyeshe changed the title Fix retry sorting and avoids loading dataclips Fix work order retry sorting and avoids loading dataclips Oct 15, 2024
@jyeshe jyeshe force-pushed the fix-sort-workorder branch from 40229bb to 8d41cad Compare October 15, 2024 12:12
@jyeshe jyeshe self-assigned this Oct 15, 2024
@jyeshe jyeshe marked this pull request as ready for review October 15, 2024 12:13
@jyeshe jyeshe requested a review from stuartc October 15, 2024 12:16
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.48%. Comparing base (8c9b9b0) to head (8d41cad).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2581      +/-   ##
==========================================
+ Coverage   90.44%   90.48%   +0.03%     
==========================================
  Files         316      316              
  Lines       10994    10994              
==========================================
+ Hits         9944     9948       +4     
+ Misses       1050     1046       -4     

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

@jyeshe jyeshe requested a review from elias-ba October 15, 2024 12:16
Copy link
Member

@stuartc stuartc left a comment

Choose a reason for hiding this comment

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

Nice one 🙌 Fyi, you don't need to avoid preloading Dataclips - by default the body is not returned in any preload for data clips, so the size of the records that come down is no bigger than any other table. But yeah this is a nice change, easier to understand too. 👍

@stuartc stuartc merged commit c35b196 into main Oct 16, 2024
8 checks passed
@stuartc stuartc deleted the fix-sort-workorder branch October 16, 2024 10:17
@jyeshe
Copy link
Member Author

jyeshe commented Oct 21, 2024

Nice one 🙌 Fyi, you don't need to avoid preloading Dataclips - by default the body is not returned in any preload for data clips, so the size of the records that come down is no bigger than any other table. But yeah this is a nice change, easier to understand too. 👍

field :body, :map, load_in_query: false 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Retrying work orders doesn't respect the order of existing/base ones
2 participants