Moodle
  1. Moodle
  2. MDL-25999

Cannot bookmark page using Admin Bookmarks block

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: None
    • Component/s: Administration, Blocks
    • Labels:
    • Environment:
      any
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_20_STABLE
    • Rank:
      15506

      Description

      Add Admin Bookmarks block.
      Navigate to any site configuration page.
      No visible option to bookmark that page.

      Tried with Site Admin and Manager accounts.

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          Confirming, this feature does not work at all because there is no way to identify admin pages using the PAGE object.

          Show
          Petr Škoda added a comment - Confirming, this feature does not work at all because there is no way to identify admin pages using the PAGE object.
          Hide
          Frank Ralf added a comment -

          JFTR

          See also http://moodle.org/mod/forum/discuss.php?d=161579 ("Admin bookmarks" block doesn't work)

          Show
          Frank Ralf added a comment - JFTR See also http://moodle.org/mod/forum/discuss.php?d=161579 ("Admin bookmarks" block doesn't work)
          Hide
          Helen Foster added a comment -

          Increasing priority because this feature doesn't work at all.

          Show
          Helen Foster added a comment - Increasing priority because this feature doesn't work at all.
          Hide
          Ray Lawrence added a comment -

          Thanks. Surprised there are so few votes for this. I'm missing it badly, esp. with Site adminstration requiring a scroll each time I need it.

          Show
          Ray Lawrence added a comment - Thanks. Surprised there are so few votes for this. I'm missing it badly, esp. with Site adminstration requiring a scroll each time I need it.
          Hide
          Frank Ralf added a comment - - edited

          IMHO that's because QA testing is a bit biased towards teachers, students and the like. There isn't even an administrator account at http://qa.moodle.net (understandably).

          EDIT:
          Just remembered having received admin login data from Helen by PM some time ago ...

          Show
          Frank Ralf added a comment - - edited IMHO that's because QA testing is a bit biased towards teachers, students and the like. There isn't even an administrator account at http://qa.moodle.net (understandably). EDIT: Just remembered having received admin login data from Helen by PM some time ago ...
          Hide
          Rossiani Wijaya added a comment -

          Adding Apu and Sam as wather.

          Update Admin bookmark for 2.0.
          Apu or Sam, when you have a chance, could you take a look the patch?

          diff url: https://github.com/rwijaya/moodle/compare/MOODLE_20_STABLE...MDL-25999_m20

          Thanks

          Show
          Rossiani Wijaya added a comment - Adding Apu and Sam as wather. Update Admin bookmark for 2.0. Apu or Sam, when you have a chance, could you take a look the patch? diff url: https://github.com/rwijaya/moodle/compare/MOODLE_20_STABLE...MDL-25999_m20 Thanks
          Hide
          Sam Hemelryk added a comment -

          Hi Rosie,

          I've had a look at the patch and picked up the following:

          blocks/admin_bookmarks/block_admin_bookmarks.php
          line: 34 - $navcount not needed, that's a navigation block static used to count the number of nav trees on the page
          line: 49 - Is the global CFG needed?
          line: 82 - copy and paste Guessing that should be true as well.
          line: 92 - PHP docs required
          line: 96 - I don't think the block should override that at all, it should just use the default function
          line: 117 - Again not needed
          line: 119~135 - That code shouldn't be in the navigation block, let alone this block, I'll create an issue to clean it up from the navigation block.

          In general it would pay to search the code for the word navigation and then look at fixing the comment/docs/uses if need be

          The only other thing I noticed is that both the navigation and admin tree are being used to locate the things, the admin tree is being used to locate bookmarks, and the navigation is being used to locate the page the user is on.
          Having looked at the code I don't think one is better than the other for this task, but I do wonder whether we should use one and not both.
          Either way perhaps not worth worrying about right now, certainly not as part of this issue.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Rosie, I've had a look at the patch and picked up the following: blocks/admin_bookmarks/block_admin_bookmarks.php line: 34 - $navcount not needed, that's a navigation block static used to count the number of nav trees on the page line: 49 - Is the global CFG needed? line: 82 - copy and paste Guessing that should be true as well. line: 92 - PHP docs required line: 96 - I don't think the block should override that at all, it should just use the default function line: 117 - Again not needed line: 119~135 - That code shouldn't be in the navigation block, let alone this block, I'll create an issue to clean it up from the navigation block. In general it would pay to search the code for the word navigation and then look at fixing the comment/docs/uses if need be The only other thing I noticed is that both the navigation and admin tree are being used to locate the things, the admin tree is being used to locate bookmarks, and the navigation is being used to locate the page the user is on. Having looked at the code I don't think one is better than the other for this task, but I do wonder whether we should use one and not both. Either way perhaps not worth worrying about right now, certainly not as part of this issue. Cheers Sam
          Hide
          Rossiani Wijaya added a comment -

          Thanks Sam for reviewing.

          I updated the patch and reposted at: https://github.com/rwijaya/moodle/compare/MOODLE_20_STABLE...MDL-25999_m20

          I will take a look the locate function over the weekend.

          Apart from the issue, I'm just wondering what is the term we are using to address side blocks? (I refer them as navigation )

          Show
          Rossiani Wijaya added a comment - Thanks Sam for reviewing. I updated the patch and reposted at: https://github.com/rwijaya/moodle/compare/MOODLE_20_STABLE...MDL-25999_m20 I will take a look the locate function over the weekend. Apart from the issue, I'm just wondering what is the term we are using to address side blocks? (I refer them as navigation )
          Hide
          Rossiani Wijaya added a comment -

          submitted to pull request.
          PULL-692 && PULL-693

          Show
          Rossiani Wijaya added a comment - submitted to pull request. PULL-692 && PULL-693
          Hide
          Stuart R Mealor added a comment -

          Has this actually been resolved, or just for earlier versions of Moodle ?
          We are running Moodle v2.4.1 on a site (updated from 1.9 through 2.x etc. to 2.4.1) and still having this same issue of no ability to add a new bookmark.
          Is a fix to simply delete the table and reinstall ?
          If it's still an issue can this tracker item be re-opened (maybe it only affects updated site and not new sites?)
          Thanks, Stuart.

          Show
          Stuart R Mealor added a comment - Has this actually been resolved, or just for earlier versions of Moodle ? We are running Moodle v2.4.1 on a site (updated from 1.9 through 2.x etc. to 2.4.1) and still having this same issue of no ability to add a new bookmark. Is a fix to simply delete the table and reinstall ? If it's still an issue can this tracker item be re-opened (maybe it only affects updated site and not new sites?) Thanks, Stuart.
          Hide
          Stuart R Mealor added a comment -

          Actually, after further investigation, it seems some pages can be Bookmarked, and others can't.
          In testing I can bookmark (for example) the Notifications page.
          But I can't bookmark a course page, or activity within.
          So I guess the question is whether this functionality could be extended to allow ANY page on a site to bookmarked? Feature request?

          Show
          Stuart R Mealor added a comment - Actually, after further investigation, it seems some pages can be Bookmarked, and others can't. In testing I can bookmark (for example) the Notifications page. But I can't bookmark a course page, or activity within. So I guess the question is whether this functionality could be extended to allow ANY page on a site to bookmarked? Feature request?
          Hide
          Aparup Banerjee added a comment -

          Hi Stuart,
          i haven't looked at this but i'd certainly go with a whole new MDL for sure.

          Show
          Aparup Banerjee added a comment - Hi Stuart, i haven't looked at this but i'd certainly go with a whole new MDL for sure.
          Hide
          Ray Lawrence added a comment -

          This is for admin pages only not course, activity, category pages. Is that what you meant?

          A facility for a course bookmarks block would be handy. Feel free t add me a s a watcher if you propose that.

          Show
          Ray Lawrence added a comment - This is for admin pages only not course, activity, category pages. Is that what you meant? A facility for a course bookmarks block would be handy. Feel free t add me a s a watcher if you propose that.
          Hide
          Stuart R Mealor added a comment -

          Thanks Aparup, yes, I agree it would be a new feature request.
          Ray, yes, it's only currently useful for Admin pages, but after working on a couple of clients sites recently I thought it might be useful to extend to 'any page' that helps a Moodle Admin.
          I'll think about it a little longer, and formulate a feature request after I've worked through some ideas.

          Show
          Stuart R Mealor added a comment - Thanks Aparup, yes, I agree it would be a new feature request. Ray, yes, it's only currently useful for Admin pages, but after working on a couple of clients sites recently I thought it might be useful to extend to 'any page' that helps a Moodle Admin. I'll think about it a little longer, and formulate a feature request after I've worked through some ideas.

            People

            • Votes:
              6 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: