search

pauln / moodle-report_componentgrades

Excel spreadsheet export tool for individual component grades in Marking Guides and Rubrics.
GNU General Public License v3.0
2 stars 13 forks source link

Fix #14 - Include group information in rubric export. #15

Closed danmarsden closed 3 years ago

danmarsden commented 3 years ago

@marcusgreen - I tried not to touch too much extra stuff, but I did sneak in a fix for the student idnumber - it wasn't respecting the setting for include/exclude student idnumber in the data - only in the header function. Let me know if you want me to change anything - thanks!

marcusgreen commented 3 years ago

Hi Dan, would it be possible to put this into the settings.php file like I have the studentid field in the new fix/pull I have created. https://github.com/pauln/moodle-report_componentgrades/pull/16

danmarsden commented 3 years ago

sure - Can you please merge in your changes from #16 and I'll rebase mine and add a new setting... I didn't use a setting as I was thinking it was probably enough to check course group mode but I guess some people will want to exlude it? - should I set it as default to "yes" include groups or "no" ?

marcusgreen commented 3 years ago

OK, will try to do that tomorrow. I am a bit of a fan of settings, which is why I suggested it. Probably default to yes (or whatever you think would be best for most people)

marcusgreen commented 3 years ago

I decided to be bold.... merged.

marcusgreen commented 3 years ago

I missed your comment about studentid number this morning, I added my own fix into that thing I just merged, but I had not looked at yours.

danmarsden commented 3 years ago

yeah - I saw that - it's the exact same fix :-) - all good - I'll rebase later and fire it through - thanks!

marcusgreen commented 3 years ago

Hi Dan, I was thinking of progressing my fixes to the plugins database, possibly over the coming weekend. I can always include your changes after that if you like

danmarsden commented 3 years ago

Thanks @marcusgreen - I've just rebased and added an admin setting for this - let me know if you want me to make any further chnages! - thanks.

danmarsden commented 3 years ago

I didn't bump the version as I assumed you would do that in the plugins db release, but let me know if you want me to bump it in this commit too.

marcusgreen commented 3 years ago

Thanks Dan, I will work on getting it all into an update in the plugins database.

marcusgreen commented 3 years ago

I have merged it but it looks like the current master is somewhat broken (almost certainly nothing to do with your changes). Will continue to look into the reasons.

marcusgreen commented 3 years ago

Ahh, turns out I didn't understand how it was supposed to work. looking good now I think. Could you pull the current master from here and confirm it does what you expect.

danmarsden commented 3 years ago

cool thanks! - looks good to me! :-)