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

order_by in JS client #121

Merged
merged 6 commits into from
Oct 28, 2019
Merged

order_by in JS client #121

merged 6 commits into from
Oct 28, 2019

Conversation

dmdashenkov
Copy link
Contributor

This PR adds a missing feature to the JS client. Now we can specify ordering for query response from the JS library.

Note that ordering is neither restored nor verified by the client itself. A server may choose to ignore it completely, or, as is the case with Firebase, the ordering may be lost when transporting data from server to client.

Since the limit parameter requires presence of order_by, this feature is vital for users who may want to use limit, even if strict ordering is not supported.

@dmdashenkov dmdashenkov self-assigned this Oct 28, 2019
@codecov
Copy link

codecov bot commented Oct 28, 2019

Codecov Report

Merging #121 into master will increase coverage by 0.22%.
The diff coverage is 94.44%.

@@             Coverage Diff              @@
##             master     #121      +/-   ##
============================================
+ Coverage     63.06%   63.29%   +0.22%     
  Complexity      179      179              
============================================
  Files            78       78              
  Lines          1814     1828      +14     
  Branches         37       37              
============================================
+ Hits           1144     1157      +13     
- Misses          659      660       +1     
  Partials         11       11

@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@dmdashenkov LGTM with minor comments to address.

/**
* Requests the query results to be ordered by the given `column` in the descending direction.
*
* Weather or not the results will be sorted in the given order depends on the implementation
Copy link
Contributor

Choose a reason for hiding this comment

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

Whether the results will be sorted in the requested order depends on the implementation of server-side communication.

/**
* Requests the query results to be ordered by the given `column` in the ascending direction.
*
* Weather or not the results will be sorted in the given order depends on the implementation
Copy link
Contributor

Choose a reason for hiding this comment

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

Please see above.

@dmdashenkov dmdashenkov merged commit cd2c4be into master Oct 28, 2019
@dmdashenkov dmdashenkov deleted the allow-query-ordering-from-js branch October 28, 2019 14:40
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.

2 participants