[#629] Added in range query for pagination for bbs and tps get accounts api #630
Conversation
There was a problem hiding this comment.
Thanks @bsdworkin, this looks close! I left a few comments below with some issues I noticed. Let me know if you have any questions about the intended functionality or how to test.
| options.SetLimit(int64(limit)) | ||
| options.SetSkip(int64(offset)) | ||
| } else { | ||
| filter = bson.M{"_id": bson.M{"$lt": rangeIndex}} |
There was a problem hiding this comment.
Since we are sorting the documents in ascending order I think you want to use $gt here
| searchParams["app_org_id"] = appOrgIDs | ||
| filter = sa.getFilterForParams(searchParams) |
There was a problem hiding this comment.
We need to apply the search params to the filter in both cases here. With the current setup it will ignore the query filters if you attempt to do pagination using the new method.
| @@ -1188,7 +1188,7 @@ func (sa *Adapter) FindAccounts(context TransactionContext, limit *int, offset * | |||
| } | |||
|
|
|||
| // FindAccountsByParams finds accounts by an arbitrary set of search params | |||
| func (sa *Adapter) FindAccountsByParams(searchParams map[string]interface{}, appID string, orgID string, limit int, offset int, allAccess bool, approvedKeys []string) ([]map[string]interface{}, error) { | |||
| func (sa *Adapter) FindAccountsByParams(searchParams map[string]interface{}, appID string, orgID string, limit int, offset int, allAccess bool, approvedKeys []string, rangeIndex string) ([]map[string]interface{}, error) { | |||
There was a problem hiding this comment.
I think the name rangeIndex is somewhat unclear. I would suggest something like pageStartID or something similar to make it clear that this value is the ID of the item that marks the start of the next page.
| @@ -41,6 +41,14 @@ post: | |||
| explode: false | |||
| schema: | |||
| type: integer | |||
| - name: range_index | |||
There was a problem hiding this comment.
I think the name range_index is somewhat unclear. I would suggest something like page_start_id or something similar to make it clear that this value is the ID of the item that marks the start of the next page (first id not included in the next page). In the description we should also indicate that this should be the id of the last item that was retrieved on the previous page.
Description
Please provide a summary of the pull request and the issue it resolves. Please add necessary details, context, dependencies, explanation of when review is needed (see next section), etc.
**Resolves #629 **
Review Time Estimate
Please give your idea of how soon this pull request needs to be reviewed by selecting one of the options below. This can be based on the criticality of the issue at hand and/or other relevant factors.
Type of changes
Please select a relevant option:
Checklist:
Please select all applicable options: