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

Sort open orders by start time ASC order #49

Conversation

JohnPhamous
Copy link
Contributor

Users want to see orders that are closest to them in time which is start_time. The current sort order is based on created_at which is when the order was created. Users don't care about that.

Users want to see orders that are closest to them in time which is `start_time`. The current sort order is based on `created_at` which is when the order was created. Users don't care about that.
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Changed the default order sorting from creation time to start time, prioritizing orders closest to the present time for better user relevance.

  • Modified /src/lib/orders/index.tsx to add sort_by='start_time' and sort_direction='ASC' parameters in orders list command
  • Potential issue: Redundant sorting occurring both at API level and client-side could cause performance overhead
  • Consider removing client-side sorting since API already handles it
  • Recommend adding documentation for sorting behavior in command help text
  • Verify sort direction 'ASC' shows soonest orders first as intended

💡 (4/5) You can add custom instructions or style guidelines for the bot here!

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@JohnPhamous JohnPhamous merged commit 3090723 into main Jan 7, 2025
1 check passed
@sigmachirality sigmachirality deleted the johnphamous/eng-1089-change-default-sort-to-soonest-orders-in-cli branch February 15, 2025 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant