Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mactimes opened this issue Oct 17, 2023 · 7 comments · Fixed by #8054
Closed

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

mactimes opened this issue Oct 17, 2023 · 7 comments · Fixed by #8054
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@mactimes
Copy link

mactimes commented Oct 17, 2023

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

@mactimes mactimes added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 17, 2023
@kenjis
Copy link
Member

kenjis commented Oct 17, 2023

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
Copy link
Author

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
Copy link
Member

kenjis commented Oct 17, 2023

@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
Copy link
Author

@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] => 
        )

)

@kenjis kenjis changed the title Bug: HTML Table Class fails with table column named "data'. Bug: HTML Table Class fails with table column named 'data'. Oct 18, 2023
@dgvirtual
Copy link
Contributor

dgvirtual commented Oct 26, 2023

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
Copy link
Member

kenjis commented Oct 26, 2023

@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
Copy link
Member

kenjis commented Oct 26, 2023

$ 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()
     {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants