search

mdjnelson / moodle-mod_customcert

Enables the creation of dynamically generated certificates with complete customisation via the web browser.
https://moodle.org/plugins/mod_customcert
GNU General Public License v3.0
90 stars 158 forks source link

Trigger event when certificate is changed #518

Closed leonstr closed 1 year ago

leonstr commented 1 year ago

If someone edits a certificate layout or edits an element I think there should be an event triggered resulting in an entry in the Moodle logs.

For example: Time: 17 Oct 2022 15:24, Component: Custom certificate, Description: The user with id '2' edited the certificate of 'customcert' activity with course module id '485'.

mdjnelson commented 1 year ago

Feel free to create a MR and I will review it. :)

mdjnelson commented 1 year ago

Merged. Thanks.

leonstr commented 1 year ago

Thanks Mark. Apologies for the work needed on top of the PR.

mdjnelson commented 1 year ago

That's OK @leonstr. I got ahead of myself thinking it would be quick to add a bunch more of events and unit tests but no, not a small task at all. 😄

If you could however look at the changes (https://github.com/mdjnelson/moodle-mod_customcert/commits/MOODLE_400_STABLE) I made and give me a +1 here that would be great since no one reviewed them. 😄

leonstr commented 1 year ago

Hi @mdjnelson there are four issues I can see:

  1. Creating a new certificate or template results in both a template_created and a template_updated event. The latter is a result of add_page() triggering both a page_created and a template_updated. Having both template_created and template_updated on creating a template doesn't make sense to me as 'create' is one action.
  2. Adding a new element results in two template_updated events. The second one is a result of save_page() triggering both a page_updated and a template_updated. Again, two events -- this time the same event -- for one action feels like a bug.
  3. If the certificate has more than one page and you add an element to one page then a page_updated is triggered for each page even though only one has been changed.
  4. For the element_* and page_* events I think get_description() should use 'the', not 'a'/'an', e.g. The user with id '...' updated the page with id '...', not The user with id '...' updated a page with id '...'. Trivial but the later reads wrong to me, and it seems to be what Moodle uses, e.g. for course events.

I'll try to find time to see if I can patch these in which case I'll open an issue with a PR for you to review. For 1. and 2. it's not a case of simply removing the template_created and template_updated from add_page() and save_page(), otherwise there'd be no template_updated when you click + Add page.

mdjnelson commented 1 year ago

Thanks a lot @leonstr, it would be much appreciated if you could create a MR for this.

mdjnelson commented 1 year ago

@leonstr I have created https://github.com/mdjnelson/moodle-mod_customcert/issues/564.