Moodle
  1. Moodle
  2. MDL-35711

mod_assign: assignment submission event is not triggered when submit button is not required

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.4
    • Component/s: Assignment
    • Labels:
    • Testing Instructions:
      Hide

      The plagiarism API is not directly testable at the moment so this minimal test should be sufficient.

      1. Create an assignment with "Require students click submit button" set to off and "Online Text" submissions on.
      2. Login as a student and make a submission to the assignment
      3. Verify there are no errors reported in the page
      Show
      The plagiarism API is not directly testable at the moment so this minimal test should be sufficient. Create an assignment with "Require students click submit button" set to off and "Online Text" submissions on. Login as a student and make a submission to the assignment Verify there are no errors reported in the page
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:

      Description

      When pressing submit button is not required (assignment settings), submission event (currently assessable_content_done, see related discussion at MDL-35710) will not be trigged (while the assignment will still be submitted). The problem is related to the fact that event trigger is not called from process_save_submission function.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Ruslan Kabalin added a comment -

            Added Daymon, Kanika and Dan to watch list, as they were those who discussed events in MDL-34544.

            Show
            Ruslan Kabalin added a comment - Added Daymon, Kanika and Dan to watch list, as they were those who discussed events in MDL-34544 .
            Hide
            Kanika Goyal added a comment -

            Hi Ruslan,
            The event is getting triggered on re-submission as well.
            The function process_save_submission is calling save function of each plugin, which in turn is triggering the event.
            Can you please provide the test instructions in case where it is not resubmitting?

            Thanks,
            Kanika

            Show
            Kanika Goyal added a comment - Hi Ruslan, The event is getting triggered on re-submission as well. The function process_save_submission is calling save function of each plugin, which in turn is triggering the event. Can you please provide the test instructions in case where it is not resubmitting? Thanks, Kanika
            Hide
            Dan Marsden added a comment -

            Hi Ruslan - haven't looked at your code in detail but we need a unique event to be triggered when drafts are enabled and the user hits "submit" - If I'm understanding this correctly (my eyes are glazing over so I may not have read this right) your code triggers the same event even when draft submissions are disabled. - That's ok by me as long as there is a way we can easily see which event is being triggered - the "submit for marking" style event is the main one I'm interested in.

            Show
            Dan Marsden added a comment - Hi Ruslan - haven't looked at your code in detail but we need a unique event to be triggered when drafts are enabled and the user hits "submit" - If I'm understanding this correctly (my eyes are glazing over so I may not have read this right) your code triggers the same event even when draft submissions are disabled. - That's ok by me as long as there is a way we can easily see which event is being triggered - the "submit for marking" style event is the main one I'm interested in.
            Hide
            Ruslan Kabalin added a comment - - edited

            @ Kanika Ah, true, confused myself with submit button pressing requirement setting. I have amended the description. So, when submit button is not required, the submission itself still occurs via process_save_submission. The event, however, is not triggered in this case.

            Show
            Ruslan Kabalin added a comment - - edited @ Kanika Ah, true, confused myself with submit button pressing requirement setting. I have amended the description. So, when submit button is not required, the submission itself still occurs via process_save_submission. The event, however, is not triggered in this case.
            Hide
            Ruslan Kabalin added a comment -

            @Dan Hi Dan, I am indeed suggesting using the same event, but it will not be triggered on draft saving, only in the case of when submit button press is not required and the work is actually submitted for grading. While of course we can distinguish between main submission and saving event by adding extra parameter or renaming the latter, this is somehow sounds incorrect, because what happen on process_save_submission is indeed "submission for marking" you are asking for, just without extra confirmation by pressing submit button.

            Show
            Ruslan Kabalin added a comment - @Dan Hi Dan, I am indeed suggesting using the same event, but it will not be triggered on draft saving, only in the case of when submit button press is not required and the work is actually submitted for grading. While of course we can distinguish between main submission and saving event by adding extra parameter or renaming the latter, this is somehow sounds incorrect, because what happen on process_save_submission is indeed "submission for marking" you are asking for, just without extra confirmation by pressing submit button.
            Hide
            Kanika Goyal added a comment -

            Ruslan, The event is getting triggered even if submit button is not required as in
            the process_save_submission there is code -

             
            if ($plugin->is_enabled()) {
                if (!$plugin->save($submission, $data)) {
                    print_error($plugin->get_error());
                }
            }
            

            and this save function will trigger the event.
            I am not sure if I am getting it right.
            Thanks,
            Kanika

            Show
            Kanika Goyal added a comment - Ruslan, The event is getting triggered even if submit button is not required as in the process_save_submission there is code - if ($plugin->is_enabled()) { if (!$plugin->save($submission, $data)) { print_error($plugin->get_error()); } } and this save function will trigger the event. I am not sure if I am getting it right. Thanks, Kanika
            Hide
            Ruslan Kabalin added a comment -

            Hi Kanika, just to be clear, I am talking about event trigger currently called "assessable_content_done" (which I suggest to rename in MDL-35710 to make it more specific). This event is currently trigged only from process_submit_for_grading() function. When pressing submit button is not compulsory in the assignment settings, and user is submitting something, the submission will take place (record will be added to mdl_assign_submission and teacher will be able to grade it), but no event will be triggered.

            Show
            Ruslan Kabalin added a comment - Hi Kanika, just to be clear, I am talking about event trigger currently called "assessable_content_done" (which I suggest to rename in MDL-35710 to make it more specific). This event is currently trigged only from process_submit_for_grading() function. When pressing submit button is not compulsory in the assignment settings, and user is submitting something, the submission will take place (record will be added to mdl_assign_submission and teacher will be able to grade it), but no event will be triggered.
            Hide
            Dan Marsden added a comment -

            Thanks Ruslan - as mentioned on the other bug - adding a new event is fine - as long as we can differentiate between them. - We don't care about submission as such - only send for marking. - perhaps a better name for the other event could have been "send_for_marking" - but we were trying to use something generic that could be used by other modules in future.

            So either different event names, or different params in the events to allow us to differentiate them is needed for us - thanks.

            Show
            Dan Marsden added a comment - Thanks Ruslan - as mentioned on the other bug - adding a new event is fine - as long as we can differentiate between them. - We don't care about submission as such - only send for marking. - perhaps a better name for the other event could have been "send_for_marking" - but we were trying to use something generic that could be used by other modules in future. So either different event names, or different params in the events to allow us to differentiate them is needed for us - thanks.
            Hide
            Damyon Wiese added a comment -

            Inline with the changes discussed in MDL-35710 this patch will need the params added to indicate that the submission is still editable.

            Show
            Damyon Wiese added a comment - Inline with the changes discussed in MDL-35710 this patch will need the params added to indicate that the submission is still editable.
            Hide
            Ruslan Kabalin added a comment -

            Added parameter, as discussed in MDL-35710.

            This patch needs to be applied after MDL-35710

            Show
            Ruslan Kabalin added a comment - Added parameter, as discussed in MDL-35710 . This patch needs to be applied after MDL-35710
            Hide
            Damyon Wiese added a comment -

            Setting the fix version to 2.4 as we should not change API's for stable releases.

            I also added a minor patch to fix the comment formatting.

            Show
            Damyon Wiese added a comment - Setting the fix version to 2.4 as we should not change API's for stable releases. I also added a minor patch to fix the comment formatting.
            Hide
            Damyon Wiese added a comment -

            This looks good to me. Thanks Ruslan.

            Show
            Damyon Wiese added a comment - This looks good to me. Thanks Ruslan.
            Hide
            Dan Poltawski added a comment -

            I've integrated this now, thanks guys.

            (I had to cherry-pick the commits because wasn't pulling in cleanlfor some reason)

            Show
            Dan Poltawski added a comment - I've integrated this now, thanks guys. (I had to cherry-pick the commits because wasn't pulling in cleanlfor some reason)
            Hide
            Ankit Agarwal added a comment -

            Works as expected.
            Thanks!

            Show
            Ankit Agarwal added a comment - Works as expected. Thanks!
            Hide
            Aparup Banerjee added a comment -

            Your issue has dug up some gold.
            It works great i've been told.
            Go forth, be brave, be bold.

            yay! "All your thoughts are belong to everyone."

            Thanks and ciao!

            Show
            Aparup Banerjee added a comment - Your issue has dug up some gold. It works great i've been told. Go forth, be brave, be bold. yay! "All your thoughts are belong to everyone." Thanks and ciao!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: