Moodle
  1. Moodle
  2. MDL-32245

Moving categories to another context using the arrows does not work

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.5, 2.2.2, 2.2.6, 2.3.3, 2.4
    • Fix Version/s: 2.2.7, 2.3.4, 2.4.1
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      1. Select Question bank
      2. Select Categories in Question bank
      3. Make sure you have at least two categories sitting under any top-level context to continue testing
      4. Categories at the top level sitting at the top or bottom of a given context should have either up/down arrow icon that can be clicked.
      5. Before you click it, make sure that at least one question in that category has an image somewhere, for example in the question text.
      6. Clicking the up/down arrow moves the category across to a new context.
      7. Verify that the question image is not broken.
      8. Now try moving that category, or another one, back the other way.
      9. Try some other category editing actions, just to verify that there are no regressions.

      Note:
      A context must have at least one category, so the up/down arrows wont appear in this case.

      Show
      1. Select Question bank 2. Select Categories in Question bank 3. Make sure you have at least two categories sitting under any top-level context to continue testing 4. Categories at the top level sitting at the top or bottom of a given context should have either up/down arrow icon that can be clicked. 5. Before you click it, make sure that at least one question in that category has an image somewhere, for example in the question text. 6. Clicking the up/down arrow moves the category across to a new context. 7. Verify that the question image is not broken. 8. Now try moving that category, or another one, back the other way. 9. Try some other category editing actions, just to verify that there are no regressions. Note: A context must have at least one category, so the up/down arrows wont appear in this case.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      39027

      Description

      1. Go to Question bank -> Categories in the settings block for any Quiz.

      2. If there is only one category in the course context, add a new category with parent Course ... -> Top.

      3. There should now be an 'up' arrow beside the first category under "Question categories for 'Course: ...'"

      4. Click that up arrow. The category should move so that it is now in the "Question categories for 'Quiz: ...'" area.

      At the moment, when you click the up arrow, nothing happens (the page just reloads).

        Issue Links

          Activity

          Hide
          sridhar added a comment -

          I have made some correction in the code for this. See the Images below

          Show
          sridhar added a comment - I have made some correction in the code for this. See the Images below
          Hide
          sridhar added a comment -

          An Extended Issue as the last item in the category doesn't need a down arrow hence fixed.

          Show
          sridhar added a comment - An Extended Issue as the last item in the category doesn't need a down arrow hence fixed.
          Hide
          Tim Hunt added a comment -

          Sridhar,

          1. Please generate diffs with the -u option. They are much easier to read.
          2. Please follow the http://docs.moodle.org/dev/Coding_style
          3. My colleague Mahmoud is actively working on a fix for this issue, so this is not a helpful issue for you to work on.
          4. Your conclusion is wrong, The first up arrow in the 'Course ...' area of the question bank should move the category to the 'Quiz ...' area. If you look at the code, you will see clearly that is what it is supposed to do, it is just that it does not work, and Mahmoud nearly has a patch that makes it work.

          Show
          Tim Hunt added a comment - Sridhar, 1. Please generate diffs with the -u option. They are much easier to read. 2. Please follow the http://docs.moodle.org/dev/Coding_style 3. My colleague Mahmoud is actively working on a fix for this issue, so this is not a helpful issue for you to work on. 4. Your conclusion is wrong, The first up arrow in the 'Course ...' area of the question bank should move the category to the 'Quiz ...' area. If you look at the code, you will see clearly that is what it is supposed to do, it is just that it does not work, and Mahmoud nearly has a patch that makes it work.
          Hide
          Aaron Wells added a comment -

          I've noticed the same thing in Moodle 2.3. I can't seem to switch a question category's context using the arrow buttons. I was able to replicate this in http://demo.moodle.net, logging in as the admin user and going to http://demo.moodle.net/question/category.php?courseid=626

          For the benefit of other users who may encounter this issue, there's a simple workaround. Click the "edit" link next to a question category, and the edit screen has a "Parent category" dropdown menu that will let you change the category's context. (You can't move a question category out of a context if it is the only top-level question category in its context, though. This would leave no question categories in the context for new questions to go in.)

          Show
          Aaron Wells added a comment - I've noticed the same thing in Moodle 2.3. I can't seem to switch a question category's context using the arrow buttons. I was able to replicate this in http://demo.moodle.net , logging in as the admin user and going to http://demo.moodle.net/question/category.php?courseid=626 For the benefit of other users who may encounter this issue, there's a simple workaround. Click the "edit" link next to a question category, and the edit screen has a "Parent category" dropdown menu that will let you change the category's context. (You can't move a question category out of a context if it is the only top-level question category in its context, though. This would leave no question categories in the context for new questions to go in.)
          Hide
          Tim Hunt added a comment -

          Reviewing this code, I spotted various things in the existing code that were not very good, and so decided to make an extra commit to clean things up.

          Thanh, would you like to give a quick review to https://github.com/timhunt/moodle/compare/master...MDL-32245. If it is OK, then I will submit this for integration.

          Show
          Tim Hunt added a comment - Reviewing this code, I spotted various things in the existing code that were not very good, and so decided to make an extra commit to clean things up. Thanh, would you like to give a quick review to https://github.com/timhunt/moodle/compare/master...MDL-32245 . If it is OK, then I will submit this for integration.
          Hide
          Thanh Le added a comment -

          Tim. I'm OK with the changes. I've also tested the revised code and it's working as expected. The interface (move icon and action) looks good on the standard moodle theme.

          Ready to integrate.

          Show
          Thanh Le added a comment - Tim. I'm OK with the changes. I've also tested the revised code and it's working as expected. The interface (move icon and action) looks good on the standard moodle theme. Ready to integrate.
          Hide
          Tim Hunt added a comment -

          Thanks Thanh. Submitting for integration now.

          To INTEGRATORS: please cherry-pick master -> 24_STABLE (noting that there are two commits.)

          Show
          Tim Hunt added a comment - Thanks Thanh. Submitting for integration now. To INTEGRATORS: please cherry-pick master -> 24_STABLE (noting that there are two commits.)
          Hide
          Sam Hemelryk added a comment -

          Thanks everyone, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks everyone, this has been integrated now
          Hide
          Mark Nelson added a comment -

          I received an error 'invalidcategory' when moving a category. This could come under another tracker issue, but I believe it is best to solve it all in one as it is related to moving a category, although it is replicated using an additional step.

          Steps to replicate:

          1. Visit a category, eg. <yoursite>/question/category.php?courseid=<courseid>.
          2. Add a new category to any existing parent category.
          3. Click on the 'Questions' link under 'Question bank' on the site menu under 'Course administration'.
          4. Select one question and move it to the newly created category.
          5. Click on the 'Categories' link under 'Question bank' on the site menu under 'Course administration'.
          6. Attempt to move category to another parent category and you will get the error specified.

          Reason: The 'cat' variable in the URL is still using the existing value when the question was moved.

          Show
          Mark Nelson added a comment - I received an error 'invalidcategory' when moving a category. This could come under another tracker issue, but I believe it is best to solve it all in one as it is related to moving a category, although it is replicated using an additional step. Steps to replicate: Visit a category, eg. <yoursite>/question/category.php?courseid=<courseid>. Add a new category to any existing parent category. Click on the 'Questions' link under 'Question bank' on the site menu under 'Course administration'. Select one question and move it to the newly created category. Click on the 'Categories' link under 'Question bank' on the site menu under 'Course administration'. Attempt to move category to another parent category and you will get the error specified. Reason: The 'cat' variable in the URL is still using the existing value when the question was moved.
          Hide
          Tim Hunt added a comment -

          Ah! I see. That is tricky! Thanks for the very clear steps to reproduce. I will do another commit to fix that now.

          Show
          Tim Hunt added a comment - Ah! I see. That is tricky! Thanks for the very clear steps to reproduce. I will do another commit to fix that now.
          Hide
          Tim Hunt added a comment -

          New commit 46b3287bbd9dfcee278e236bfc504881eb61bf0e on the MDL-32245 branch. Please can you cherry-pick that to all branches.

          Show
          Tim Hunt added a comment - New commit 46b3287bbd9dfcee278e236bfc504881eb61bf0e on the MDL-32245 branch. Please can you cherry-pick that to all branches.
          Hide
          Sam Hemelryk added a comment -

          Thanks Tim, I've cherry-picked the fix.. All yours once more Mark

          Show
          Sam Hemelryk added a comment - Thanks Tim, I've cherry-picked the fix.. All yours once more Mark
          Hide
          Mark Nelson added a comment -

          Hi Tim, that fixes the issue with the invalid category error displaying but there is still a trivial issue. When moving the newly created category down to a new parent, it adds it to the bottom of the list below, rather than the top, which is the most intuitive place to put it. However, if you move it back up to the initial category where it was placed when first created, then down again it appears at the top of the list.

          Show
          Mark Nelson added a comment - Hi Tim, that fixes the issue with the invalid category error displaying but there is still a trivial issue. When moving the newly created category down to a new parent, it adds it to the bottom of the list below, rather than the top, which is the most intuitive place to put it. However, if you move it back up to the initial category where it was placed when first created, then down again it appears at the top of the list.
          Hide
          Tim Hunt added a comment -

          True, it is still not perfect. However, this week we have gone from something that did not work at all, to something that works with one small glitch, and we need to get the weekly out, so I have created MDL-37249 for the remaining problem. I suggest we pass this now.

          Show
          Tim Hunt added a comment - True, it is still not perfect. However, this week we have gone from something that did not work at all, to something that works with one small glitch, and we need to get the weekly out, so I have created MDL-37249 for the remaining problem. I suggest we pass this now.
          Hide
          Mark Nelson added a comment -

          Hi Tim, sounds fine to me. Up to an integrator to pass this for testing as I am unable to after I have failed it.

          Show
          Mark Nelson added a comment - Hi Tim, sounds fine to me. Up to an integrator to pass this for testing as I am unable to after I have failed it.
          Hide
          Mark Nelson added a comment -

          That was quick. Passing as agreed in comments above.

          Show
          Mark Nelson added a comment - That was quick. Passing as agreed in comments above.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Changes are now upstream, thanks for your collaboration!

          If you are going to have any celebration next days, enjoy with your gang, if not, too!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Changes are now upstream, thanks for your collaboration! If you are going to have any celebration next days, enjoy with your gang, if not, too! Ciao

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: