Merge pull request #16485 from niden/T16471-count-orderby

T16471 count orderby

.github/actions/build-phalcon-win/action.yml

@@ -74,15 +74,15 @@ runs:
         cache_dir: ${{ env.CACHE_DIR }}
 
     - name: Configure Developer Command Prompt for MSVC compiler
-      uses: ilammy/msvc-dev-cmd@v1.10.0
+      uses: ilammy/msvc-dev-cmd@v1.12.1
       with:
         arch: ${{ inputs.arch }}
 
     # Workaround for
     # PHP Warning:  PHP Startup: Can't load module 'C:\tools\php\ext\php_zephir_parser.dll'
     # as it's linked with 14.29, but the core is linked with 14.16 in Unknown on line 0
     - name: Configure Developer Command Prompt for MSVC compiler
-      uses: ilammy/msvc-dev-cmd@v1.10.0
+      uses: ilammy/msvc-dev-cmd@v1.12.1
       if: ${{ inputs.php_version }} == '7.4'
       with:
         arch: ${{ inputs.arch }}

.github/workflows/main.yml

@@ -284,7 +284,7 @@ jobs:
           COMPOSER_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Download Phalcon Pecl Package
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v3
         with:
           name: phalcon-pecl
           path: ./phalcon-pecl
@@ -386,12 +386,13 @@ jobs:
       - name: Get the release version
         id: get-version
         run: |
-          echo ::set-output name=version::${GITHUB_REF#refs/tags/}
+          echo "version=${GITHUB_REF#refs/tags/}" >> $GITHUB_ENV
+#          echo ::set-output name=version::${GITHUB_REF#refs/tags/}
 #          echo "version=${GITHUB_REF#refs/tags/}" >> "$GITHUB_ENV" # This needs to be checked
 
       - name: Download Phalcon build artifacts
         id: download
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v3
         with:
           path: ./build-artifacts
 
@@ -407,8 +408,8 @@ jobs:
         uses: ncipollo/release-action@v1
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
-          name: ${{ steps.get-version.outputs.version }}
-          tag: ${{ steps.get-version.outputs.version }}
+          name: ${{ env.version }}
+          tag: ${{ env.version }}
           bodyFile: "./build-artifacts/release/release-notes.md"
           allowUpdates: true
           artifacts: "./build-artifacts/release/*.zip,./build-artifacts/release/*.tgz"

CHANGELOG-5.0.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## [5.5.0](https://github.com/phalcon/cphalcon/releases/tag/v5.5.1) (xxxx-xx-xx)
+
+### Changed
+
+### Added
+
+### Fixed
+
+- Fixed `Phalcon\Mvc\Model::count` to ignore the `order` parameter (needed for Posgresql) [#16471](https://github.com/phalcon/cphalcon/issues/16471)
+
+### Removed
+
 ## [5.5.0](https://github.com/phalcon/cphalcon/releases/tag/v5.5.0) (2023-12-25)
 
 ### Changed

phalcon/Mvc/Model.zep

@@ -1184,6 +1184,13 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
     {
         var result;
 
+        /**
+         * Removing `order by` for postgresql
+         */
+        if (isset(parameters["order"])) {
+            unset parameters["order"];
+        }
+
         let result = self::groupResult("COUNT", "rowcount", parameters);
 
         if typeof result === "string" {

tests/database/Mvc/Model/CountCest.php

@@ -30,7 +30,7 @@ class CountCest
     /**
      * @var InvoicesMigration
      */
-    private $invoiceMigration;
+    private InvoicesMigration $invoiceMigration;
 
     /**
      * Executed before each test
@@ -63,7 +63,7 @@ public function _before(DatabaseTester $I): void
      * @group  pgsql
      * @group  sqlite
      */
-    public function mvcModelCount(DatabaseTester $I)
+    public function mvcModelCount(DatabaseTester $I): void
     {
         /**
          * TODO: The following tests need to skip sqlite because we will get
@@ -94,12 +94,25 @@ public function mvcModelCount(DatabaseTester $I)
             ]
         );
         $I->assertInstanceOf(Simple::class, $results);
-        $I->assertEquals(1, (int) $results[0]->inv_cst_id);
-        $I->assertEquals(20, (int) $results[0]->rowcount);
-        $I->assertEquals(2, (int) $results[1]->inv_cst_id);
-        $I->assertEquals(12, (int) $results[1]->rowcount);
-        $I->assertEquals(3, (int) $results[2]->inv_cst_id);
-        $I->assertEquals(1, (int) $results[2]->rowcount);
+
+        if ('pgsql' !== $I->getDriver()) {
+            $matrix = [
+                0 => [1, 20],
+                1 => [2, 12],
+                2 => [3, 1],
+            ];
+        } else {
+            $matrix = [
+                0 => [3, 1],
+                1 => [2, 12],
+                2 => [1, 20],
+            ];
+        }
+
+        foreach ($matrix as $id => $expected) {
+            $I->assertSame($expected[0], (int) $results[$id]->inv_cst_id);
+            $I->assertSame($expected[1], (int) $results[$id]->rowcount);
+        }
 
         $results = Invoices::count(
             [
@@ -108,12 +121,11 @@ public function mvcModelCount(DatabaseTester $I)
             ]
         );
         $I->assertInstanceOf(Simple::class, $results);
-        $I->assertEquals(3, (int) $results[0]->inv_cst_id);
-        $I->assertEquals(1, (int) $results[0]->rowcount);
-        $I->assertEquals(2, (int) $results[1]->inv_cst_id);
-        $I->assertEquals(12, (int) $results[1]->rowcount);
-        $I->assertEquals(1, (int) $results[2]->inv_cst_id);
-        $I->assertEquals(20, (int) $results[2]->rowcount);
+
+        foreach ($matrix as $id => $expected) {
+            $I->assertSame($expected[0], (int) $results[$id]->inv_cst_id);
+            $I->assertSame($expected[1], (int) $results[$id]->rowcount);
+        }
 
         /**
          * @issue https://github.com/phalcon/cphalcon/issues/15486
@@ -154,12 +166,13 @@ public function mvcModelCount(DatabaseTester $I)
      * @param  DatabaseTester $I
      *
      * @author Phalcon Team <team@phalcon.io>
-     * @since  2020-01-29
+     * @since  2023-12-26
+     * @issue  https://github.com/phalcon/cphalcon/issues/16471
      *
      * @group  mysql
      * @group  pgsql
      */
-    public function mvcModelCountColumnMap(DatabaseTester $I)
+    public function mvcModelCountColumnMap(DatabaseTester $I): void
     {
         /**
          * @todo The following tests need to skip sqlite because we will get
@@ -190,12 +203,29 @@ public function mvcModelCountColumnMap(DatabaseTester $I)
             ]
         );
         $I->assertInstanceOf(Simple::class, $results);
-        $I->assertEquals(1, (int) $results[0]->cst_id);
-        $I->assertEquals(20, (int) $results[0]->rowcount);
-        $I->assertEquals(2, (int) $results[1]->cst_id);
-        $I->assertEquals(12, (int) $results[1]->rowcount);
-        $I->assertEquals(3, (int) $results[2]->cst_id);
-        $I->assertEquals(1, (int) $results[2]->rowcount);
+
+        /**
+         * This is here because each engine sorts their groupped results
+         * differently
+         */
+        if ('mysql' !== $I->getDriver()) {
+            $matrix = [
+                0 => [3, 1],
+                1 => [2, 12],
+                2 => [1, 20],
+            ];
+        } else {
+            $matrix = [
+                0 => [1, 20],
+                1 => [2, 12],
+                2 => [3, 1],
+            ];
+        }
+
+        foreach ($matrix as $id => $expected) {
+            $I->assertSame($expected[0], (int) $results[$id]->cst_id);
+            $I->assertSame($expected[1], (int) $results[$id]->rowcount);
+        }
 
         $results = InvoicesMap::count(
             [
@@ -204,12 +234,11 @@ public function mvcModelCountColumnMap(DatabaseTester $I)
             ]
         );
         $I->assertInstanceOf(Simple::class, $results);
-        $I->assertEquals(3, (int) $results[0]->cst_id);
-        $I->assertEquals(1, (int) $results[0]->rowcount);
-        $I->assertEquals(2, (int) $results[1]->cst_id);
-        $I->assertEquals(12, (int) $results[1]->rowcount);
-        $I->assertEquals(1, (int) $results[2]->cst_id);
-        $I->assertEquals(20, (int) $results[2]->rowcount);
+
+        foreach ($matrix as $id => $expected) {
+            $I->assertSame($expected[0], (int) $results[$id]->cst_id);
+            $I->assertSame($expected[1], (int) $results[$id]->rowcount);
+        }
     }
 
     /**

tests/database/Paginator/Adapter/Model/PaginateCest.php

@@ -333,4 +333,53 @@ public function paginatorAdapterModelPaginateView(DatabaseTester $I): void
         $actual = $view->getVar('paginate');
         $I->assertInstanceOf(Repository::class, $actual);
     }
+
+    /**
+     * @param DatabaseTester $I
+     *
+     * @author Phalcon Team <team@phalcon.io>
+     * @since  2023-12-26
+     * @issue  https://github.com/phalcon/cphalcon/issues/16471
+     *
+     * @group mysql
+     * @group pgsql
+     */
+    public function paginatorAdapterModelPaginateWithOrder(DatabaseTester $I)
+    {
+        $I->wantToTest('Paginator\Adapter\Model - paginate() - with order');
+
+        /** @var PDO $connection */
+        $connection = $I->getConnection();
+        $migration  = new InvoicesMigration($connection);
+        $invId      = 'default';
+
+        $this->insertDataInvoices($migration, 17, $invId, 2, 'ccc');
+        $this->insertDataInvoices($migration, 11, $invId, 3, 'aaa');
+        $this->insertDataInvoices($migration, 31, $invId, 1, 'aaa');
+        $this->insertDataInvoices($migration, 15, $invId, 2, 'bbb');
+
+        $paginator = new Model(
+            [
+                'model'      => Invoices::class,
+                'parameters' => [
+                    'inv_cst_id >= 2',
+                    'order' => 'inv_cst_id'
+                ],
+                'limit'      => 5,
+                'page'       => 1,
+            ]
+        );
+
+        // First Page
+        $page = $paginator->paginate();
+
+        $I->assertInstanceOf(Repository::class, $page);
+
+        $I->assertCount(5, $page->getItems());
+        $I->assertEquals(1, $page->getPrevious());
+        $I->assertEquals(2, $page->getNext());
+        $I->assertEquals(9, $page->getLast());
+        $I->assertEquals(5, $page->getLimit());
+        $I->assertEquals(1, $page->getCurrent());
+    }
 }