Fix sort params

app/Api/V1/Controllers/Controller.php

@@ -67,7 +67,6 @@ abstract class Controller extends BaseController
     protected array $accepts                        = ['application/json', 'application/vnd.api+json'];
 
     /** @var array<int, string> */
-    protected array               $allowedSort;
     protected bool                $convertToPrimary = false;
     protected TransactionCurrency $primaryCurrency;
     protected ParameterBag        $parameters;
@@ -78,7 +77,6 @@ abstract class Controller extends BaseController
     public function __construct()
     {
         // get global parameters
-        $this->allowedSort = config('firefly.allowed_sort_parameters');
         $this->middleware(
             function ($request, $next) {
                 $this->parameters = $this->getParameters();
@@ -150,13 +148,7 @@ abstract class Controller extends BaseController
             }
             if (null !== $value) {
                 $value = (int)$value;
-                if ($value < 1) {
+                $value     = min(max(1, $value), 2 ** 16);
-                    $value = 1;
-                }
-                if ($value > 2 ** 16) {
-                    $value = 2 ** 16;
-                }
-
                 $bag->set($integer, $value);
             }
             if (null === $value
@@ -173,39 +165,8 @@ abstract class Controller extends BaseController
         }
 
         // sort fields:
-        return $this->getSortParameters($bag);
-    }
-
-    private function getSortParameters(ParameterBag $bag): ParameterBag
-    {
-        $sortParameters = [];
-
-        try {
-            $param = (string)request()->query->get('sort');
-        } catch (BadRequestException $e) {
-            Log::error('Request field "sort" contains a non-scalar value. Value set to NULL.');
-            Log::error($e->getMessage());
-            Log::error($e->getTraceAsString());
-            $param = '';
-        }
-        if ('' === $param) {
-            return $bag;
-        }
-        $parts          = explode(',', $param);
-        foreach ($parts as $part) {
-            $part      = trim($part);
-            $direction = 'asc';
-            if ('-' === $part[0]) {
-                $part      = substr($part, 1);
-                $direction = 'desc';
-            }
-            if (in_array($part, $this->allowedSort, true)) {
-                $sortParameters[] = [$part, $direction];
-            }
-        }
-        $bag->set('sort', $sortParameters);
-
         return $bag;
+        //return $this->getSortParameters($bag);
     }
 
     /**

app/Api/V1/Controllers/Models/Account/ShowController.php

@@ -82,17 +82,19 @@ class ShowController extends Controller
 
         // get list of accounts. Count it and split it.
         $this->repository->resetAccountOrder();
-        // TODO fix sort.
+        $collection  = $this->repository->getAccountsByType($types, $params['sort']);
-        $collection  = $this->repository->getAccountsByType($types);
         $count       = $collection->count();
 
         // continue sort:
+        // TODO if the user sorts on DB dependent field there must be no slice before enrichment, only after.
+        // TODO still need to figure out how to do this easily.
         $accounts    = $collection->slice(($this->parameters->get('page') - 1) * $params['limit'], $params['limit']);
 
         // enrich
         /** @var User $admin */
         $admin       = auth()->user();
         $enrichment  = new AccountEnrichment();
+        $enrichment->setSort($params['sort']);
         $enrichment->setDate($this->parameters->get('date'));
         $enrichment->setStart($this->parameters->get('start'));
         $enrichment->setEnd($this->parameters->get('end'));

app/Api/V1/Requests/Models/Account/ShowRequest.php

@@ -24,7 +24,9 @@ declare(strict_types=1);
 namespace FireflyIII\Api\V1\Requests\Models\Account;
 
 use Carbon\Carbon;
+use FireflyIII\Models\Account;
 use FireflyIII\Models\Preference;
+use FireflyIII\Rules\IsValidSortInstruction;
 use FireflyIII\Support\Facades\Preferences;
 use FireflyIII\Support\Http\Api\AccountFilter;
 use FireflyIII\Support\Request\ConvertsDataTypes;
@@ -55,7 +57,7 @@ class ShowRequest extends FormRequest
         return [
             'type'  => $this->convertString('type', 'all'),
             'limit' => $limit,
-            'sort'  => $this->convertString('sort', 'order'),
+            'sort' => $this->convertSortParameters('sort',Account::class),
             'page'  => $page,
         ];
     }
@@ -68,7 +70,7 @@ class ShowRequest extends FormRequest
             'date'  => 'date',
             'start' => 'date|present_with:end|before_or_equal:end|before:2038-01-17|after:1970-01-02',
             'end'   => 'date|present_with:start|after_or_equal:start|before:2038-01-17|after:1970-01-02',
-            'sort'  => 'nullable|in:active,iban,name,order,-active,-iban,-name,-order', // TODO improve me.
+            'sort'  => ['nullable', new IsValidSortInstruction(Account::class)],
             'type'  => sprintf('in:%s', $keys),
             'limit' => 'numeric|min:1|max:131337',
             'page'  => 'numeric|min:1|max:131337',

app/Repositories/Account/AccountRepository.php

@@ -484,14 +484,19 @@ class AccountRepository implements AccountRepositoryInterface, UserGroupInterfac
             $query->accountTypeIn($types);
         }
 
-        // add sort parameters. At this point they're filtered to allowed fields to sort by:
+        // add sort parameters
+        $allowed = config('firefly.allowed_db_sort_parameters.Account', []);
+        $sorted = 0;
         if (0 !== count($sort)) {
             foreach ($sort as $param) {
+                if(in_array($param[0], $allowed, true)) {
                     $query->orderBy($param[0], $param[1]);
+                    ++$sorted;
+                }
             }
         }
 
-        if (0 === count($sort)) {
+        if (0 === $sorted) {
             if (0 !== count($res)) {
                 $query->orderBy('accounts.order', 'ASC');
             }

app/Rules/IsValidSortInstruction.php

@@ -0,0 +1,64 @@
+<?php
+/*
+ * IsValidSortInstruction.php
+ * Copyright (c) 2025 james@firefly-iii.org
+ *
+ * This file is part of Firefly III (https://github.com/firefly-iii).
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+namespace FireflyIII\Rules;
+
+use Closure;
+use Illuminate\Contracts\Validation\ValidationRule;
+
+class IsValidSortInstruction implements ValidationRule
+{
+    private string $class;
+
+    public function __construct(string $class)
+    {
+        $this->class = $class;
+    }
+
+    public function validate(string $attribute, mixed $value, Closure $fail): void
+    {
+        $shortClass = str_replace('FireflyIII\\Models\\', '', $this->class);
+        if (!is_string($value)) {
+            $fail('validation.invalid_sort_instruction')->translate(['object' => $shortClass]);
+            return;
+        }
+        $validParameters = config(sprintf('firefly.allowed_sort_parameters.%s', $shortClass));
+        if (!is_array($validParameters)) {
+            $fail('validation.no_sort_instructions')->translate(['object' => $shortClass]);
+            return;
+        }
+        $parts = explode(',', $value);
+        foreach ($parts as $i => $part) {
+            $part = trim($part);
+            if (strlen($part) < 2) {
+                $fail('validation.invalid_sort_instruction_index')->translate(['index' => $i, 'object' => $shortClass]);
+                return;
+            }
+            if ('-' === $part[0]) {
+                $part = substr($part, 1);
+            }
+            if (!in_array($part, $validParameters, true)) {
+                $fail('validation.invalid_sort_instruction_index')->translate(['index' => $i, 'object' => $shortClass]);
+                return;
+            }
+        }
+    }
+}

app/Support/JsonApi/Enrichments/AccountEnrichment.php

@@ -75,6 +75,7 @@ class AccountEnrichment implements EnrichmentInterface
     private array               $endBalances     = [];
     private array               $objectGroups    = [];
     private array               $mappedObjects   = [];
+    private array               $sort            = [];
 
     /**
      * TODO The account enricher must do conversion from and to the primary currency.
@@ -115,6 +116,7 @@ class AccountEnrichment implements EnrichmentInterface
         $this->collectObjectGroups();
         $this->collectBalances();
         $this->appendCollectedData();
+        $this->sortData();
 
         return $this->collection;
     }
@@ -144,8 +146,7 @@ class AccountEnrichment implements EnrichmentInterface
     {
         $set = AccountMeta::whereIn('name', ['is_multi_currency', 'include_net_worth', 'currency_id', 'account_role', 'account_number', 'BIC', 'liability_direction', 'interest', 'interest_period', 'current_debt'])
                           ->whereIn('account_id', $this->ids)
-            ->get(['account_meta.id', 'account_meta.account_id', 'account_meta.name', 'account_meta.data'])->toArray()
+                          ->get(['account_meta.id', 'account_meta.account_id', 'account_meta.name', 'account_meta.data'])->toArray();
-        ;
 
         /** @var array $entry */
         foreach ($set as $entry) {
@@ -173,8 +174,7 @@ class AccountEnrichment implements EnrichmentInterface
         $notes = Note::query()->whereIn('noteable_id', $this->ids)
                      ->whereNotNull('notes.text')
                      ->where('notes.text', '!=', '')
-            ->where('noteable_type', Account::class)->get(['notes.noteable_id', 'notes.text'])->toArray()
+                     ->where('noteable_type', Account::class)->get(['notes.noteable_id', 'notes.text'])->toArray();
-        ;
         foreach ($notes as $note) {
             $this->notes[(int)$note['noteable_id']] = (string)$note['text'];
         }
@@ -184,8 +184,7 @@ class AccountEnrichment implements EnrichmentInterface
     private function collectLocations(): void
     {
         $locations = Location::query()->whereIn('locatable_id', $this->ids)
-            ->where('locatable_type', Account::class)->get(['locations.locatable_id', 'locations.latitude', 'locations.longitude', 'locations.zoom_level'])->toArray()
+                             ->where('locatable_type', Account::class)->get(['locations.locatable_id', 'locations.latitude', 'locations.longitude', 'locations.zoom_level'])->toArray();
-        ;
         foreach ($locations as $location) {
             $this->locations[(int)$location['locatable_id']]
                 = [
@@ -207,8 +206,7 @@ class AccountEnrichment implements EnrichmentInterface
             ->setUserGroup($this->userGroup)
             ->setAccounts($this->collection)
             ->withAccountInformation()
-            ->setTypes([TransactionTypeEnum::OPENING_BALANCE->value])
+            ->setTypes([TransactionTypeEnum::OPENING_BALANCE->value]);
-        ;
         $journals = $collector->getExtractedJournals();
         foreach ($journals as $journal) {
             $this->openingBalances[(int)$journal['source_account_id']]
@@ -374,8 +372,7 @@ class AccountEnrichment implements EnrichmentInterface
         $set = DB::table('object_groupables')
                  ->whereIn('object_groupable_id', $this->ids)
                  ->where('object_groupable_type', Account::class)
-            ->get(['object_groupable_id', 'object_group_id'])
+                 ->get(['object_groupable_id', 'object_group_id']);
-        ;
 
         $ids = array_unique($set->pluck('object_group_id')->toArray());
 
@@ -434,4 +431,31 @@ class AccountEnrichment implements EnrichmentInterface
 
         return bcsub($end, $start);
     }
+
+    public function setSort(array $sort): void
+    {
+        $this->sort = $sort;
+    }
+
+    private function sortData(): void
+    {
+        $dbParams = config('firefly.allowed_db_sort_parameters.Account', []);
+        /** @var array<string,string> $parameter */
+        foreach ($this->sort as $parameter) {
+            if (in_array($parameter[0], $dbParams, true)) {
+                continue;
+            }
+            switch ($parameter[0]) {
+                default:
+                    throw new FireflyException(sprintf('Account enrichment cannot sort on field "%s"', $parameter[0]));
+                case 'current_balance':
+                case 'pc_current_balance':
+                    $this->collection = $this->collection->sortBy(static function (Account $account) use ($parameter) {
+                        return $account->meta['balances'][$parameter[0]] ?? '0';
+                    }, SORT_NUMERIC, 'desc' === $parameter[1]);
+                    break;
+            }
+        }
+    }
+
 }

app/Support/Request/ConvertsDataTypes.php

@@ -31,7 +31,6 @@ use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Support\Facades\Steam;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Log;
-
 use function Safe\preg_replace;
 
 /**
@@ -99,6 +98,24 @@ trait ConvertsDataTypes
         return Steam::filterSpaces($string);
     }
 
+    public function convertSortParameters(string $field, string $class): array
+    {
+        // assume this all works, because the validator would have caught any errors.
+        $parameter      = (string)request()->query->get($field);
+        $parts          = explode(',', $parameter);
+        $sortParameters = [];
+        foreach ($parts as $part) {
+            $part      = trim($part);
+            $direction = 'asc';
+            if ('-' === $part[0]) {
+                $part      = substr($part, 1);
+                $direction = 'desc';
+            }
+            $sortParameters[] = [$part, $direction];
+        }
+        return $sortParameters;
+    }
+
     public function clearString(?string $string): ?string
     {
         $string = $this->clearStringKeepNewlines($string);

config/firefly.php

@@ -827,8 +827,24 @@ return [
     // dynamic date ranges are as follows:
     'dynamic_date_ranges'       => ['last7', 'last30', 'last90', 'last365', 'MTD', 'QTD', 'YTD'],
 
-    // only used in v1
+    'allowed_sort_parameters'    => [
-    'allowed_sort_parameters'      => ['order', 'name', 'iban'],
+        'Account' => ['id', 'order', 'name', 'iban', 'active', 'account_type_id',
+                      'current_balance',
+                      'pc_current_balance',
+                      'opening_balance',
+                      'pc_opening_balance',
+                      'virtual_balance',
+                      'pc_virtual_balance',
+                      'debt_amount',
+                      'pc_debt_amount',
+                      'balance_difference',
+                      'pc_balance_difference',
+        ],
+    ],
+    'allowed_db_sort_parameters' => [
+        'Account' => ['id', 'order', 'name', 'iban', 'active', 'account_type_id'],
+    ],
+
 
     // preselected account lists possibilities:
     'preselected_accounts'       => ['all', 'assets', 'liabilities'],

resources/lang/en_US/validation.php

@@ -24,6 +24,9 @@
 declare(strict_types=1);
 
 return [
+    'invalid_sort_instruction'       => 'The sort instruction is invalid for an object of type ":object".',
+    'invalid_sort_instruction_index'       => 'The sort instruction at index #:index is invalid for an object of type ":object".',
+    'no_sort_instructions' => 'There are no sort instructions defined for an object of type ":object".',
     'webhook_budget_info'            => 'Cannot deliver budget information for transaction related webhooks.',
     'webhook_account_info'           => 'Cannot deliver account information for budget related webhooks.',
     'webhook_transaction_info'       => 'Cannot deliver transaction information for budget related webhooks.',