From d42b3eb0b17a55b6691e7b064552bc317f367fb8 Mon Sep 17 00:00:00 2001
From: Andre Klapper <a9016009@gmx.de>
Date: Thu, 4 Jan 2024 10:16:15 +0100
Subject: [PATCH] Fix "Map omits required key" exception by comparing result
 order against modern field keys

Summary:
Sorting a Maniphest search query by custom fields throws a "Map returned omits required key" exception.

The `isCustomFieldOrderKey()` check still tested against legacy field key format (for example `[custom:]std:maniphest:deadline.due`) while the code passes modern field key format (for example `custom.deadline.due`).

After fixing this, `PhutilTypeSpec::checkMap()` throws an exception when a non-optional (extra) key `$column` is in `$columns` but not in the array of type parameters below to check against:
`"Got unexpected parameters: customfield, customfield.index.table, customfield.index.key"`
Thus add optional types for customfields in `buildPagingClauseFromMultipleColumns()` to allow them instead of throwing another exception.

Closes T15631

Test Plan:
* Have a custom field (with search: true) defined via http://phorge.localhost/config/edit/maniphest.custom-field-definitions/
* Have two tasks have the custom field set
* Go to http://phorge.localhost/maniphest/query/ , select the custom field (e.g. "Due Date (Reversed)") in the "Order By" field, and click the "Search" button. Iterate through the results via the "Next" button if needed

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15631

Differential Revision: https://we.phorge.it/D25504
---
 .../policy/PhabricatorCursorPagedPolicyAwareQuery.php      | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
index 6ad91b52ea..cdd941a5f4 100644
--- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
@@ -735,6 +735,9 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
         array(
           'table' => 'optional string|null',
           'column' => 'string',
+          'customfield' => 'optional bool',
+          'customfield.index.key' => 'optional string',
+          'customfield.index.table' => 'optional string',
           'value' => 'wild',
           'type' => 'string',
           'reverse' => 'optional bool',
@@ -1747,7 +1750,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
 
     $map = array();
     foreach ($fields->getFields() as $field) {
-      $map['custom:'.$field->getFieldKey()] = $field->getValueForStorage();
+      $map[$field->getModernFieldKey()] = $field->getValueForStorage();
     }
 
     return $map;
@@ -1758,7 +1761,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
    * @task customfield
    */
   protected function isCustomFieldOrderKey($key) {
-    $prefix = 'custom:';
+    $prefix = 'custom.';
     return !strncmp($key, $prefix, strlen($prefix));
   }
 
-- 
2.51.2