Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #46: add callbacks for modifying a form #51

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dmitriim
Copy link

@dmitriim dmitriim commented Dec 29, 2022

This pull requests adds following callbacks to let other plugins to extend edit dates form the same way as they can extend core moodle activity forms:

  • report_editdates_form_elements
  • report_editdates_form_definition_after_data
  • report_editdates_form_validation
  • report_editdates_form_post_actions

This is required, if for example some customisation has been been applied to an activity form to manipulate dates fields (e.g. to limit a possible time options for an assignment due date) and we would like to apply the same customisation to edit dates form to make sure that academics receive a consistent experience.

@cameron1729
Copy link

Thanks @dmitriim - these changes make sense to me. I looked at them in tandum with catalyst/moodle-local_submissionrestrict#3 which helped to give me some context for the how the hooks are intended to be used. Code gets my +1

@timhunt
Copy link
Member

timhunt commented Jan 1, 2023

So, err, you have done something. Apparently, if, like Cameron, I go and read some other code, I might be able to work out what you aren doing and why.

Or, here is an idea. If you are proposing a change in some code, why don't you explain what you are doing, and why, then I will review it.

So far, it seems like you know exactly what you are doing and why, and you think you have explained it, but really you have not. So, please try to actually explain.

@dmitriim
Copy link
Author

@timhunt I have added more information to the description of the pull request. Hope it makes more sense now.

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.

3 participants