Moodle
  1. Moodle
  2. MDL-39303

Exceeding forum post threshold for blocking does not prevent user from resubmitting form.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.6, 2.4
    • Fix Version/s: 2.3.7, 2.4.4
    • Component/s: Forum
    • Labels:
    • Testing Instructions:
      Hide

      Test 1

      1. Login as a teacher and create a forum with a time period for blocking set.
      2. Set a post threshold for blocking and for warning.
      3. Log in as a student and post until you are on your last message, which will push your post count to the threshold for blocking when posted.
      4. Post this message, then on the screen where it says "Your post was successfully added." click the back button in the browser and fill in the post again and resubmit.
      5. Ensure when you resubmit you are given a warning about exceeding the limit.

      Test 2

      1. Login as a teacher and create a forum with a time period for blocking set.
      2. Set a post threshold for blocking and for warning.
      3. Log in as a student and click on to add a new discussion, but do not submit this form.
      4. Open another tab in your browser and create a discussion and continue replying until you reach the threshold for blocking.
      5. Go back to the other tab and attempt to post a new discussion.
      6. Ensure when you submit you are given a warning about exceeding the limit.
      Show
      Test 1 Login as a teacher and create a forum with a time period for blocking set. Set a post threshold for blocking and for warning. Log in as a student and post until you are on your last message, which will push your post count to the threshold for blocking when posted. Post this message, then on the screen where it says "Your post was successfully added." click the back button in the browser and fill in the post again and resubmit. Ensure when you resubmit you are given a warning about exceeding the limit. Test 2 Login as a teacher and create a forum with a time period for blocking set. Set a post threshold for blocking and for warning. Log in as a student and click on to add a new discussion, but do not submit this form. Open another tab in your browser and create a discussion and continue replying until you reach the threshold for blocking. Go back to the other tab and attempt to post a new discussion. Ensure when you submit you are given a warning about exceeding the limit.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-39303_m24
    • Pull Master Branch:
      MDL-39303_master
    • Rank:
      49924

      Description

      It is possible to exceed the total number of allowed posts by resubmitting the form because the forum code is so awesome.

        Issue Links

          Activity

          Hide
          Andrew Davis added a comment -

          Personally I really dislike this type or wrapping

          print_error($thresholdwarning->errorcode, $thresholdwarning->module, $thresholdwarning->link,
              $thresholdwarning->additional);
          

          imo, either put it all on one line or wrap it properly.

          print_error($thresholdwarning->errorcode,
                      $thresholdwarning->module,
                      $thresholdwarning->link,
                      $thresholdwarning->additional);
          

          Just personal opinion of course

          This looks suspiciously like code has been copied and pasted when it should probably go into a function.

          private function print_threshold_warning($thresholdwarning) {
              // Check that the user will not exceed the blocking threshold.
              if (!empty($thresholdwarning) && !$thresholdwarning->canpost) {
                  print_error($thresholdwarning->errorcode,
                              $thresholdwarning->module,
                              $thresholdwarning->link,
                              $thresholdwarning->additional);
          
              } 
          }
          .
          .
          print_threshold_warning($thresholdwarning);
          .
          .
          

          Otherwise its fine

          Show
          Andrew Davis added a comment - Personally I really dislike this type or wrapping print_error($thresholdwarning->errorcode, $thresholdwarning->module, $thresholdwarning->link, $thresholdwarning->additional); imo, either put it all on one line or wrap it properly. print_error($thresholdwarning->errorcode, $thresholdwarning->module, $thresholdwarning->link, $thresholdwarning->additional); Just personal opinion of course This looks suspiciously like code has been copied and pasted when it should probably go into a function. private function print_threshold_warning($thresholdwarning) { // Check that the user will not exceed the blocking threshold. if (!empty($thresholdwarning) && !$thresholdwarning->canpost) { print_error($thresholdwarning->errorcode, $thresholdwarning->module, $thresholdwarning->link, $thresholdwarning->additional); } } . . print_threshold_warning($thresholdwarning); . . Otherwise its fine
          Hide
          Mark Nelson added a comment -

          Thanks Andrew,

          Thanks for your input. I did consider adding it into a function, but it is simply an if statement so decided against it. However, the print_error does take up a few lines, so I guess there is no harm. However, I am going to wait until MDL-39192 gets integrated before continuing work on this, as this patch will conflict with the recent changes I have done on that.

          Show
          Mark Nelson added a comment - Thanks Andrew, Thanks for your input. I did consider adding it into a function, but it is simply an if statement so decided against it. However, the print_error does take up a few lines, so I guess there is no harm. However, I am going to wait until MDL-39192 gets integrated before continuing work on this, as this patch will conflict with the recent changes I have done on that.
          Hide
          Mark Nelson added a comment -

          Hi Andrew, I made the changes you suggested. I am now submitting to integration.

          Show
          Mark Nelson added a comment - Hi Andrew, I made the changes you suggested. I am now submitting to integration.
          Hide
          Sam Hemelryk added a comment -

          Thanks Mark, your fix has been integrated now.

          That area is one whopping great mess, hopefully we'll see the new forum in the near future!

          Show
          Sam Hemelryk added a comment - Thanks Mark, your fix has been integrated now. That area is one whopping great mess, hopefully we'll see the new forum in the near future!
          Hide
          Rossiani Wijaya added a comment -

          This is working as expected.

          Tested for 2.3, 2.4 and master

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working as expected. Tested for 2.3, 2.4 and master Test passed.
          Hide
          Dan Poltawski added a comment -

          Thanks! You're changes are now spread to the world through this git and our source control repositories.

          No time to rest though, we've got days to make 2.5 the best yet!

          ciao

          Show
          Dan Poltawski added a comment - Thanks! You're changes are now spread to the world through this git and our source control repositories. No time to rest though, we've got days to make 2.5 the best yet! ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: