fix: [Table] field named data will produce bugged output#8054
fix: [Table] field named data will produce bugged output#8054kenjis merged 4 commits intocodeigniter4:developfrom
data will produce bugged output#8054Conversation
|
@ping-yee Can you add test code? |
data will produce bugged output problem.data will produce bugged output
|
@kenjis Done. |
| // If there is no $args[0], skip this and treat as an associative array | ||
| // This can happen if there is only a single key, for example this is passed to table->generate | ||
| // array(array('foo'=>'bar')) | ||
| if (isset($args[0]) && count($args) === 1 && is_array($args[0]) && ! isset($args[0]['data'])) { |
There was a problem hiding this comment.
It will cause the setHeading function to be executed incorrectly when the heading field contain named data.
There was a problem hiding this comment.
The && ! isset($args[0]['data']) came from this commit: bcit-ci/CodeIgniter@f0b69a85
Are you absolutely sure that just removing it does not cause another issue?
Frankly, I don't get the code yet.
There was a problem hiding this comment.
I am not sure but the all of the test cases get passed.
And I also don't get where will use this condition and the data processing of the array field contains data so far.
Or we can add one more process to check that whether the heading array has been handled correctly when the table is being generated.
There was a problem hiding this comment.
the all of the test cases get passed.
Yes. But we cannot see the coverage in Coveralls.
coverage/coveralls — Target branch is ahead of PR base. Rebase the PR to fix.
Can you rebase the PR branch?
There was a problem hiding this comment.
Can you rebase the PR branch?
Done.
There was a problem hiding this comment.
Based on my usage of the table class (quite extensive) I have not noticed any issues thus far with the patch applied.
|
If anyone has a good understanding of the Table's code, please review this PR. |
7500d18 to
39903e6
Compare
|
This bug has been introduced in: |

Description
Fixes #8051
Checklist: