Moodle
  1. Moodle
  2. MDL-45483

Forum maximum number of attachments invalid

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.9, 2.5.5, 2.6.2, 2.7
    • Fix Version/s: 2.5.7, 2.6.4, 2.7.1
    • Component/s: Forum
    • Labels:

      Description

      I have set the maximum number of attachments in a standard forum to 20, but it only allows 11 attachments.

      It looks to be a basic error in the attachments dropdown code. Here's a snippet from the forum editing page:

      <select name="maxattachments" id="id_maxattachments">
      <option value="0">0</option>
      <option value="1">1</option>
      <option value="2">2</option>
      <option value="3">3</option>
      <option value="4">4</option>
      <option value="5">5</option>
      <option value="6">6</option>
      <option value="7">7</option>
      <option value="8">8</option>
      <option value="9">9</option>
      <option value="10">10</option>
      <option value="11" selected="selected">20</option>
      <option value="12">50</option>
      <option value="13">100</option>
      </select>

        Gliffy Diagrams

          Activity

          Hide
          Kevin Wiliarty added a comment -

          I can see what the problem is here. The select box uses the array keys for the values, and in this case the keys are implicitly just ordered numbers. The display options start to make jumps, but the keys do not. We can fix this by specifying keys that match the values. I'll post a patch soon.

          Show
          Kevin Wiliarty added a comment - I can see what the problem is here. The select box uses the array keys for the values, and in this case the keys are implicitly just ordered numbers. The display options start to make jumps, but the keys do not. We can fix this by specifying keys that match the values. I'll post a patch soon.
          Hide
          Charles Fulton added a comment -

          Verified that this is broken in 2.7.

          Show
          Charles Fulton added a comment - Verified that this is broken in 2.7.
          Hide
          Kevin Wiliarty added a comment -

          I've written a fix for master, but I can also backport if the fix is right.

          Show
          Kevin Wiliarty added a comment - I've written a fix for master, but I can also backport if the fix is right.
          Hide
          CiBoT added a comment -
          Show
          CiBoT added a comment - Results for MDL-45483 Remote repository: https://github.com/kwiliarty/moodle.git Remote branch wip- MDL-45483 -master to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3357 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/3357/artifact/work/smurf.html
          Hide
          Ankit Agarwal added a comment -

          Thanks Kevin for the patch,
          looks great. Just a minor code style issue http://docs.moodle.org/dev/Coding_style#Wrapping_Arrays

          It would be awesome if you can provide backport branches (please note that we don't bump the first 8 digits of the version in stable branches)

          Show
          Ankit Agarwal added a comment - Thanks Kevin for the patch, looks great. Just a minor code style issue http://docs.moodle.org/dev/Coding_style#Wrapping_Arrays It would be awesome if you can provide backport branches (please note that we don't bump the first 8 digits of the version in stable branches)
          Hide
          Kevin Wiliarty added a comment -

          Thanks for the speedy replay, Ankit.

          I've fixed the coding style for the associative arrays. In the meantime, master moved along with a lot of version bumps, so I incremented my version by 1 over what core currently has in master. I'm not sure whether that was the right approach, so just let me know if it ought to be something different. It's all rebased and force pushed.

          I also put together backports for 2.6 and 2.5 (against the latest from core). The version bumps on those are incremented by just 1.

          Show
          Kevin Wiliarty added a comment - Thanks for the speedy replay, Ankit. I've fixed the coding style for the associative arrays. In the meantime, master moved along with a lot of version bumps, so I incremented my version by 1 over what core currently has in master. I'm not sure whether that was the right approach, so just let me know if it ought to be something different. It's all rebased and force pushed. I also put together backports for 2.6 and 2.5 (against the latest from core). The version bumps on those are incremented by just 1.
          Hide
          Ankit Agarwal added a comment -

          Thanks Kevin, looks good to me.

          Pushing for integration.

          Show
          Ankit Agarwal added a comment - Thanks Kevin, looks good to me. Pushing for integration.
          Hide
          Kevin Wiliarty added a comment -

          Just added a pull branch name and diff URL for 2.7

          Show
          Kevin Wiliarty added a comment - Just added a pull branch name and diff URL for 2.7
          Hide
          Marina Glancy added a comment - - edited

          Thanks Kevin, this was integrated in 2.5, 2.6, 2.7 and master

          edited: we still integrate in 2.5 for two more weeks if the issue was submitted before the freeze with 2.5 branch

          Show
          Marina Glancy added a comment - - edited Thanks Kevin, this was integrated in 2.5, 2.6, 2.7 and master edited: we still integrate in 2.5 for two more weeks if the issue was submitted before the freeze with 2.5 branch
          Hide
          Kevin Wiliarty added a comment -

          Thanks, Marina and all.

          Show
          Kevin Wiliarty added a comment - Thanks, Marina and all.
          Hide
          Frédéric Massart added a comment -

          Passing, thanks.

          Show
          Frédéric Massart added a comment - Passing, thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          So, finally, this landed, making Moodle better, many thanks!

          Be patient and understanding.
          Life is too short to be
          vengeful or malicious.

          Phillips Brooks

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - So, finally, this landed, making Moodle better, many thanks! Be patient and understanding. Life is too short to be vengeful or malicious. Phillips Brooks Closing as fixed, ciao
          Hide
          Kevin Wiliarty added a comment -

          Show
          Kevin Wiliarty added a comment -

            People

            • Votes:
              9 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: