Moodle
  1. Moodle
  2. MDL-33717

File manager should display a better message when the user is exempt from file size limits

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Filepicker, Repositories
    • Labels:
    • Testing Instructions:
      Hide

      As admin go My Private Files. At the top right of the file manager thing you should see "Maximum size for new files: Unlimited - drag and drop available"

      Log in as a student and go to My Private Files. Now you should see something like "Maximum size for new files: 5MB - drag and drop available" The actual number there will depend on your specific site settings. Just make sure that a number is displayed.

      Show
      As admin go My Private Files. At the top right of the file manager thing you should see "Maximum size for new files: Unlimited - drag and drop available" Log in as a student and go to My Private Files. Now you should see something like "Maximum size for new files: 5MB - drag and drop available" The actual number there will depend on your specific site settings. Just make sure that a number is displayed.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-33717_admin_message2
    • Rank:
      41724

      Description

      If a user with the capability moodle/course:ignorefilesizelimits goes to their private files they see a message like "Maximum size for new files: -1 bytes - drag and drop available"

      It should be something like "Maximum size for new files: unlimited - drag and drop available"

      To be even more useful I suggest the drag and drop message mentions the server file limit:

      "Maximum size for new files: unlimited - drag and drop available (up to 8Mb)"

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          Proof of concept patch for this:

          diff --git a/lib/moodlelib.php b/lib/moodlelib.php
          index 6978a42..d487e6d 100644
          --- a/lib/moodlelib.php
          +++ b/lib/moodlelib.php
          @@ -6045,6 +6045,10 @@ function display_size($size) {
           
               static $gb, $mb, $kb, $b;
           
          +    if ($size === -1) {
          +        return 'Unlimited (subject server limit XXX)';
          +    }
          +
               if (empty($gb)) {
                   $gb = get_string('sizegb');
                   $mb = get_string('sizemb');
          
          Show
          Dan Poltawski added a comment - Proof of concept patch for this: diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 6978a42..d487e6d 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -6045,6 +6045,10 @@ function display_size($size) { static $gb, $mb, $kb, $b; + if ($size === -1) { + return 'Unlimited (subject server limit XXX)'; + } + if (empty($gb)) { $gb = get_string('sizegb'); $mb = get_string('sizemb');
          Hide
          Andrew Davis added a comment -

          Added a branch. The commit is at https://github.com/andyjdavis/moodle/commit/5afbc46617049f338aa666da3585b49e5afca207

          This isn't usable until MDL-27156 is all done. Assuming that the constant is integrated there I'll modify this fix to use it.

          Show
          Andrew Davis added a comment - Added a branch. The commit is at https://github.com/andyjdavis/moodle/commit/5afbc46617049f338aa666da3585b49e5afca207 This isn't usable until MDL-27156 is all done. Assuming that the constant is integrated there I'll modify this fix to use it.
          Hide
          Andrew Davis added a comment -

          Also, I used your fix Dan, sort of. I removed "(subject server limit XXX)". This is not a perfect solution. The problem is that PHP's file limit limits uploads but not files from some other repositories. We can either tell the user they're limited when they're actually only limited sometimes. Or leave out the sometimes limitation message.

          I haven't been able to come up with anything that concisely explains the situation.

          Show
          Andrew Davis added a comment - Also, I used your fix Dan, sort of. I removed "(subject server limit XXX)". This is not a perfect solution. The problem is that PHP's file limit limits uploads but not files from some other repositories. We can either tell the user they're limited when they're actually only limited sometimes. Or leave out the sometimes limitation message. I haven't been able to come up with anything that concisely explains the situation.
          Hide
          Andrew Davis added a comment -

          Ive closed a duplicate issues that was marked as "must fix for 2.3". Therefore I have conferred the same status on this issue.

          Show
          Andrew Davis added a comment - Ive closed a duplicate issues that was marked as "must fix for 2.3". Therefore I have conferred the same status on this issue.
          Hide
          Andrew Davis added a comment -

          Updated branch and added testing instructions. Putting this up for peer review.

          Show
          Andrew Davis added a comment - Updated branch and added testing instructions. Putting this up for peer review.
          Hide
          Adrian Greeve added a comment -

          Hi Andrew,

          I don't see any problem with the code and I agree with the fact that the user is not restricted by the server upload limits, as using a different repository will get around this restriction.

          As for the message: I think that "Unlimited" is fine. I can think of a few others, but for something concise (even though not totally accurate) I think this will do.

          Show
          Adrian Greeve added a comment - Hi Andrew, I don't see any problem with the code and I agree with the fact that the user is not restricted by the server upload limits, as using a different repository will get around this restriction. As for the message: I think that "Unlimited" is fine. I can think of a few others, but for something concise (even though not totally accurate) I think this will do.
          Hide
          Dan Poltawski added a comment -

          Martin - can we have a +1 for this? Its surely better than displaying -1, though is not ideal for the long term.

          Show
          Dan Poltawski added a comment - Martin - can we have a +1 for this? Its surely better than displaying -1, though is not ideal for the long term.
          Hide
          Martin Dougiamas added a comment -

          Yep, fine. +1

          Show
          Martin Dougiamas added a comment - Yep, fine. +1
          Hide
          Dan Poltawski added a comment -

          Thanks Andrew, integrated now.

          Show
          Dan Poltawski added a comment - Thanks Andrew, integrated now.
          Hide
          Dan Poltawski added a comment -

          Tested and passed.

          Show
          Dan Poltawski added a comment - Tested and passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay!

          Many, many thanks for your hard work!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay! Many, many thanks for your hard work! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: