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

          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