Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-38703

Moving a hidden item to visible section sets the item to visible in 2.3

    Details

    • Testing Instructions:
      Hide

      Run unit tests

      1. Hide an item using eye icon
      2. Move hidden item to a non-hidden section
      3. VERIFY: that the hidden item is not shown.
      4. Add two course modules to one section, set one hidden, one visible. Note which is hidden and which is not.
      5. Hide the entire section using the eye icon
      6. VERIFY: all course modules are hidden
      7. Unhide the section
      8. VERIFY the visibility of course modules is the same as step 3. With one visible and one not.
      Show
      Run unit tests Hide an item using eye icon Move hidden item to a non-hidden section VERIFY: that the hidden item is not shown. Add two course modules to one section, set one hidden, one visible. Note which is hidden and which is not. Hide the entire section using the eye icon VERIFY: all course modules are hidden Unhide the section VERIFY the visibility of course modules is the same as step 3. With one visible and one not.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE

      Description

      In Moodle 2.3.6

      1. Hide an item
      2. Move hidden item to a non-hidden section

      Item moves to correct location, but is now shown!

      In Moodle 2.3.6, 2.4.3, and Master

      1. Create a new item, setting its visibility to hidden.
      2. Save and return to course.
      3. Move item to a non-hidden section

      Item moves to correct location, but is now shown!

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            poltawski Dan Poltawski added a comment -

            The second half of this bug report is caused by MDL-38713.

            The first half is due to a complex mess of issues, but basically comes down to visibleold not being correctly set in Moodle 2.3.

            Show
            poltawski Dan Poltawski added a comment - The second half of this bug report is caused by MDL-38713 . The first half is due to a complex mess of issues, but basically comes down to visibleold not being correctly set in Moodle 2.3.
            Hide
            poltawski Dan Poltawski added a comment -

            Ok, so I have fixed this by ensuring that visibleold is correctly set in set_coursemodule_visible().

            I've backported the unit tests from 2.4 for this change (with a few changes to match the current API) and before the patch they are broken, after they are fixed. This also fixes problems discovered in the unit tests I wrote for MDL-38700 too.

            Show
            poltawski Dan Poltawski added a comment - Ok, so I have fixed this by ensuring that visibleold is correctly set in set_coursemodule_visible(). I've backported the unit tests from 2.4 for this change (with a few changes to match the current API) and before the patch they are broken, after they are fixed. This also fixes problems discovered in the unit tests I wrote for MDL-38700 too.
            Hide
            poltawski Dan Poltawski added a comment - - edited

            TO INTEGRATOR: this is blocked by MDL-38700 and related to MDL-38703 for non-eye based actions on all branches.

            Show
            poltawski Dan Poltawski added a comment - - edited TO INTEGRATOR: this is blocked by MDL-38700 and related to MDL-38703 for non-eye based actions on all branches.
            Hide
            willylee Willy Lee added a comment -

            Should moving a hidden item get shown just by moving it to a non-hidden section? We have faculty with hidden items inside sections and they move them around expecting them to retain state (hidden or shown) unless they move them to a hidden section. The current testing instructions imply that they should always inherit the visibility of the container.

            So I have an activity and an answer key. I've hidden the answer key. I decide to move both up a week in my weekly course format. I don't think the answer key should get unhidden.

            Show
            willylee Willy Lee added a comment - Should moving a hidden item get shown just by moving it to a non-hidden section? We have faculty with hidden items inside sections and they move them around expecting them to retain state (hidden or shown) unless they move them to a hidden section. The current testing instructions imply that they should always inherit the visibility of the container. So I have an activity and an answer key. I've hidden the answer key. I decide to move both up a week in my weekly course format. I don't think the answer key should get unhidden.
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Willy,

            Should moving a hidden item get shown just by moving it to a non-hidden section?

            No - I think it was a typo in my testing instructions, sorry!

            Show
            poltawski Dan Poltawski added a comment - Hi Willy, Should moving a hidden item get shown just by moving it to a non-hidden section? No - I think it was a typo in my testing instructions, sorry!
            Hide
            willylee Willy Lee added a comment -

            Thanks Dan, that matches how we use it here.

            Show
            willylee Willy Lee added a comment - Thanks Dan, that matches how we use it here.
            Hide
            damyon Damyon Wiese 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.

            Thanks!

            Show
            damyon Damyon Wiese 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. Thanks!
            Hide
            willylee Willy Lee added a comment -

            We've run the functional tests here at Carleton and they pass.

            Show
            willylee Willy Lee added a comment - We've run the functional tests here at Carleton and they pass.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Dan I tested/reviewed MDL-38700, MDL-38703 and MDL-38713 together and they have all been integrated now.

            Show
            damyon Damyon Wiese added a comment - Thanks Dan I tested/reviewed MDL-38700 , MDL-38703 and MDL-38713 together and they have all been integrated now.
            Hide
            andyjdavis Andrew Davis added a comment -

            Works as described. Passing.

            Show
            andyjdavis Andrew Davis added a comment - Works as described. Passing.
            Hide
            poltawski Dan Poltawski added a comment -

            Did you remember to call thankDevelopers() for 'this_weeks_work'? Defaulting to PARAM_SHODDY thanking.

            line 1289 of \lib\changes.php: call to debugging()
            line 281 of \lib\are.php: call to moodleform->detectMissingThanks()
            line 202 of \lib\now.php: call to moodleform->_is_poor_form()
            line 73 of \course\upstream.php: call to moodleform->forgetingToThank()

            Show
            poltawski Dan Poltawski added a comment - Did you remember to call thankDevelopers() for 'this_weeks_work'? Defaulting to PARAM_SHODDY thanking. line 1289 of \lib\changes.php: call to debugging() line 281 of \lib\are.php: call to moodleform->detectMissingThanks() line 202 of \lib\now.php: call to moodleform->_is_poor_form() line 73 of \course\upstream.php: call to moodleform->forgetingToThank()

              People

              • Votes:
                10 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/May/13