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

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Petr Skoda 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 Skoda 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: