Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

      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)"

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              poltawski 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
              poltawski 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
              andyjdavis 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
              andyjdavis 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
              andyjdavis 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
              andyjdavis 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
              andyjdavis 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
              andyjdavis 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
              andyjdavis Andrew Davis added a comment -

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

              Show
              andyjdavis Andrew Davis added a comment - Updated branch and added testing instructions. Putting this up for peer review.
              Hide
              abgreeve 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
              abgreeve 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
              poltawski 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
              poltawski 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
              dougiamas Martin Dougiamas added a comment -

              Yep, fine. +1

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

              Thanks Andrew, integrated now.

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

              Tested and passed.

              Show
              poltawski Dan Poltawski added a comment - Tested and passed.
              Hide
              stronk7 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
              stronk7 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:
                    Fix Release Date:
                    25/Jun/12