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:
    • Rank:
      44455

      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.

        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: