Paginator sortMap. #18898

dereuromark · 2025-09-09T12:50:36Z

Resolves #18897

Feel free to further adjust.

josbeir · 2025-09-09T12:58:47Z

Looks good, should we handle what i mentioned here: #18897 (comment)

(sharing the same field name on multiple sortmaps)

dereuromark · 2025-09-09T13:08:36Z

I don't think we need validation to prevent "overlapping" fields since each mapped key represents a complete, independent sorting strategy. This is actually a feature - it lets you create multiple sort options that include
the same fields in different combinations.

The concern about ambiguity is resolved by the design:

No ambiguity exists because direct field sorting is disabled when sortMap is defined
Each sort key (item1, item2) is self-contained and independent
The question "In what direction should title be sorted?" doesn't arise because title itself is not a valid sort key

Copilot

Pull Request Overview

This PR introduces a new sortMap feature to the NumericPaginator class that allows creating friendly sort keys that map to one or more actual database fields. The implementation provides advanced sorting capabilities including simple 1:1 mappings, multi-column sorting, fixed direction sorting, and shorthand syntax support.

Key changes include:

Added sortMap configuration option to allow mapping user-friendly sort keys to database fields
Implemented support for multiple sorting patterns including simple mapping, multi-column sorting, and fixed directions
Enhanced test coverage with comprehensive test cases for all sortMap functionality

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/Datasource/Paging/NumericPaginator.php	Added sortMap configuration and resolveSortMapping method to handle mapped sorting logic
tests/TestCase/Datasource/Paging/NumericPaginatorTest.php	Added comprehensive test cases covering all sortMap functionality scenarios
tests/TestCase/Datasource/Paging/PaginatorTestTrait.php	Updated test expectations to include sortMap null default in merged options

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-09T13:14:48Z

src/Datasource/Paging/NumericPaginator.php

+            // Check sortMap first for mapped sorting
+            if (isset($options['sortMap'])) {
+                $mappedOrder = $this->resolveSortMapping($options['sort'], $options['sortMap'], $direction);
+                if ($mappedOrder !== null) {
+                    // Use mapped order and merge with existing order
+                    $existingOrder = isset($options['order']) && is_array($options['order']) ? $options['order'] : [];
+                    $options['order'] = $mappedOrder + $existingOrder;
+                } else {
+                    // Sort key not in sortMap, clear sort
+                    $options['order'] = [];
+                    $options['sort'] = null;
+                    unset($options['direction']);
+
+                    return $options;
+                }
+            } else {


[nitpick] The logic for handling sortMap vs traditional sorting creates nested conditional blocks that reduce readability. Consider extracting the sortMap handling into a separate method to improve code organization and maintainability.

josbeir · 2025-09-09T13:17:42Z

I don't think we need validation to prevent "overlapping" fields since each mapped key represents a complete, independent sorting strategy. This is actually a feature - it lets you create multiple sort options that include the same fields in different combinations.

The concern about ambiguity is resolved by the design:
* No ambiguity exists because direct field sorting is disabled when sortMap is defined

* Each sort key (item1, item2) is self-contained and independent

* The question "In what direction should title be sorted?" doesn't arise because title itself is not a valid sort key

Hmm, i'm probably not understanding it completely but

'sortMap' => [
  'item1' => ['title', 'created']
  'item2' => ['title', 'modified']
]

// sorting on both definitions: 
&item1=asc&item2=desc

Translates into the following statement right ?

SORT BY
   title ASC
   created ASC,
   title DESC,
   modified DESC

ADmad · 2025-09-09T15:10:09Z

&item1=asc&item2=desc

The paginatorhelper doesn't generate query strings like this. The direction can be specific only for 1 field, sort=item1&dir=asc

dereuromark · 2025-09-09T16:29:00Z

Imo we dont even need even sort asc/desc if we bind that into the key.

sort=title-asc
sort=title-desc

A flat list with all possible options

ADmad · 2025-09-09T16:56:33Z

Imo we dont even need even sort asc/desc if we bind that into the key.

Yes that could be an additional enhancement, though what I would really like is the ability to do multi field/colum sort. The query string for it could be like sort[f1]=asc&sort[f2]=desc

josbeir · 2025-09-09T17:41:13Z

@ADmad I think that kind of sort style is not very readable for the end user. This is something Drupal (facets/views) does and it is constantly a fight to get right for proper SEO

Maybe something like /products?sort=price:asc,rating:desc would be more friendly ?
or /products?sort=+price,-rating

With that said, we should of course be weary not to go back to Cake 1-2.x style query params. Good times... 🫠

Another thought that crosses my mind is then making sure that it is always returned in the same order to keep a canonical structure.

ADmad · 2025-09-09T20:16:42Z

I think that kind of sort style is not very readable for the end user

The average non-techie user doesn't read/understand query string params :)

This is something Drupal (facets/views) does and it is constantly a fight to get right for proper SEO

I am not knowledgeable enough about how search engines deal with query string these days to be able to comment on this.

Maybe something like /products?sort=price:asc,rating:desc would be more friendly

This could work I guess.

BTW if for e.g. you have a search form with a multi-value field and using PRG you will get foo[] in the query string :)

markstory · 2025-09-10T02:55:11Z

Translates into the following statement right ?

Yes. You ask a bad question, you get a bad answer. The SQL you posted is syntactically correct, and is what I would expect to happen if all of those ordering clauses were combined.

A flat list with all possible options

This could work but we'll need to make PaginatorHelper aware of how to change direction. How would application code express that field-asc and field-desc are related and opposite directions?

With that said, we should of course be weary not to go back to Cake 1-2.x style query params. Good times... 🫠

No thank you 😄

markstory · 2025-09-10T02:56:16Z

src/Datasource/Paging/NumericPaginator.php

+     * - `sortMap` - A map of sort keys to their corresponding database fields. Allows
+     *   creating friendly sort keys that map to one or more actual fields. When defined,
+     *   only the mapped keys will be sortable. Supports simple mapping, multi-column
+     *   sorting, and fixed direction sorting. You can also use numeric arrays for 1:1
+     *   mappings where the field name is the same as the sort key. Example:


This has some overlap with sortableFields should we keep both long term?

sortableFields does become redundant with the addition of sortMap, so it could be deprecated.

Yes, I think we can make this into something that is just as simple to use and handles more advanced use cases as well.

Isnt it already now? :)

dereuromark · 2025-09-10T13:26:32Z

I added a demo of combined sort & dir.
Wonder if that would work out better.

dereuromark · 2025-09-11T10:25:32Z

For me it looks quite complete.

What's Missing:

A way to specify default directions that can still be toggled. For example:

Default: title ASC, created DESC
When toggled: title DESC, created ASC

Potential Solution:

We could enhance the syntax to support default directions that flip together:

  'sortMap' => [
      // New syntax: use '@' prefix for toggleable fields with defaults
      'newest' => [
          '@title' => 'asc',     // Default asc, flips to desc
          '@created' => 'desc',  // Default desc, flips to asc
      ],
  ]

When user clicks the sort link:

First click: title ASC, created DESC (defaults)
Second click: title DESC, created ASC (inverted)

We could also invert this and make the @ signal that this is hardcoding the direction, as this might be more rare used.
Or, thinking about it (and in line of other languages, e.g. CSS),

  'newest' => [
      'title' => 'asc!',     // Only asc
      'created' => 'desc',  // Default desc, flips to asc
  ],

Use ! to signal that this is hardcoded in that direction.

josbeir · 2025-09-11T18:25:56Z

For me it looks quite complete.

What's Missing:

A way to specify default directions that can still be toggled. For example:
* Default: title ASC, created DESC

* When toggled: title DESC, created ASC
Potential Solution:

We could enhance the syntax to support default directions that flip together:
  'sortMap' => [
      // New syntax: use '@' prefix for toggleable fields with defaults
      'newest' => [
          '@title' => 'asc',     // Default asc, flips to desc
          '@created' => 'desc',  // Default desc, flips to asc
      ],
  ]
When user clicks the sort link:
* First click: title ASC, created DESC (defaults)

* Second click: title DESC, created ASC (inverted)
We could also invert this and make the @ signal that this is hardcoding the direction, as this might be more rare used. Or, thinking about it (and in line of other languages, e.g. CSS),
  'newest' => [
      'title' => 'asc!',     // Only asc
      'created' => 'desc',  // Default desc, flips to asc
  ],
Use ! to signal that this is hardcoded in that direction.

I like the idea!

rochamarcelo · 2025-09-12T11:31:53Z

Could we have classes to define how a field should be ordered instead of a new syntax?

class OrderField 
{
  
    public function defaultDir(): ?string
    {
    }

    public function isFixedDir(): bool
    {
    }

    public function ormColumn(): array
}

dereuromark · 2025-09-12T11:48:11Z

As opt-in? I feel like as default this could become quite some overhead for apps to create.

dereuromark · 2025-09-12T12:19:06Z

or do you mean sth like

 namespace Cake\Datasource\Paging;

  class SortField 
  {
      protected string $field;
      protected ?string $defaultDirection;
      protected bool $locked;

      public function __construct(string $field, ?string $defaultDirection = null, bool $locked = false)
      {
          $this->field = $field;
          $this->defaultDirection = $defaultDirection;
          $this->locked = $locked;
      }

      public static function asc(string $field): self
      {
          return new self($field, 'asc', false);
      }

      public static function desc(string $field): self
      {
          return new self($field, 'desc', false);
      }

      public static function locked(string $field, string $direction): self
      {
          return new self($field, $direction, true);
      }

      public function getField(): string
      {
          return $this->field;
      }

      public function getDirection(string $requestedDirection, bool $directionSpecified): string
      {
          if ($this->locked) {
              return $this->defaultDirection;
          }

          if (!$directionSpecified && $this->defaultDirection) {
              return $this->defaultDirection;
          }

          return $requestedDirection;
      }

      public function isLocked(): bool
      {
          return $this->locked;
      }
  }

Usage Examples:

  'sortMap' => [
      'newest' => [
          SortField::desc('created'),  // Default desc, toggleable
          SortField::asc('title'),     // Default asc, toggleable
      ],
      'popular' => [
          SortField::locked('score', 'desc'),  // Always desc
          'author',  // Still support strings for BC
      ],
  ]

rochamarcelo · 2025-09-12T13:09:12Z

Yes, I like that. Maybe add an interface for it with methods getField and isLocked.

I'm not sure if the methods desc and asc are necessary, but the names could be clear, something like defaultDesc and defaultAsc.

josbeir · 2025-09-12T15:55:45Z

We could maybe also create factory class for building the sortmap then. This would allow for a more stric 6284 t and readable way for defining them.

$factory = SortMapFactory::create()
    ->field('title')
    ->field('created')
    ->field('rank', locked: true, default: 'desc')
    ->combo('newest', [
        SortField::desc('created'),
        SortField::asc('title'),
    ])
    ->combo('popular', [
        SortField::desc('score'),
        SortField::asc('comments_count'),
    ])
    ->combo('recently-updated', [
        SortField::desc('modified'),
        SortField::asc('title'),
    ])
    ->raw('alpha-group', [
        'group_name' => 'asc',
        'name' => 'asc',
    ]);

dereuromark · 2025-09-14T10:49:51Z

I added factory and SortField now with tests.
Also deprecated old sortableFields now.

markstory · 2025-09-16T03:09:26Z

src/Datasource/Paging/NumericPaginator.php

+                if ($value instanceof SortField) {
+                    $field = $value->getField();
+                    $fieldDirection = $value->getDirection($direction, $directionSpecified);
+                    $order[$field] = $fieldDirection;
+                } elseif (is_int($key)) {
+                    // Indexed array: field uses querystring direction
+                    // e.g., ['modified', 'name']
+                    $order[$value] = $direction;
+                } elseif (str_ends_with($value, '!')) {
+                    // Associative array: check for locked (!) or default direction
+                    // Locked direction (ends with !): always use specified direction
+                    // e.g., ['created' => 'desc!'] always sorts desc
+                    $order[$key] = rtrim($value, '!');
+                } elseif (!$directionSpecified) {
+                    // Default direction that can be toggled
+                    // No direction specified, use the default
+                    $order[$key] = $value;
+                } else {
+                    // Direction specified, use it for all toggleable fields
+                    $order[$key] = $direction;
+                }


It looks like we have both the data-object pattern, and array shaped data format. Can we pick one API? This is net new code, we shouldn't start off with two different APIs.

To me the data-object pattern is easier to operate as I can use LSP to explore it more easily. It is also less ambiguous and easier to evolve and extend.

I am fine with only factories and objects.
We can then remove all array configs.

markstory · 2025-09-16T03:10:07Z

src/Datasource/Paging/SortField.php

+/**
+ * Represents a sort field configuration for pagination.
+ */
+class SortField


I think classes like this are a great way for use to provide stronger typing and easier to operate and remember APIs.

markstory · 2025-09-16T03:14:53Z

src/View/Helper/PaginatorHelper.php

+        // Check if we should use combined format
+        $sortFormat = $this->getConfig('options.sortFormat', 'separate');
+        if ($sortFormat === 'combined') {
+            // Use combined format: field-asc or field-desc
+            $paging = ['sort' => $key . '-' . $dir, 'page' => 1];
+        } else {
+            // Use traditional separate format
+            $paging = ['sort' => $key, 'direction' => $dir, 'page' => 1];
+        }


Do we need this if we're introducing SortField?

markstory · 2025-09-16T03:16:23Z

tests/TestCase/Datasource/Paging/SortMapFactoryTest.php

+        $sortMap = SortMapFactory::create()
+            ->sortKey('newest')
+                ->desc('created')
+                ->asc('title')
+            ->sortKey('oldest')
+                ->asc('created')
+                ->asc('title')
+            ->sortKey('popular')
+                ->locked('score', SortField::DESC)
+                ->desc('views')
+            ->sortKey('alphabetical')
+                ->asc('name')
+            ->build();


Do we need two different builder patterns? Can we pick one instead.

Which ones u prefer?

i like the SortMapFactory,, i'm just not sure about the fluent style of building. This is something that is not used in other cake api's i think and seems a bit too locked in.

I'd additionally love to see something that would expose an instance of the factory through a callable, this way you would not need to call ->build()

'sortMap' => function(Builder $builder) { return $builder ->key('newest', SortField::desc('title'), SortField::asc('title')) ->key('oldest', SortField::asc('created'), SortField::asc('title')) ->key('popular', SortField::desc('score', locked: true), SortField::desc('view'), .... .... ); });

i would not use the word 'sort' (sortKey) in method names, This seems a bit redundant given the Factory shares the same name :-)

Maybe we should just use use 'sorts as key to define the sortMap ?

Which ones u prefer?

I find the factory easier to understand when reading code. The fluent builder is interesting, but the method chains are dense, and I could see a stray sortKey() call causing unexpected results.

So only keeping SortField?
I also like the idea of SortMapFactory and maybe a callable support.

Do you want to make a PR with the final showcase/api of this targeting this one?

I believe we should decide on one of the following options:

SortField only

Fluent Builder + SortField (as Enum/Consts - SortField::DESC)

Factory builder + typed SortField (SortField::asc('fieldName')

Same as 3 + callable shortcut; also opens up options for clean organization

In case of factory builder: do pass sortFields as an array or as variadic parameters.

Personally i'd go for option 4 + variadic

Here is my idea #18924

Sample code

$map = (new SortMap()) ->push( C852 'active', new SortField("active"), "name", SortField::locked("created", SortField::DESC), ) ->push('title') ->push("group", "Groups.name");

Let me know what you think.

@josbeir Do you want to make a PR towards here with your suggested improvements?

@josbeir Do you want to make a PR towards here with your suggested improvements?

I'll try to take a shot at it in the coming days

…ctory - Rename 'sortMap' configuration option to 'sorts' - Rename SortMapFactory to SortsFactory with non-fluent API - Remove SortField::locked() method, add $locked parameter to asc()/desc() - Add callable 'sorts' option that receives SortsFactory instance The new API supports: ```php 'sorts' => function($factory) { return $factory ->add('name', SortField::asc('Users.name')) ->add('popularity', SortField::desc('score', locked: true)); } ```

josbeir · 2025-10-10T19:11:03Z

Updated with following changes:

Rename sortMap → sorts configuration option
Rename SortMapFactory → SortsFactory with simplified non-fluent API
Replace SortField::locked() with parameter: SortField::desc('field', locked: true)
Add callable sorts option with add() method accepting unlimited SortField arguments
Maintain backward compatibility with deprecation warnings

The current thought i have: do we keep SortFieldFactory, SortsFactory or both. I think they both could be useful.

Current implementation covers:

Basic:

$settings = [
    'sorts' => [
        'User.id', // Use field as is.
        'name' => 'Users.name',
        'email' => 'Users.email',
        'newest' => [
            'created' => 'desc!',  // Always DESC - locked
            'title' => 'asc',      // Default ASC, but toggleable
        ],
        'created' => [
           'Users.created',
           'User.name'
         ],
   ]
];

SortField Objects:

$settings = [
    'sorts' => [
        'newest' => [
            SortField::desc('created'),  // Default desc, toggleable
            SortField::asc('title'),     // Secondary sort
        ],
        'popular' => [
            SortField::desc('score', locked: true), // Always desc
            'author',  // Still support strings for BC
        ],
        'alphabetical' => SortField::asc('title'), // Single field
        'priority' => SortField::desc('priority', locked: true), // Locked
    ]
];

Factory builder

// Simple
$settings = [
    'sorts' => [
        'relevance' => SortFieldFactory::create()
            ->desc('search_score', locked: true)  // Always desc
            ->desc('popularity')                  // Default desc, toggleable
            ->asc('title')                       // Tiebreaker
            ->build(),
            
        'price_low' => SortFieldFactory::create()
            ->asc('price', locked: true)         // Always ascending
            ->asc('title')                       // Tiebreaker
            ->build(),
            
        'price_high' => SortFieldFactory::create()
            ->desc('price', locked: true)        // Always descending
            ->asc('title')                       // Tiebreaker
            ->build(),
    ]
];

// Instance
$factory = new SortsFactory();
$factory->add('newest', SortField::desc('published'), SortField::asc('title'));
$factory->add('popular', SortField::desc('views', locked: true), SortField::desc('comments'));
$factory->add('alphabetical', SortField::asc('title'));
$factory->add('author');  // Shorthand: creates 'author' => ['author']

$settings = [
    'sorts' => $factory->build()
];

// Callable with new instance, should return instance w/o build() method
$settings = [
    'sorts' => function(SortsFactory $factory) {
        return $factory
            ->add('newest', 
                SortField::desc('created'), 
                SortField::asc('title')
            )
            ->add('popular', 
                SortField::desc('score', locked: true),
                SortField::desc('comments')
            )
            ->add('alphabetical', SortField::asc('title'));
    }
];

…rt-map

src/Datasource/Paging/NumericPaginator.php

ADmad · 2025-10-11T08:55:53Z

The existing tests cases using the deprecated sortableFields option will need to be wrapped in $this->deprecated(..) to fix CI.

Since the new sorts option also supports a list of fields as used by sortableFields, couldn't be add the new feature using the same option name sortableFields? That way existing apps won't require any change and one can update sortableFields are required to take advance of the new "sort field" field feature. Option name sorts sounds a bit weird to me.

ADmad · 2025-10-11T09:01:00Z

src/Datasource/Paging/SortsFactory.php

+ *
+ * Provides non-fluent interface for building sorts with multiple sort keys and fields.
+ */
+class SortsFactory


Not too thrilled about this class name. Assuming we continue using the option name sortableFields as I suggested in another comment, this class could be named SortableFieldsFactory.

More nitpick, should we use the Builder suffix instead of Factory, to be consistent with other classes like RoutesBuidler, ViewBuilder, CorsBuilder, FunctionsBuilder.

Added to plan

josbeir · 2025-10-11T09:06:23Z

The existing tests cases using the deprecated sortableFields option will need to be wrapped in $this->deprecated(..) to fix CI.

Since the new sorts option also supports a list of fields as used by sortableFields, couldn't be add the new feature using the same option name sortableFields? That way existing apps won't require any change and one can update sortableFields are required to take advance of the new "sort field" field feature. Option name sorts sounds a bit weird to me.

So, current plan:

Merge new C852 functionality into existing sortableFields option
Refactor SortsFactory to SortableFieldsBuilder
Refactor SortFieldFactory to SortFieldBuilder

No need for $this->deprecated(..) in tests then :-)

Things to keep in mind:

Paginator sortMap. #18898 (comment) (only use OOP for building)
Paginator sortMap. #18898 (comment) (do we need 2 different Factories)
Paginator sortMap. #18898 (comment) (do we need configurable options.sortFormat)

ADmad · 2025-10-11T09:12:53Z

So new plan:

Merge new functionality into existing sortableFields option
Refactor SortsFactory to SortableFieldsFactory

No need for $this->deprecated(..) in tests then :-)

Works for me (with SortableFieldsBuilder), but please wait for feedback from others :)

rochamarcelo · 2025-10-11T12:15:57Z

Should I close #18924 ?

What do you think about the changes in there?

dereuromark · 2025-10-11T12:25:59Z

Builder sounds also fine to me.

josbeir · 2025-10-11T12:57:13Z

Should I close #18924 ?

What do you think about the changes in there?

@rochamarcelo I think the new SortsFactory behaves exactly the same way as your idea now, The difference is more in the way the SortField class is used. I personally like the explicitness of how it is implemented now:

SortField::desc('score', locked: true),
SortField::asc('comments')

Paginator sortMap.

1d06b06

dereuromark added this to the 5.3.0 milestone Sep 9, 2025

dereuromark added the enhancement label Sep 9, 2025

ADmad requested a review from Copilot September 9, 2025 13:13

Copilot AI reviewed Sep 9, 2025

View reviewed changes

markstory reviewed Sep 10, 2025

View reviewed changes

Showcasing combined sort-dir.

2a54105

Use ! for locking.

7eb19a3

Add factory and object approach.

b548754

dereuromark added 3 commits September 14, 2025 13:11

Add factory and object approach.

1989178

Add factory and object approach.

3bfc2dd

Add factory and object approach.

3e5d264

Merge branch '5.next' into 5.next-paginator-sort-map

2955d16

markstory reviewed Sep 16, 2025

View reviewed changes

josbeir added 3 commits October 10, 2025 20:46

Cs fix

e0ed28e

More cs fixes

6479811

Merge remote-tracking branch 'origin/5.next' into 5.next-paginator-so…

140e00c

…rt-map

ADmad reviewed Oct 11, 2025

View reviewed changes

src/Datasource/Paging/NumericPaginator.php Show resolved Hide resolved

ADmad reviewed Oct 11, 2025

View reviewed changes

Paginator sortMap. #18898

Are you sure you want to change the base?

Paginator sortMap. #18898

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Current implementation covers:

Uh oh!