Skip to content

Conversation

itslenny
Copy link
Contributor

@itslenny itslenny commented Aug 14, 2025

What kind of change does this PR introduce?

Feature

What is the current behavior?

The list v2 endpoint does not support sorting like the old list endpoint

What is the new behavior?

  • Support sorting by name, created_at, and updated_at to parity the v1 list endpoint
  • Fix pagination issue when objects and folders have similar names
  • Add robust test coverage to ensure pagination and sorting work perfectly in all scenarios

@coveralls
Copy link

coveralls commented Aug 14, 2025

Pull Request Test Coverage Report for Build 17679481788

Details

  • 137 of 139 (98.56%) changed or added relevant lines in 5 files are covered.
  • 13 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.5%) to 76.473%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/storage/object.ts 70 72 97.22%
Files with Coverage Reduction New Missed Lines %
src/storage/storage.ts 2 71.27%
src/storage/object.ts 11 91.05%
Totals Coverage Status
Change from base Build 17552349119: 0.5%
Covered Lines: 21136
Relevant Lines: 27403

💛 - Coveralls

@itslenny itslenny force-pushed the feat/add-support-for-sorting-to-list-v2 branch from 24b869c to 251558f Compare September 8, 2025 23:28
@itslenny itslenny force-pushed the feat/add-support-for-sorting-to-list-v2 branch from 251558f to e7ec3a0 Compare September 10, 2025 22:45
@itslenny itslenny marked this pull request as ready for review September 10, 2025 23:02
@itslenny itslenny force-pushed the feat/add-support-for-sorting-to-list-v2 branch from e7ec3a0 to c7d468f Compare September 10, 2025 23:16
@itslenny itslenny force-pushed the feat/add-support-for-sorting-to-list-v2 branch from c7d468f to 542373e Compare September 10, 2025 23:21
@@ -269,27 +269,55 @@ export class StorageKnexDB implements Database {
nextToken?: string
maxKeys?: number
startAfter?: string
sortOrder?: string
Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding the order in the sortBy object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. I originally had it outside because it sorts by name even if sortBy is undefined so this allowed setting asc/desc without including the sortBy object, but having it all grouped together probably makes more sense.

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.

3 participants