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:

      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).

        Gliffy Diagrams

          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: