Skip to content

remove bad data added to export - #22

Open
jay-oswald wants to merge 2 commits into
pauln:masterfrom
catalyst:master-catalyst
Open

remove bad data added to export#22
jay-oswald wants to merge 2 commits into
pauln:masterfrom
catalyst:master-catalyst

Conversation

@jay-oswald

Copy link
Copy Markdown

The issue comes from the header function correctly using $showgroups which checks both the showgroups config setting, but also checks the course groupmode. However the data function previously

The inconsistency in the header and data functions causes the issue. I have passed in $showgroups to the generating fucntions to make it consistent with the header, there should be no more edge cases now as it all simply uses $showgroups. If if is false there can not be a header or data, and if its true there will always be a header and data.

It was added in #15 and fixes #20

I also removed the $row variable being used on line 121 of rubic.php, it is not used anywhere, and now makes it consistant with guide and btec files.

@marcusgreen

Copy link
Copy Markdown
Collaborator

Hi Jay, what does the bad data looks like without this fix. Part of the reason I ask is that I don't actively maintain this plugin (though will accept nice pulls of course), and I now maintain one that covers similar ground
https://github.com/marcusgreen/moodle-report_advancedgrading
(See issues for an odd recurring thing that might be a bug...)

@jay-oswald

Copy link
Copy Markdown
Author

The same as in that linked issue, it basically causes an off by 1 error, where the header decides it should not output, but each row decides that it should output.

So the group string ends up getting output in the 1st column of the score data, which shifts the score data all over by 1 column, and also drops off the last column, which from memory is grading time.

I haven't dug deeply into why it happens, but it's happening at the moment on a site, and making the if statements in the header and data functions the same will eliminate the chance of whatever edge case its happening in.

@marcusgreen

Copy link
Copy Markdown
Collaborator

sorry I didn't read your linked issues

@dragos5436

Copy link
Copy Markdown

Hi there, can we still expect this PR to be merged or not since the plugin is no longer maintained?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show groups setting results in mismatched column headers and data

3 participants