I'm providing a patch for master. This adds the requested "disapprove" functionality to both the database and the glossary activity. Approved entries now have a small "disapprove" button, which sends them back to the "not approved" state.

Note that this adds some language strings, but re-uses existing icons.

For the database activity, note that there is now another "special key" ("##disapprove##") to be used in templates, and this key is part of the new standard template, however for existing activities this key will not be inserted automatically.

The current patch is for master only. However, I can provide a backport to 2.5 or earlier (if it can be integrated).

Requesting peer review for this patch.

Henning, thanks for your patch. Hopefully it can be peer reviewed soon.

[Y] Syntax
[N] Whitespace
[Y] Output
[Y] Language
[Y] Databases
[Y] Testing
[Y] Security
[-] Documentation
[Y] Git
[Y] Sanity check

Hello Henning,

Everything here looks quite good. I only have one small comment.

In the new sections that you have created you have tabbed the indents into the code.

• mod/data/lib.php from line 1282 - 1288
• mod/glossary/lib.php from line 1242 - 1243

Once you have changed these to spaces, you will be ready for integration.

Thanks.

Thanks for reviewing this, Adrian.

I have now fixed the whitespace issues, rebased the branch, and provided a 2.5 backport (not sure whether this minor feature change can be integrated into the stable branch, but it's actually only a cherry-pick).

Great, thanks for that.

Submitting for integration.

Hi Henning, just found a bunch of points that would be great to fix before allowing this to land:

1) Some warnings from the code-checker: http://ci.stronk7.com/job/Precheck%20remote%20branch/541/artifact/work/smurf.html

2) The new log action (disapprove entry) has to be added to:

• the corresponding mod/glossary/db/log.php file
• restore. See define_restore_log_rules() for those modules.

Apart from that, it looks perfect. Final detail, I don't think this can land to stables now (new functionality). So this issue will add the new code only for master.

Later you can create a backport request and it will be evaluated.

Well done, ciao

Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

I have now added the log actions; thanks for pointing this out.

I have also fixed the coding style problems as requested - however, now the changes are inconsistent in style with the surrounding code.

Requesting peer review again (seems I can't directly put this up for integration review).

Submitting back to integration.

Integrated to master - thanks Henning!

Great work Henning, passes the tests

This issue along with 77 of it's friends has been sent upstream and released to the world.

Thankyou for your contribution.

FYI QA test on a glossary entry is here MDLQA-6670 (for 2.6; also added to the master) I'm thinking the button should really say 'unapprove' rather than 'disapprove' because they have different meanings.

Removing docs_required as this is documented now in http://docs.moodle.org/26/en/Using_Database and in http://docs.moodle.org/26/en/Using_Glossary

I agree with Mary that we should use 'Unapprove' rather than 'Disapprove' so have changed it in en_fix for Moodle 2.6.1 and also the master copy of the QA test.

Just arguing with Helen that "unapprove", that has landed to a lot of places... is questionable (as a verb, it's correct as "unapproved" adjetive). Perhaps using something like "undo approval" is 100% correct, not ambiguous and means exactly what is expected. Any change will be performed @ MDL-43590, in any case.