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

REFACTOR: support custom order by column #1668

Merged
merged 2 commits into from
Jul 9, 2024
Merged

Conversation

ycdesu
Copy link
Collaborator

@ycdesu ycdesu commented Jul 9, 2024

Now user is able to specify the ordering column.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.85%. Comparing base (396ee68) to head (8a3ef17).
Report is 16 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1668      +/-   ##
==========================================
+ Coverage   22.77%   22.85%   +0.08%     
==========================================
  Files         622      621       -1     
  Lines       44809    44857      +48     
==========================================
+ Hits        10204    10254      +50     
+ Misses      33865    33851      -14     
- Partials      740      752      +12     
Files Coverage Δ
pkg/service/trade.go 30.43% <100.00%> (+8.71%) ⬆️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be27d32...8a3ef17. Read the comment docs.

@@ -304,11 +309,24 @@ func (s *TradeService) Query(options QueryTradesOptions) ([]types.Trade, error)
sel = sel.Where(sq.Eq{"exchange": options.Sessions})
}

orderByColumn := "traded_at"
if options.OrderByColumn != "" {
if options.OrderByColumn != "traded_at" && options.OrderByColumn != "gid" {
Copy link
Owner

Choose a reason for hiding this comment

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

using switch case might be better here?

sel = sel.OrderBy("traded_at " + options.Ordering)
} else {
sel = sel.OrderBy("traded_at ASC")
if options.Ordering != "ASC" && options.Ordering != "DESC" {
Copy link
Owner

Choose a reason for hiding this comment

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

ditto

@c9s c9s changed the title refactor: support custom order by column REFACTOR: support custom order by column Jul 9, 2024
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 297 BBG

@ycdesu ycdesu merged commit 015c5fc into main Jul 9, 2024
2 of 3 checks passed
@ycdesu ycdesu deleted the yc/custom-orderby-column branch July 9, 2024 09:46
@bbgokarma-bot
Copy link

Hi @ycdesu,

Well done! 302 BBG has been sent to your polygon wallet. Please check the following tx:

https://polygonscan.com/tx/0x6c0ca1cfbdd7ee27e52a41f0cb41377dafe2302010f04960849a475df87dfdb8

Thank you for your contribution!

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.

4 participants