Add new ordering method allowing ordering by multiple fields #679

diesieben07 · 2024-12-22T20:13:27Z

Add new @strawberry_django.ordering() annotation that supersedes @strawberry_django.order() offering better support for ordering by multiple fields.

This is still a draft, because it is not finished, but I have added it here for feedback.

Description

Currently the behavior is almost exactly the same as for @strawberry_django.order except for the following changes:

Top-level ordering argument is generated as a list instead of an optional input. This allows better control over sorting by multiple fields (for example "sort by name, then by birthdate").
@strawberry_django.ordering() exposes the one_of setting of @strawberry.input() and allows setting a default via the Strawberry Django settings. It is useful to set this to true, because sorting by multiple fields should be given as a list of objects with a single field set each. This way object property order is irrelevant for the sorting.
OrderSequence type and its associated machinery is removed from ordering

Still missing:

Migration path from order to ordering in a schema-compatible way:
This already works, you can just have both parameters, but we should validate that only one of them is set by clients and send a useful error message.
Documentation
Tests for custom order methods when using ordering.

Types of Changes

Issues Fixed or Closed by This PR

Fixes #678.

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Summary by Sourcery

Add ordering field to allow ordering by a list of fields.

New Features:

Added a new @strawberry_django.ordering() annotation to supersede @strawberry_django.order().
The new ordering field takes a list of orderings, allowing for more complex ordering logic such as "sort by name, then by birthdate".
Added one_of setting to @strawberry_django.ordering() to allow defaulting via Django settings.

Tests:

Added tests for the new ordering functionality.

sourcery-ai · 2024-12-22T20:13:32Z

Reviewer's Guide by Sourcery

This pull request introduces a new ordering method, @strawberry_django.ordering(), which enhances sorting by multiple fields. It replaces the existing @strawberry_django.order() annotation. The top-level ordering argument is now a list, enabling more complex sorting scenarios. The one_of setting is exposed for finer control over input structure. While the core functionality is in place, migration guidelines, documentation, and comprehensive tests are still pending.

Sequence diagram for the new ordering process

sequenceDiagram
    participant C as Client
    participant F as Field
    participant O as Ordering Processor
    participant Q as QuerySet

    C->>F: Query with ordering list
    F->>O: process_ordering()
    O->>O: process_ordering_default()
    O->>Q: Apply ordering args
    Q-->>F: Ordered QuerySet
    F-->>C: Ordered results

Class diagram showing the new ordering functionality

classDiagram
    class StrawberryDjangoFieldOrdering {
      +order: type|UnsetType|None
      +ordering: type|UnsetType|None
      +__init__(order, ordering, **kwargs)
      +get_order()
      +get_ordering()
      +get_queryset(queryset, info, order, ordering, **kwargs)
    }

    class StrawberryDjangoSettings {
      +ORDERING_DEFAULT_ONE_OF: bool
    }

    class OrderingDecorator {
      <<function>>
      +model: Model
      +name: str|None
      +one_of: bool|None
      +description: str|None
      +directives: Sequence[object]|None
    }

    StrawberryDjangoFieldOrdering ..> StrawberryDjangoSettings : uses
    OrderingDecorator ..> StrawberryDjangoSettings : uses settings

    note for StrawberryDjangoFieldOrdering "New ordering support added"
    note for OrderingDecorator "New @ordering decorator"
    note for StrawberryDjangoSettings "Added ORDERING_DEFAULT_ONE_OF setting"

File-Level Changes

Change	Details	Files
Introduced new `ordering` argument and associated logic.	Added `ordering` argument to `strawberry_django.fields.field`. Implemented the `process_ordering` function to handle the new ordering logic. Updated `StrawberryDjangoFieldOrdering` to manage the `ordering` argument. Added new tests for the `ordering` functionality. Modified existing tests to use the new `ordering` argument and syntax.	`strawberry_django/fields/field.py` `strawberry_django/ordering.py` `strawberry_django/type.py` `strawberry_django/fields/field.py` `tests/test_ordering.py`
Added new `ordering` decorator and input type.	Created the `ordering` decorator to define ordering input types. Added settings for default `one_of` behavior in ordering inputs. Included tests for the new decorator and settings.	`strawberry_django/ordering.py` `strawberry_django/settings.py` `tests/test_settings.py`
Refactored existing order-related code.	Removed `OrderSequence` type and related code. Simplified ordering processing logic. Updated tests to reflect the code changes.	`strawberry_django/ordering.py` `tests/test_ordering.py`
Added legacy order tests.	Created a new test file specifically for legacy order functionality. Added comprehensive tests covering various scenarios for the old `order` annotation. Ensured that existing order-related tests are preserved and functional alongside the new ordering tests.	`tests/test_legacy_order.py`
Updated test types.	Added `sweetness` field to the `Fruit` type to support new sorting tests. Modified existing types as needed to accommodate the changes in ordering logic and tests.	`tests/types.py`

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#678	Add support for ordering by multiple fields using a list-based API instead of relying on object key order	✅

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time. You can also use
this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

for more information, see https://pre-commit.ci

codecov-commenter · 2024-12-22T20:16:28Z

Codecov Report

Attention: Patch coverage is 94.59459% with 4 lines in your changes missing coverage. Please review.

Project coverage is 89.29%. Comparing base (48b54fd) to head (cb815bd).

Files with missing lines	Patch %	Lines
strawberry_django/ordering.py	94.44%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
+ Coverage   89.17%   89.29%   +0.12%     
==========================================
  Files          41       41              
  Lines        3778     3849      +71     
==========================================
+ Hits         3369     3437      +68     
- Misses        409      412       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Add new ordering method allowing ordering by multiple fields

a721659

[pre-commit.ci] auto fixes from pre-commit.com hooks

cb815bd

for more information, see https://pre-commit.ci

diesieben07 mentioned this pull request Dec 22, 2024

Improve support for ordering by multiple fields #678

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new ordering method allowing ordering by multiple fields #679

Add new ordering method allowing ordering by multiple fields #679

diesieben07 commented Dec 22, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Dec 22, 2024 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

codecov-commenter commented Dec 22, 2024 •

edited

Loading

Add new ordering method allowing ordering by multiple fields #679

Are you sure you want to change the base?

Add new ordering method allowing ordering by multiple fields #679

Conversation

diesieben07 commented Dec 22, 2024 • edited by sourcery-ai bot Loading

Description

Still missing:

Types of Changes

Issues Fixed or Closed by This PR

Checklist

Summary by Sourcery

sourcery-ai bot commented Dec 22, 2024 • edited Loading

Reviewer's Guide by Sourcery

Sequence diagram for the new ordering process

Class diagram showing the new ordering functionality

File-Level Changes

Assessment against linked issues

Interacting with Sourcery

Customizing Your Experience

Getting Help

codecov-commenter commented Dec 22, 2024 • edited Loading

Codecov Report

diesieben07 commented Dec 22, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Dec 22, 2024 •

edited

Loading

codecov-commenter commented Dec 22, 2024 •

edited

Loading