Include user details in records exported from Database activity

Looks interesting, just need to be considered together with MDL-16593 to decide if user name and mail is enough or we want more fields to be selected and exported (dates... whatever).

Note: I'd perform one $userrec = get_record('user') and the fullname() function instead of current implementation in patch.

Thanks for the tip Eloy

Patch ported to 2.x, added date/time information and approval status in the same way.

This should fulfill the requirements, although a more sophisticated solution would certainly be possible.

Would be great if someone could have a look at the patch and say whether it can be integrated.

Code looks good to me. Sorry it took so long to peer review

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Hi Henning,
Petr has pointed out that there needs to be a finer control of capability checks here that regulate who can take away the sensitive private data.

Perhaps implementing a new 'mod/data:exportuserinfo' capability with RISK_PERSONAL in the riskbitmask would help here. (similar to 'moodle/backup:userinfo' )

the rest of the patch looks good to me

reopening this for Henning to look at.

Hi Aparup and Steve,

after some time of inactivity, I have now amended the patch. Exporting user details now needs the extra capability 'mod/data:exportuserinfo'. Two remarks on the capabilities:

1) 'mod/data:exportuserinfo' has the risk bit RISK_PERSONAL set. But so have 'mod/data:exportentry' and 'mod/data:exportallentries'.

2) It seems that access to the "Export" facility is not controlled by 'mod/data:exportallentries' but rather by 'mod/data:viewalluserpresets'. See line 58 in mod/data/export.php and line 34 in mod/data/lib.php. This may be a bug, but it is unrelated, so I didn't try to fix it here.

The fix is for master at the moment. I would appreciate if it could be backported to 2.3; it should be a simple cherry-pick, but I'm not sure whether that's OK with the release policy since a capability was added.

Cheers
Henning

Excellent, thanks Henning. We're going to 2.3 in late Sep I think so it'd great if this made it into the release by then.

Looks good to me Henning

Thanks for reviewing - can someone send the patch to integration review, then? Unfortunately, I don't have access to that button.

Hi Henning,
I've noticed that the capability check on 'mod/data:exportuserinfo' is done in export.php, perhaps this would be better done within lib.php:data_get_exportdata() as the capability seems to apply more centrally to that function (even better cap checks for when others call that function).

$userdetails = $userdetails && has_capability('mod/data:exportuserinfo', $context);

since this is for master atm : perhaps we could update the function by passing in context?

Hi Aparup,

now changed as requested - in a way that is (hopefully) backward compatible.

Thanks Henning, that seems to do it

I've integrated that into master only as there is an API change here (tagged as so). I've also tweaked minor whitespaces.

Note that this was affecting 2.2+ and 2.3+ as well but can't be backported due to it being an improvement + api changes.

If there's further need to backport please open a separate issue.

There is no option to "Include approval status"... attaching screen

Require approval was set "no" in my settings...once I set that to "yes" I can see the checkbox and all things works as expected.
Passing
Thanks

For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility!

Many thanks for your collaboration, yay!

Closing, ciao