Moodle
  1. Moodle
  2. MDL-33192

Drag-drop global listener may conflict with other Drag-drop instances

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Course
    • Labels:
      None
    • Testing Instructions:
      Hide

      For regression testing, make sure that drag-drop of sections, resources and blocks still work as before after this patch is applied.

      To test the feature itself, try adding a separate drag-drop instance on the page that already uses moodle-core-dragdrop module (e.g. course/view.php). You may use MDL-33292 for testing, it will not work without this patch.

      Show
      For regression testing, make sure that drag-drop of sections, resources and blocks still work as before after this patch is applied. To test the feature itself, try adding a separate drag-drop instance on the page that already uses moodle-core-dragdrop module (e.g. course/view.php). You may use MDL-33292 for testing, it will not work without this patch.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33192-master-1
    • Rank:
      41113

      Description

      Drag-drop global listener may conflict with other Drag-drop instances that do not use moodle-core-dragdrop YUI module. inGroup check should reply on drag instance where applicable, not on drop instance only (inGroup). Custom inGroup function for drag instances needs to be added to moodle-core-dragdrop module.

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Hi Ruslan,

          The changes look fine. I have to admit without seeing the conflict I can only speculate on its need or effectiveness.
          However as this doesn't appear to break anything I think get it up for integration.
          Although could you please update the test instruction to include testing the two areas moodle-core-dragdrop is being used so that we can at least be positive its not breaking anything.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Ruslan, The changes look fine. I have to admit without seeing the conflict I can only speculate on its need or effectiveness. However as this doesn't appear to break anything I think get it up for integration. Although could you please update the test instruction to include testing the two areas moodle-core-dragdrop is being used so that we can at least be positive its not breaking anything. Cheers Sam
          Hide
          Andrew Nicols added a comment -

          FYI, something that was breaking without this fix was MDL-33292

          Show
          Andrew Nicols added a comment - FYI, something that was breaking without this fix was MDL-33292
          Hide
          Ruslan Kabalin added a comment -

          Thanks Sam for revision, testing instruction has been updated. Ready for integration now.

          Show
          Ruslan Kabalin added a comment - Thanks Sam for revision, testing instruction has been updated. Ready for integration now.
          Hide
          Dan Poltawski added a comment -

          Ruslan, I don't think you have taken what Sam was getting at with he testing instructions. What could break by doing this change? Where should we test for regressions?

          Until the related issue is integrated the important point is that we don't introduce regressions.

          Show
          Dan Poltawski added a comment - Ruslan, I don't think you have taken what Sam was getting at with he testing instructions. What could break by doing this change? Where should we test for regressions? Until the related issue is integrated the important point is that we don't introduce regressions.
          Hide
          Ruslan Kabalin added a comment -

          Thanks Dan, got it now, added missing bit to instruction.

          Show
          Ruslan Kabalin added a comment - Thanks Dan, got it now, added missing bit to instruction.
          Hide
          Dan Poltawski added a comment -

          Hi Ruslan,

          inGroup is using camelCase rather than underscores. Its both inconsistent with the other code and against our guidelines.

          Please make it consistent.

          Show
          Dan Poltawski added a comment - Hi Ruslan, inGroup is using camelCase rather than underscores. Its both inconsistent with the other code and against our guidelines. Please make it consistent.
          Hide
          Ruslan Kabalin added a comment -

          Thanks Dan, function names are using snakecase now.

          Show
          Ruslan Kabalin added a comment - Thanks Dan, function names are using snakecase now.
          Hide
          Dan Poltawski added a comment -

          Integrated, thanks Ruslan

          Show
          Dan Poltawski added a comment - Integrated, thanks Ruslan
          Hide
          Adrian Greeve added a comment -

          I tried out the different drag and drop sections. I didn't have any luck with the patch from MDL-33292, but I didn't spot any regressions either.
          Test passed

          Show
          Adrian Greeve added a comment - I tried out the different drag and drop sections. I didn't have any luck with the patch from MDL-33292 , but I didn't spot any regressions either. Test passed
          Hide
          Adrian Greeve added a comment -

          I managed to get patch MDL-33292 working. There were conflicts with the rebase that was stopping it from working.

          Show
          Adrian Greeve added a comment - I managed to get patch MDL-33292 working. There were conflicts with the rebase that was stopping it from working.
          Hide
          Dan Poltawski added a comment -

          Congratulations!

          Your work has made into the latest Moodle release!

          You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

          Show
          Dan Poltawski added a comment - Congratulations! Your work has made into the latest Moodle release! You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: