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

[Fix] Sort nulls last for pool publish date #12186

Merged
merged 4 commits into from
Dec 5, 2024
Merged

Conversation

petertgiles
Copy link
Contributor

@petertgiles petertgiles commented Dec 4, 2024

πŸ€– Resolves #11535

πŸ‘‹ Introduction

Sorts publish date with nulls last on the pool table.

πŸ•΅οΈ Details

I just added a new sorting scope to the pool paginated query. It would be nice to generalize this to something we could use throughout the schema but for now it's a stopgap.

The new scope could be used for any physical column in the table but is not advanced enough to handle relationships like Lighthouse's directive can. It also only accepts a single column at a time.

πŸ§ͺ Testing

Frontend

  1. Rebuild and log in as admin@test.com
  2. Navigate to /en/admin/pools
  3. Play with sorting options
  • initial sort is still created date
  • bookmarks is still the overriding sort option
  • sorting by publishing date, ascending and descending, sorts nulls to the end
  • other sorting options unaffected

Backend

  1. Navigate to /admin/graphiql and add an authorization header with a bearer token for admin@test.com
  2. Play with sorting options
query Pools($where:PoolFilterInput, $orderByColumn: OrderByColumnInput) {
  poolsPaginated(first: 100, orderByColumn: $orderByColumn, where:$where) {
    data {
      id      
      publishedAt
    }    
  }
}
-----
{
  "where": {
    "statuses": [
      "DRAFT",
      "PUBLISHED"
    ]
  },
  "orderByColumn": {
    "column": "published_at",
    "order": "DESC",
    "nulls": "ORDER_LAST"
  }
}
  • scope only accepts real column names
  • scope only accepts real options for "order" and "nulls"
  • scope handles undefined "nulls" option
  • scope sorts as expected

πŸ“Έ Screenshot

image

🚚 Deployment

Copy link
Contributor

@vd1992 vd1992 left a comment

Choose a reason for hiding this comment

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

Looks and functions well, aside from linting my only thing to note it we should probably have that bespoke sort covered under some sort of PHP test.

@petertgiles
Copy link
Contributor Author

we should probably have that bespoke sort covered under some sort of PHP test

Good call!
Added in test suite.

Copy link
Contributor

@vd1992 vd1992 left a comment

Choose a reason for hiding this comment

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

Fantastic! Thanks for adding testing

@petertgiles petertgiles added this pull request to the merge queue Dec 5, 2024
Merged via the queue into main with commit c6ad383 Dec 5, 2024
12 checks passed
@petertgiles petertgiles deleted the 11535-sort-nulls-last branch December 5, 2024 17:49
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.

πŸ› When sorting Pools by Publishing Date, null dates should appear at bottom when sorting descending
2 participants