codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.22k stars 1.87k forks source link

Bug: HTML Table Class fails with table column named 'data'. #8051

Closed mactimes closed 8 months ago

mactimes commented 8 months ago

PHP Version

8.2

CodeIgniter4 Version

4.4.1

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

mariadb Ver 15.1 Distrib 10.11.4-MariaDB, for debian-linux-gnu (x86_64) using EditLine wrapper

What happened?

HTML Table Class produces bugged output as follows:

Data do Orçamento
2023-10-16 21:12:29
2023-10-16 21:12:29
2023-10-16 21:12:29

Steps to Reproduce

Serve Model->asArray()->findAll() to generate() method of HTML Table class when source table contains column named "data".

Expected Output

Codigo OrçamentoData do OrçamentoTipo de DescontoValor do Desconto
codigo12023-10-16 21:50:08NENHUM
codigo22023-10-16 21:50:08REAL10.00
codigo32023-10-16 21:50:08PERCENTUAL10.00

Anything else?

ci4-html_table_class-bug_report

kenjis commented 8 months ago

Can you show the minimum source code to reproduce the issue?

Or can you send a Pull Request to fix it? See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

mactimes commented 8 months ago

Can you show the minimum source code to reproduce the issue?

Or can you send a Pull Request to fix it? See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

Sure. I'm attaching all related files. I believe those should be enough to reproduce the issue on a newly deployed, running CI4 installation. Please, do let me know should you require any further information.

On'ly change I'm making is commenting the foreign key from Orcamento's Migration so you have all the files to reproduce the issue without having to make any changes.

I just would like to highlight again that changing column name from "data" to, let's say "data_orcamento" makes the problem disappear. I haven't yet looked deeper into the issue, so I cannot send a pull request yet.

Relevant route from Config\Routes.php $routes->get('/orcamentos', 'Orcamentos::index');

Database storage engine is InnoDB. Database default charset is ut88mb4. Database default collate is utf8mb4_unicode_ci.

ci4-issue_8051.zip

kenjis commented 8 months ago

@mactimes Thank you. But this is an issue on HTML Table. We don't need the Migration or Model code to reproduce the issue.

Can you show sample output of $OrcamentoModel->asArray()->findAll()?

        $table_template = [
            'table_open' => '<table border="1" cellpadding="2" cellspacing="1">',

            'thead_open'  => '<thead>',
            'thead_close' => '</thead>',

            'heading_row_start'  => '<tr>',
            'heading_row_end'    => '</tr>',
            'heading_cell_start' => '<th>',
            'heading_cell_end'   => '</th>',

            'tfoot_open'  => '<tfoot>',
            'tfoot_close' => '</tfoot>',

            'footing_row_start'  => '<tr>',
            'footing_row_end'    => '</tr>',
            'footing_cell_start' => '<td>',
            'footing_cell_end'   => '</td>',

            'tbody_open'  => '<tbody>',
            'tbody_close' => '</tbody>',

            'row_start'  => '<tr>',
            'row_end'    => '</tr>',
            'cell_start' => '<td>',
            'cell_end'   => '</td>',

            'row_alt_start'  => '<tr>',
            'row_alt_end'    => '</tr>',
            'cell_alt_start' => '<td>',
            'cell_alt_end'   => '</td>',

            'table_close' => '</table>',
        ];

        $table = new \CodeIgniter\View\Table($table_template);
        $table->setHeading([
            'codigo' => 'Codigo Orçamento',
            'data' => 'Data do Orçamento',
            'tipo_desconto' => 'Tipo de Desconto',
            'valor_desconto' => 'Valor do Desconto',
        ])->setSyncRowsWithHeading(true);

        $sampleData = [/.../]; // We need this data.

        return $table->generate($sampleData);
mactimes commented 8 months ago

@mactimes Thank you. But this is an issue on HTML Table. We don't need the Migration or Model code to reproduce the issue.

Can you show sample output of $OrcamentoModel->asArray()->findAll()?

        $table_template = [
            'table_open' => '<table border="1" cellpadding="2" cellspacing="1">',

            'thead_open'  => '<thead>',
            'thead_close' => '</thead>',

            'heading_row_start'  => '<tr>',
            'heading_row_end'    => '</tr>',
            'heading_cell_start' => '<th>',
            'heading_cell_end'   => '</th>',

            'tfoot_open'  => '<tfoot>',
            'tfoot_close' => '</tfoot>',

            'footing_row_start'  => '<tr>',
            'footing_row_end'    => '</tr>',
            'footing_cell_start' => '<td>',
            'footing_cell_end'   => '</td>',

            'tbody_open'  => '<tbody>',
            'tbody_close' => '</tbody>',

            'row_start'  => '<tr>',
            'row_end'    => '</tr>',
            'cell_start' => '<td>',
            'cell_end'   => '</td>',

            'row_alt_start'  => '<tr>',
            'row_alt_end'    => '</tr>',
            'cell_alt_start' => '<td>',
            'cell_alt_end'   => '</td>',

            'table_close' => '</table>',
        ];

        $table = new \CodeIgniter\View\Table($table_template);
        $table->setHeading([
            'codigo' => 'Codigo Orçamento',
            'data' => 'Data do Orçamento',
            'tipo_desconto' => 'Tipo de Desconto',
            'valor_desconto' => 'Valor do Desconto',
        ])->setSyncRowsWithHeading(true);

        $sampleData = [/.../]; // We need this data.

        return $table->generate($sampleData);

Here's the output of $OrcamentoModel->asArray()->findAll():

Array
(
    [0] => Array
        (
            [id] => 1
            [id_cliente] => 1
            [codigo] => codigo1
            [data] => 2023-10-16 21:53:25
            [tipo_desconto] => NENHUM
            [valor_desconto] => 
            [created_at] => 2023-10-16 21:53:25
            [updated_at] => 2023-10-16 21:53:25
            [deleted_at] => 
        )
    [1] => Array
        (
            [id] => 2
            [id_cliente] => 2
            [codigo] => codigo2
            [data] => 2023-10-16 21:53:25
            [tipo_desconto] => REAL
            [valor_desconto] => 10.00
            [created_at] => 2023-10-16 21:53:25
            [updated_at] => 2023-10-16 21:53:25
            [deleted_at] => 
        )
    [2] => Array
        (
            [id] => 3
            [id_cliente] => 3
            [codigo] => codigo3
            [data] => 2023-10-16 21:53:25
            [tipo_desconto] => PERCENTUAL
            [valor_desconto] => 10.00
            [created_at] => 2023-10-16 21:53:25
            [updated_at] => 2023-10-16 21:53:25
            [deleted_at] => 
        )

)
dgvirtual commented 8 months ago

The bug affected one of my tables this way:

 public function tabletest()
    {
        $inputArray = array(
            'watches' => array(
                'label' => 'First category label',
                'data' => 26,
                'calc' => '* 15 = 390 €'
            ),
            'calls_ex' => array(
                'label' => 'Second category label',
                'data' => 0,
                'calc' => 0
            ),
        );
        $table = new \CodeIgniter\View\Table();
        $table->setHeading('Category', 'Data', 'Calculation');
        $table_string = $table->generate($inputArray);
        var_dump($table_string);
    }

Would return table like this:

<table border="0" cellpadding="4" cellspacing="0">
    <thead>
        <tr>
            <th>Category</th>
            <th>Data</th>
            <th>Calculation</th>
        </tr>
    </thead>
    <tbody>
        <tr>
            <td calc="* 15 = 390 €" label="First category label">26</td>
        </tr>
        <tr>
            <td calc="0" label="Second category label">0</td>
        </tr>
    </tbody>
</table>
kenjis commented 8 months ago

@dgvirtual Thank you!

In v4.2.12:

<table border="0" cellpadding="4" cellspacing="0">
<thead>
<tr>
<th>Category</th><th>Data</th><th>Calculation</th></tr>
</thead>
<tbody>
<tr>
<td>First category label</td><td>26</td><td>* 15 = 390 €</td></tr>
<tr>
<td>Second category label</td><td>0</td><td>0</td></tr>
</tbody>
</table>

In v4.3.8:

<table border="0" cellpadding="4" cellspacing="0">
<thead>
<tr>
<th>Category</th><th>Data</th><th>Calculation</th></tr>
</thead>
<tbody>
<tr>
<td>First category label</td><td>26</td><td>* 15 = 390 €</td></tr>
<tr>
<td>Second category label</td><td>0</td><td>0</td></tr>
</tbody>
</table>

In v4.4.2 or develop:

<table border="0" cellpadding="4" cellspacing="0">
<thead>
<tr>
<th>Category</th><th>Data</th><th>Calculation</th></tr>
</thead>
<tbody>
<tr>
<td calc="* 15 = 390 €" label="First category label">26</td></tr>
<tr>
<td calc="0" label="Second category label">0</td></tr>
</tbody>
</table>
kenjis commented 8 months ago
$ git diff -wb v4.2.12...develop system/View/Table.php
diff --git a/system/View/Table.php b/system/View/Table.php
index a7ded4fd5c..df0fbebd2b 100644
--- a/system/View/Table.php
+++ b/system/View/Table.php
@@ -17,6 +17,8 @@ use CodeIgniter\Database\BaseResult;
  * HTML Table Generating Class
  *
  * Lets you create tables manually or from database result objects, or arrays.
+ *
+ * @see \CodeIgniter\View\TableTest
  */
 class Table
 {
@@ -83,6 +85,11 @@ class Table
      */
     public $function;

+    /**
+     * Order each inserted row by heading keys
+     */
+    private bool $syncRowsWithHeading = false;
+
     /**
      * Set the template from the table config file if it exists
      *
@@ -162,6 +169,7 @@ class Table
         // Turn off the auto-heading feature since it's doubtful we
         // will want headings from a one-dimensional array
         $this->autoHeading         = false;
+        $this->syncRowsWithHeading = false;

         if ($columnLimit === 0) {
             return $array;
@@ -207,7 +215,40 @@ class Table
      */
     public function addRow()
     {
-        $this->rows[] = $this->_prepArgs(func_get_args());
+        $tmpRow = $this->_prepArgs(func_get_args());
+
+        if ($this->syncRowsWithHeading && ! empty($this->heading)) {
+            // each key has an index
+            $keyIndex = array_flip(array_keys($this->heading));
+
+            // figure out which keys need to be added
+            $missingKeys = array_diff_key($keyIndex, $tmpRow);
+
+            // Remove all keys which don't exist in $keyIndex
+            $tmpRow = array_filter($tmpRow, static fn ($k) => array_key_exists($k, $keyIndex), ARRAY_FILTER_USE_KEY);
+
+            // add missing keys to row, but use $this->emptyCells
+            $tmpRow = array_merge($tmpRow, array_map(fn ($v) => ['data' => $this->emptyCells], $missingKeys));
+
+            // order keys by $keyIndex values
+            uksort($tmpRow, static fn ($k1, $k2) => $keyIndex[$k1] <=> $keyIndex[$k2]);
+        }
+        $this->rows[] = $tmpRow;
+
+        return $this;
+    }
+
+    /**
+     * Set to true if each row column should be synced by keys defined in heading.
+     *
+     * If a row has a key which does not exist in heading, it will be filtered out
+     * If a row does not have a key which exists in heading, the field will stay empty
+     *
+     * @return $this
+     */
+    public function setSyncRowsWithHeading(bool $orderByKey)
+    {
+        $this->syncRowsWithHeading = $orderByKey;

         return $this;
     }
@@ -411,6 +452,8 @@ class Table
      * Set table data from a database result object
      *
      * @param BaseResult $object Database result object
+     *
+     * @return void
      */
     protected function _setFromDBResult($object)
     {
@@ -428,6 +471,8 @@ class Table
      * Set table data from an array
      *
      * @param array $data
+     *
+     * @return void
      */
     protected function _setFromArray($data)
     {
@@ -436,12 +481,14 @@ class Table
         }

         foreach ($data as &$row) {
-            $this->rows[] = $this->_prepArgs($row);
+            $this->addRow($row);
         }
     }

     /**
      * Compile Template
+     *
+     * @return void
      */
     protected function _compileTemplate()
     {