Moodle
  1. Moodle
  2. MDL-35568

Creating new resources via DND in a hidden section sets visibility incorrectly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide
      • Drag a new file into a visible section
      • Confirm that the new file is visible
      • Hide the section
      • Confirm that both the new file and the section as a whole appear hidden
      • Drag another new file into the now hidden section
      • Confirm that the new file appears hidden
      • Toggle the section visibility
      • Confirm that both new files and the section as a whole appear visible
      Show
      Drag a new file into a visible section Confirm that the new file is visible Hide the section Confirm that both the new file and the section as a whole appear hidden Drag another new file into the now hidden section Confirm that the new file appears hidden Toggle the section visibility Confirm that both new files and the section as a whole appear visible
    • Workaround:
      Hide

      Unhide the section, toggle the resource and then hide the section again.

      Edit the resource's settings and change the visibility there.

      Show
      Unhide the section, toggle the resource and then hide the section again. Edit the resource's settings and change the visibility there.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-35568_dndupload_hidden
    • Rank:
      44281

      Description

      If a section is hidden, and you drag/drop a new resource into it, the new resource is created with visible of 1.
      When you unhide the section, the resource visibility goes from 1 (visible) to 0 (hidden).

      Since it's not easy to change the visibility of a resource within a hidden section, this could lead to confusion, chaos and eventually anarchy.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Hi, Andrew.

          Could you please expand on this? I tried creating a resource in a hidden section and its default visibility was hidden.

          Or is this a double-negative thing?: "incorrectly sets visibility incorrectly"

          I did note that when using drag-and-drop, resources are always set to visible. Is that what you are referring to?

          Show
          Michael de Raadt added a comment - Hi, Andrew. Could you please expand on this? I tried creating a resource in a hidden section and its default visibility was hidden. Or is this a double-negative thing?: "incorrectly sets visibility incorrectly" I did note that when using drag-and-drop, resources are always set to visible. Is that what you are referring to?
          Hide
          Andrew Nicols added a comment -

          Hi, Michael.

          Sorry about that - being a bit rusty at the moment as I've been really busy.
          I've updated the title and description, and have provided test instructions for when the issue is fixed.

          Show
          Andrew Nicols added a comment - Hi, Michael. Sorry about that - being a bit rusty at the moment as I've been really busy. I've updated the title and description, and have provided test instructions for when the issue is fixed.
          Hide
          Michael de Raadt added a comment -

          Thanks for the additional information.

          Show
          Michael de Raadt added a comment - Thanks for the additional information.
          Hide
          Rex Lorenzo added a comment -

          Shouldn't this be raised a security issue? With us promoting drag and drop as the best way to upload files, we have run into problems in which a user can drag and drop files into a hidden section via drag and drop, but the files are visible in the resource listing page: /mod/resource/index.php.

          This caused some sensitive content being released.

          Show
          Rex Lorenzo added a comment - Shouldn't this be raised a security issue? With us promoting drag and drop as the best way to upload files, we have run into problems in which a user can drag and drop files into a hidden section via drag and drop, but the files are visible in the resource listing page: /mod/resource/index.php. This caused some sensitive content being released.
          Hide
          Michael de Raadt added a comment -

          Hi, Rex.

          This is a bug in behaviour, but I don't think this is a security issue.

          Thanks for launching the linked issue.

          Show
          Michael de Raadt added a comment - Hi, Rex. This is a bug in behaviour, but I don't think this is a security issue. Thanks for launching the linked issue.
          Hide
          Davo Smith added a comment -

          The attached patch checks the section visibility and uses it to set the module visibility (but it always sets the 'visibleold' to '1', so that the resource will unhide when the section is unhidden).

          BTW, whilst working on this, I noticed that creating activities/resources in a hidden section defaults to 'hidden' with 'visibleold' set to hidden as well - there doesn't seem to be a way to create a resource in a hidden section that will unhide when the section is unhidden (other than, with this patch, via drag and drop).

          Show
          Davo Smith added a comment - The attached patch checks the section visibility and uses it to set the module visibility (but it always sets the 'visibleold' to '1', so that the resource will unhide when the section is unhidden). BTW, whilst working on this, I noticed that creating activities/resources in a hidden section defaults to 'hidden' with 'visibleold' set to hidden as well - there doesn't seem to be a way to create a resource in a hidden section that will unhide when the section is unhidden (other than, with this patch, via drag and drop).
          Hide
          Damyon Wiese added a comment -

          This patch cleanly cherry-picks back to 23 (no drag and drop in 22).

          Show
          Damyon Wiese added a comment - This patch cleanly cherry-picks back to 23 (no drag and drop in 22).
          Hide
          Damyon Wiese added a comment -

          I linked a new issue for the 'visibleold' problem you found Davo (I agree it should work like it does in this patch).

          Show
          Damyon Wiese added a comment - I linked a new issue for the 'visibleold' problem you found Davo (I agree it should work like it does in this patch).
          Hide
          Damyon Wiese added a comment -

          Peer Review Checklist:

          [W] Syntax - Very minor comment issue - comments missing fullstops.
          [-] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [Y] Testing
          [-] Security
          [-] Documentation
          [Y] Git - (Cleanly cherry picks back to 23)
          [Y] Sanity check

          Thanks Davo - Sending for integration review.

          Show
          Damyon Wiese added a comment - Peer Review Checklist: [W] Syntax - Very minor comment issue - comments missing fullstops. [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git - (Cleanly cherry picks back to 23) [Y] Sanity check Thanks Davo - Sending for integration review.
          Hide
          Damyon Wiese added a comment -

          Sorry - pushed wrong branch.

          Show
          Damyon Wiese added a comment - Sorry - pushed wrong branch.
          Hide
          Damyon Wiese added a comment -

          Actually - there is a problem with this.

          It only adds the dimmed class to the resource - it does not update the show/hide links etc.

          See toggle_hide_resource_ui in course/yui/toolboxes/toolboxes.js for the full list of what needs to be done (really should call that function).

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Actually - there is a problem with this. It only adds the dimmed class to the resource - it does not update the show/hide links etc. See toggle_hide_resource_ui in course/yui/toolboxes/toolboxes.js for the full list of what needs to be done (really should call that function). Regards, Damyon
          Hide
          Davo Smith added a comment -

          Damyon - I'm not quite sure what is missing from my code.

          As far as I can tell the steps that are needed are:

          1. Add the 'dimmed' class - done by my javascript
          2. Change the link + alt tag on the show/hide icon - not needed as the the action icons were generated on the server with the correct status
          3. Cope with conditional hiding of icons - not an issue as a newly drag & dropped resource can never have conditional hiding applied (as there has been no opportunity to open up the settings and apply this)
          4. Toggle the availability icon status - again, not an issue as custom availability could not have been configured for a newly drag and dropped icon.

          Please let me know if I've missed some subtlety in the code that I haven't already accounted for.

          Show
          Davo Smith added a comment - Damyon - I'm not quite sure what is missing from my code. As far as I can tell the steps that are needed are: 1. Add the 'dimmed' class - done by my javascript 2. Change the link + alt tag on the show/hide icon - not needed as the the action icons were generated on the server with the correct status 3. Cope with conditional hiding of icons - not an issue as a newly drag & dropped resource can never have conditional hiding applied (as there has been no opportunity to open up the settings and apply this) 4. Toggle the availability icon status - again, not an issue as custom availability could not have been configured for a newly drag and dropped icon. Please let me know if I've missed some subtlety in the code that I haven't already accounted for.
          Hide
          Damyon Wiese added a comment -

          MDL-37430 includes a change to make toggle_hide_resource_ui easier to call. Suggest we wait for that (or base this on top of that patch).

          Show
          Damyon Wiese added a comment - MDL-37430 includes a change to make toggle_hide_resource_ui easier to call. Suggest we wait for that (or base this on top of that patch).
          Hide
          Damyon Wiese added a comment -

          Sorry Davo - I thought the show/hide icons would need updating but you're right it does work.

          Thanks - sending for integration now.

          Show
          Damyon Wiese added a comment - Sorry Davo - I thought the show/hide icons would need updating but you're right it does work. Thanks - sending for integration now.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Sam Hemelryk added a comment -

          Thanks guys this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks guys this has been integrated now.
          Hide
          Frédéric Massart added a comment -

          Test successful on master. Thanks!

          Show
          Frédéric Massart added a comment - Test successful on master. Thanks!
          Hide
          Dan Poltawski added a comment -

          Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

          Show
          Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: