Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-39303

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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 Master Branch:
      MDL-39303_master

      Description

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

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            andyjdavis 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
            andyjdavis 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
            markn 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
            markn 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
            markn Mark Nelson added a comment -

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

            Show
            markn Mark Nelson added a comment - Hi Andrew, I made the changes you suggested. I am now submitting to integration.
            Hide
            samhemelryk 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
            samhemelryk 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
            rwijaya Rossiani Wijaya added a comment -

            This is working as expected.

            Tested for 2.3, 2.4 and master

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - This is working as expected. Tested for 2.3, 2.4 and master Test passed.
            Hide
            poltawski 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
            poltawski 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:
                  Fix Release Date:
                  13/May/13