Fix wrong data for column in exported csv in Reports->Lessons #4975
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
merkushin
approved these changes
Apr 1, 2022
| @@ -351,7 +351,7 @@ public function generate_report( $report ) { | |||
|
|
|||
| // Process each row. | |||
| foreach ( $this->items as $item ) { | |||
| $data[] = $this->get_row_data( $item ); | |||
| $data[] = array_replace( $columns, $this->get_row_data( $item ) ); | |||
There was a problem hiding this comment.
I wonder if it doesn't cause any performance issues?
There was a problem hiding this comment.
I was thinking about this too, but hopefully in this particular case it won't come to that, the loop will be running for number of rows * number of properties time in total, we don't have that many properties (columns) usually, and for the number of rows to become an issue, I think we'll be facing out of memory or something like that before that happens :p So I think compared to the time it takes to fetch the data and prepare a row, the time taken here hopefully won't become too significant
|
I'm not sure how I missed this one. 😄 GJ finding the issue! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes the issue mentioned here #4964 (review)
Changes proposed in this Pull Request
The column data was not sorted according to the column headers. So when 'module' column was added in the headers, the data was not being sorted, so the module column appeared after the title column, but the data for the module column was in the last column in each row. So it resulted in all columns except the title showing wrong data (1 right circular shift except the title). Here we're sorting the data as per column headers to make them appear in the same order.
Testing instructions
Screenshot / Video
Before - (Notice that the students column is showing a date, also the rest of the columns)
After-
