Moodle
  1. Moodle
  2. MDL-26199

Better navigation when moving from the question bank to import/export

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.1
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      1. Go to either course settings -> question bank or quiz settings -> question bank
      2. Select a non-default category in the question bank.
      3. Navigate around between questions, categories, import and export, and verify that it remembers the category you are looking at.

      Show
      1. Go to either course settings -> question bank or quiz settings -> question bank 2. Select a non-default category in the question bank. 3. Navigate around between questions, categories, import and export, and verify that it remembers the category you are looking at.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      15867

      Description

      2.0 better navigation when importing/exporting category.
      Moodle 2.0.

      You are in the question bank and have selected a category (or sub-category). You then want to export the questions from the selected category OR import questions into the selected category. You click on the export or import link in the Settings/Question bank navigation block. Contrary to your expectations, in the export or import page, the Export or Import dropdown list does not show the category that you had previously selected, it always shows the very first category in the list of categories (or sub-categories).

      Can we please have the previously selected category be made 'sticky' and show as selected in the drop-down list?

      Joseph

        Issue Links

          Activity

          Hide
          Joseph Rézeau added a comment -

          Will test this ASAP. Thanks, Tim.

          Show
          Joseph Rézeau added a comment - Will test this ASAP. Thanks, Tim.
          Hide
          Tim Hunt added a comment -

          Me == idiot. that last fix was a different bug. Sorry about the noise.

          Show
          Tim Hunt added a comment - Me == idiot. that last fix was a different bug. Sorry about the noise.
          Hide
          Tim Hunt added a comment -

          Sam, which approach to doing this do you prefer?
          https://github.com/timhunt/moodle/compare/master...MDL-26199b
          or
          https://github.com/timhunt/moodle/commit/4a120040d5e7005986051a6ec0797f5c53d42e09
          or something else?

          Also, note that there is a second commit that is needed (on either branch) to finish the job: See https://github.com/timhunt/moodle/compare/master...MDL-26199a. (You might guess I vote a )

          Even with this second commit, I get a weird problem. Suppose you are on the export page. The Question bank section of the navigation will be collapsed, as if the navigation does not know what the current node is. however, when you expand the Question bank bit of the tree, you will see that Export is in bold. Also the 'breadcrumbs' are correct. I think that is a navigation bug, and therefore your problem. Please could you confirm. Thanks.

          Show
          Tim Hunt added a comment - Sam, which approach to doing this do you prefer? https://github.com/timhunt/moodle/compare/master...MDL-26199b or https://github.com/timhunt/moodle/commit/4a120040d5e7005986051a6ec0797f5c53d42e09 or something else? Also, note that there is a second commit that is needed (on either branch) to finish the job: See https://github.com/timhunt/moodle/compare/master...MDL-26199a . (You might guess I vote a ) Even with this second commit, I get a weird problem. Suppose you are on the export page. The Question bank section of the navigation will be collapsed, as if the navigation does not know what the current node is. however, when you expand the Question bank bit of the tree, you will see that Export is in bold. Also the 'breadcrumbs' are correct. I think that is a navigation bug, and therefore your problem. Please could you confirm. Thanks.
          Hide
          Tim Hunt added a comment -

          P.S. Once you have confirmed the correct approach, I will back-part this to 2.0.

          Show
          Tim Hunt added a comment - P.S. Once you have confirmed the correct approach, I will back-part this to 2.0.
          Hide
          Sam Hemelryk added a comment -

          Hi Tim,

          Interesting task

          Typically if a URL parameter is required in the navigation code in order to correctly generate things I've deemed it that the navigation is wrong, the idea being that navigation behaviour should never change so that users know that navigation item will always lead them to the same place.
          There's been several instances where people have hit this problem and up until now that has always stuck true - HOWEVER all of those instances have been in regards to navigation and not settings.

          I've been looking over the solutions you have presently, I can see the need for a solution, but I'm not sure they are the best solutions to the problem at hand.
          A quick grep shows that the question_extend_settings_navigation is used when generating course settings and when generating quiz settings. As course settings are generated for all contexts with the course context as a parent the following things need to be taken into regard:

          • MDL-26199b there is a potential bug here should any other component within the course use either param cat or category with two numbers separated by a comma.
          • 4a120040 same as above but only if the param has been added to the pages URL and only for cat.
            Certainly of the two I prefer 4a120040, this would be first time I prefer a PAGE->url param but given that its potentially coming from two vars I think it is the better solution.

          There is however one other option to this problem that I see, what if at the appropriate places (pretty much the files in MDL-26199a) when working with question categories we extend the settings navigation to add options to import/export this category?
          You could even add a branch for the category being worked with and add the options to that branch.
          This would have the advantage that you would not be changing the behaviour of the import/export options depending on what you are doing, instead you would have specific options as well. As a bonus it may also help improve the navbar for the question bank.
          The disadvantage to this is that we would need to write a function to extend the settings and call it on required pages.

          As for my preference - I'd rather keep URL params out of the equation so I'd personally lean towards exploring how to extend the settings at the appropriate times.
          However if you choose to go with the URL params I'd certainly prefer 4a120040 if only because it would appear to reduce the chance of collision with params used in other places of Moodle.

          I'm very keen to hear your thoughts on this.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Tim, Interesting task Typically if a URL parameter is required in the navigation code in order to correctly generate things I've deemed it that the navigation is wrong, the idea being that navigation behaviour should never change so that users know that navigation item will always lead them to the same place. There's been several instances where people have hit this problem and up until now that has always stuck true - HOWEVER all of those instances have been in regards to navigation and not settings. I've been looking over the solutions you have presently, I can see the need for a solution, but I'm not sure they are the best solutions to the problem at hand. A quick grep shows that the question_extend_settings_navigation is used when generating course settings and when generating quiz settings. As course settings are generated for all contexts with the course context as a parent the following things need to be taken into regard: MDL-26199 b there is a potential bug here should any other component within the course use either param cat or category with two numbers separated by a comma. 4a120040 same as above but only if the param has been added to the pages URL and only for cat. Certainly of the two I prefer 4a120040, this would be first time I prefer a PAGE->url param but given that its potentially coming from two vars I think it is the better solution. There is however one other option to this problem that I see, what if at the appropriate places (pretty much the files in MDL-26199 a) when working with question categories we extend the settings navigation to add options to import/export this category? You could even add a branch for the category being worked with and add the options to that branch. This would have the advantage that you would not be changing the behaviour of the import/export options depending on what you are doing, instead you would have specific options as well. As a bonus it may also help improve the navbar for the question bank. The disadvantage to this is that we would need to write a function to extend the settings and call it on required pages. As for my preference - I'd rather keep URL params out of the equation so I'd personally lean towards exploring how to extend the settings at the appropriate times. However if you choose to go with the URL params I'd certainly prefer 4a120040 if only because it would appear to reduce the chance of collision with params used in other places of Moodle. I'm very keen to hear your thoughts on this. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          I'm clicking looks great to me because I've got to click something.
          Really I'd be keen to hear your feedback - I am happy with your solution so far though I just think there is more to consider/discuss

          Show
          Sam Hemelryk added a comment - I'm clicking looks great to me because I've got to click something. Really I'd be keen to hear your feedback - I am happy with your solution so far though I just think there is more to consider/discuss
          Hide
          Tim Hunt added a comment -

          I don't think that adding &cat=1,2 to the URL really changes where that link in the navigation goes. I think it merely preserves some state when you got from place A to place B using the navigation, in some cases. And that is definitely a big usability win.

          Now, you might want to argue that it is not the job of the navigation to preserve state, and I can see the logic of that. I suppose that logical place to store state is in the session (or, in some cases, in a user_preference, but I think that is inappropriate here). In about Moodle 1.6 the question bank UI used to use the session to track 'current category', and that was a total disaster. It only tracked a single current category, and so it was impossible to work with two different browser tabs, for example to copy some information about questions in one course into another.

          But, even if we did something more sophisticated, for example using the session to store a current category for each course/quiz you have looked at the question bank in, you might have a single course with a large question bank, and want to have to different sub-categories open in different browser tabs to do stuff.

          In fact, you have probably encountered the irritations of using session to track this sort of state yourself, here. Jira uses the session to track all the information about that 'current search'. Have you ever wanted to have two different bug lists open in different tabs? Annoying isn't it.

          So, I really think that allowing some state to be preserved through links in the navigation is the way to build the nicest user experience, and that is what I am most interested in optimising.

          By the way, did you have a look into my 'weird problem'?

          Show
          Tim Hunt added a comment - I don't think that adding &cat=1,2 to the URL really changes where that link in the navigation goes. I think it merely preserves some state when you got from place A to place B using the navigation, in some cases. And that is definitely a big usability win. Now, you might want to argue that it is not the job of the navigation to preserve state, and I can see the logic of that. I suppose that logical place to store state is in the session (or, in some cases, in a user_preference, but I think that is inappropriate here). In about Moodle 1.6 the question bank UI used to use the session to track 'current category', and that was a total disaster. It only tracked a single current category, and so it was impossible to work with two different browser tabs, for example to copy some information about questions in one course into another. But, even if we did something more sophisticated, for example using the session to store a current category for each course/quiz you have looked at the question bank in, you might have a single course with a large question bank, and want to have to different sub-categories open in different browser tabs to do stuff. In fact, you have probably encountered the irritations of using session to track this sort of state yourself, here. Jira uses the session to track all the information about that 'current search'. Have you ever wanted to have two different bug lists open in different tabs? Annoying isn't it. So, I really think that allowing some state to be preserved through links in the navigation is the way to build the nicest user experience, and that is what I am most interested in optimising. By the way, did you have a look into my 'weird problem'?
          Hide
          Sam Hemelryk added a comment -

          Hi Tim,

          Thanks for the feedback.
          Indeed I like to think that the it isn't the job of the navigation to preserve state, but I do see the advantage of it doing so, especially in situations like this.

          I suppose ideally I would like to see the calling code inform the navigation of the state rather than having the navigation guess the state by looking to the page URL or optional params.
          On top of that is that idea that the each state should be represented on the navigation. Rather than changing the link to maintain the state, add additional content that offers the option of maintaining state so that the user can decide.
          But then I am not a average quiz administrator - perhaps the reality is that if you are browsing a category you would only ever want to export that category in which changing the having only the dynamic state is fine.
          What ever you decide there I am happy with, but what do you think of having the calling code set the state rather than looking to the URL/params?

          As for the weird problem I've just been having a look now and you're quite right!
          It seems to be affecting all active branches - the branch itself is marked active but it is isn't reflecting that upon its parents.
          I'll create a blocker to fix that now.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Tim, Thanks for the feedback. Indeed I like to think that the it isn't the job of the navigation to preserve state, but I do see the advantage of it doing so, especially in situations like this. I suppose ideally I would like to see the calling code inform the navigation of the state rather than having the navigation guess the state by looking to the page URL or optional params. On top of that is that idea that the each state should be represented on the navigation. Rather than changing the link to maintain the state, add additional content that offers the option of maintaining state so that the user can decide. But then I am not a average quiz administrator - perhaps the reality is that if you are browsing a category you would only ever want to export that category in which changing the having only the dynamic state is fine. What ever you decide there I am happy with, but what do you think of having the calling code set the state rather than looking to the URL/params? As for the weird problem I've just been having a look now and you're quite right! It seems to be affecting all active branches - the branch itself is marked active but it is isn't reflecting that upon its parents. I'll create a blocker to fix that now. Cheers Sam
          Hide
          Tim Hunt added a comment -

          I think my point is that for different types of state, there are different levels of persistence and scope required:

          1. For some things, a user_preference is appropriate. That choice is remembered forever, and immediately starts applying everywhere else in Moodle, once set in one place. (Indeed, if you are logged in from two or more separate browsers at the same time, it immediately affects all of them.)

          2. For other things, session is appropriate. That is less permanent, but still applies in all open browser windows and tabs (in this browser/device) from the moment you make the change.)

          3. Sometimes you need something more specific, that only affects this browser tab. The only way I know to do this is URL parameters.

          "each state should be represented on the navigation" is not a sensible idea, and we don't do that anywhere else. For example, the Participants list: the state there is the: currently selected group; the state of the initials bar; and the sort order of the columns. None of that is in the navigation, quite rightly. All those are handled by session, if I recall. I bet it is a pain if you are trying to compare membership of two groups. Yes, here is some irritating behaviour:

          1. Open http://moodle.org/user/index.php?contextid=53&roleid=0&id=5&perpage=20&accesssince=0&search=&group=172 in one browser tab.
          2. Open http://moodle.org/user/index.php?contextid=53&roleid=0&id=5&perpage=20&accesssince=0&search=&group=179 in a second browser tab.
          3. Go back to the first browser tab, and click on 2. in the paging bar.
          4. Note that this tab has switched to the other group.

          Obviously the navigation is not involved here, but this is a fundamental about about how hyperlinks are supposed to work. The navigation is simply a collection of links, that should work like any other links should work.

          Show
          Tim Hunt added a comment - I think my point is that for different types of state, there are different levels of persistence and scope required: 1. For some things, a user_preference is appropriate. That choice is remembered forever, and immediately starts applying everywhere else in Moodle, once set in one place. (Indeed, if you are logged in from two or more separate browsers at the same time, it immediately affects all of them.) 2. For other things, session is appropriate. That is less permanent, but still applies in all open browser windows and tabs (in this browser/device) from the moment you make the change.) 3. Sometimes you need something more specific, that only affects this browser tab. The only way I know to do this is URL parameters. "each state should be represented on the navigation" is not a sensible idea, and we don't do that anywhere else. For example, the Participants list: the state there is the: currently selected group; the state of the initials bar; and the sort order of the columns. None of that is in the navigation, quite rightly. All those are handled by session, if I recall. I bet it is a pain if you are trying to compare membership of two groups. Yes, here is some irritating behaviour: 1. Open http://moodle.org/user/index.php?contextid=53&roleid=0&id=5&perpage=20&accesssince=0&search=&group=172 in one browser tab. 2. Open http://moodle.org/user/index.php?contextid=53&roleid=0&id=5&perpage=20&accesssince=0&search=&group=179 in a second browser tab. 3. Go back to the first browser tab, and click on 2. in the paging bar. 4. Note that this tab has switched to the other group. Obviously the navigation is not involved here, but this is a fundamental about about how hyperlinks are supposed to work. The navigation is simply a collection of links, that should work like any other links should work.
          Hide
          Tim Hunt added a comment -

          After further discussion with Sam, am pushing the MDL-26199a branch for integration (after re-basing).

          I have also created a new MDL-28000 for further work later.

          Show
          Tim Hunt added a comment - After further discussion with Sam, am pushing the MDL-26199 a branch for integration (after re-basing). I have also created a new MDL-28000 for further work later.
          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.

          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.
          Hide
          Sam Hemelryk added a comment -

          Thanks Tim, this has been integrated now.
          Nice bug number for the new issue btw

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Tim, this has been integrated now. Nice bug number for the new issue btw Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Testing this as I was am clear on what it was doing and there are no testing instructions

          Show
          Sam Hemelryk added a comment - Testing this as I was am clear on what it was doing and there are no testing instructions
          Hide
          Sam Hemelryk added a comment -

          Thanks Tim - this passed
          btw I spotted the testing instructions ... they were there all along!

          Show
          Sam Hemelryk added a comment - Thanks Tim - this passed btw I spotted the testing instructions ... they were there all along!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Moodleland is now a better place to live, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Moodleland is now a better place to live, thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: