Bugfix: limit and offset params caused re-ordered resource list
#2702
Conversation
john-odonnell
force-pushed
the
johnodon-resources-offset
branch
3 times, most recently
from
e29864f to
35f7efa
Compare
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| ## [1.19.2] - 2022-01-13 | ||
|
|
||
| ### Fixed | ||
| - Previously, including `limit` or `offset` parameters to a resource list request |
There was a problem hiding this comment.
Lists should be surrounded by blank lines
| @@ -114,7 +114,7 @@ def search account: nil, kind: nil, owner: nil, offset: nil, limit: nil, search: | |||
| raise ArgumentError, "'offset' contains an invalid value. 'offset' must be an integer greater than or equal to 0." | |||
| end | |||
|
|
|||
| scope = scope.order(:resource_id).limit( | |||
There was a problem hiding this comment.
This is an excellent find, @john-odonnell!
However, I would suggest that the fix should be to make sure the order is applied to all queries, rather than remove it for all queries (I didn't see another order instance, but I also may have missed it).
It's important that the order be consistent, and if no order is given, postgresql documents that there is no order guarantee.
There was a problem hiding this comment.
Good catch, thanks. I updated the PR to order the resources before limiting based on limit & offset.
# Sort results alphabetically by resource ID
scope = scope.order(:resource_id)
if offset || limit
...
john-odonnell
force-pushed
the
johnodon-resources-offset
branch
from
35f7efa to
5226879
Compare
john-odonnell
force-pushed
the
johnodon-resources-offset
branch
from
5226879 to
357af55
Compare
|
Code Climate has analyzed commit 357af55 and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 90.0% (-1.5% change). View more on Code Climate. |
Desired Outcome
From ONYX-14314:
Implemented Changes
By default, listed resources are ordered by time-of-creation, which means related resources are typically grouped. When
listoroffsetparameters are provided, the listed resources are ordered alphabetically by resource ID, which means resources are grouped by type.I've removed the additional sorting when
listoroffsetparameters are provided, so a limited or offset list will return resources in the expected order.Connected Issue/Story
CyberArk internal issue ID: ONYX-14314
Definition of Done
At least 1 todo must be completed in the sections below for the PR to be
merged.
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
READMEs) were updated in this PRBehavior
Security