Skip to content

Add sorting foundation as util and using it within indexed-db-handler #2773

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

Merged
merged 18 commits into from
May 13, 2025

Conversation

calcaide
Copy link
Collaborator

@calcaide calcaide commented Apr 11, 2025

Description

This pull request adds the ability to specify sorting options when querying the store:

this.store.query('target', { sort: { attribute: 'id', direction: 'desc' }});

🎟️ Jira ticket

How to Test

  • Run tests within addons/api

@calcaide calcaide self-assigned this Apr 11, 2025
Copy link

vercel bot commented Apr 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
boundary-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 13, 2025 6:53pm
boundary-ui-desktop ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 13, 2025 6:53pm

Copy link
Collaborator

@hashicc hashicc left a comment

Choose a reason for hiding this comment

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

🕺 This looks great, a few minor comments, I think continuing this pattern for the other types with tests is all that's really left

lisbet-alvarez
lisbet-alvarez previously approved these changes May 9, 2025
Copy link
Collaborator

@lisbet-alvarez lisbet-alvarez left a comment

Choose a reason for hiding this comment

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

Looks great!! Just left a couple of suggestions

Use the schema for a stricter lookup of existing attribute. The two
value exceptions currently supported are created_time and id.
created_time exists on every record from the api but may not be defined
on the model. id exists on every record but is not considered an
attribute by json api record definition
When sorting on an attribute it should be an attribute explicitly
defined on the mode, or an id or created_time
Copy link
Collaborator

@lisbet-alvarez lisbet-alvarez left a comment

Choose a reason for hiding this comment

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

Looks great!! Awesome work!

Just left one question.

Copy link
Collaborator

@hashicc hashicc left a comment

Choose a reason for hiding this comment

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

Although I picked up the PR for some tweaks I am going to approve it as is. The need for a custom sorting or multi-sort might be necessary in a follow-up PR

Copy link
Collaborator

@ZedLi ZedLi left a comment

Choose a reason for hiding this comment

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

Looks good!

@hashicc hashicc merged commit b0d26fe into main May 13, 2025
21 checks passed
@hashicc hashicc deleted the feat-icu-16781-sorting-foundation branch May 13, 2025 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants