Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-39480

Notices and warnings while trying to access site wide badge criteria list as teacher

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5
    • Component/s: Badges
    • Labels:
    • Testing Instructions:
      Hide
      1. Create a side wide badge as admin. Site administration > badges.
      2. Once the badge is completely set up follow this document http://docs.moodle.org/25/en/Badges_FAQ to allow teachers to access site badges.
      3. Log in as a teacher and go to Site administration / ► Badges / ► Manage badges
      4. Click on the name of the site badge and check that no errors are displayed.
      Show
      Create a side wide badge as admin. Site administration > badges. Once the badge is completely set up follow this document http://docs.moodle.org/25/en/Badges_FAQ to allow teachers to access site badges. Log in as a teacher and go to Site administration / ► Badges / ► Manage badges Click on the name of the site badge and check that no errors are displayed.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-39480_badge

      Description

      1. Create a side wide badge as admin. Site administration > badges.
      2. Once the badge is completely set up and available copy the URL.
      3. Log in as a teacher and go to the saved URL.
        Errors as displayed.

        Gliffy Diagrams

        1. bonfire-screenshot-20130503-113014-516.png
          114 kB
        2. sitebadges.png
          43 kB
        3. sitebadgesinnavigation.png
          63 kB
        4. t.png
          431 kB

          Issue Links

            Activity

            Hide
            marycooch Mary Cooch added a comment -

            Hi Ankit. This is because (as I learned from Yuliya ) teachers can't award site badges, only course badges. See http://docs.moodle.org/25/en/Badges_FAQ so I don't think this is a bug.

            Show
            marycooch Mary Cooch added a comment - Hi Ankit. This is because (as I learned from Yuliya ) teachers can't award site badges, only course badges. See http://docs.moodle.org/25/en/Badges_FAQ so I don't think this is a bug.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            In that case the page should not be shown to teachers at all. Should throw a no permission error.

            Show
            ankit_frenz Ankit Agarwal added a comment - In that case the page should not be shown to teachers at all. Should throw a no permission error.
            Hide
            marycooch Mary Cooch added a comment -

            When someone has sitewide ability to manage site badges they can access the criteria from Administration>Badges>Manage badges (see screenshot)

            Show
            marycooch Mary Cooch added a comment - When someone has sitewide ability to manage site badges they can access the criteria from Administration>Badges>Manage badges (see screenshot)
            Hide
            marycooch Mary Cooch added a comment -

            Site badges in Navigation block screenshot added

            Show
            marycooch Mary Cooch added a comment - Site badges in Navigation block screenshot added
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            This is what I did this time:-

            1. Added a site badge
            2. Edited teacher role to be in system context
            3. Gave a user that role
            4. Logged in as that user, I cannot see the site admin link. But as Mary pointed out its working for her, so can be just my setup.
              anyway moving my attention to MDLQA-5714 for the time being
            Show
            ankit_frenz Ankit Agarwal added a comment - This is what I did this time:- Added a site badge Edited teacher role to be in system context Gave a user that role Logged in as that user, I cannot see the site admin link. But as Mary pointed out its working for her, so can be just my setup. anyway moving my attention to MDLQA-5714 for the time being
            Hide
            andyjdavis Andrew Davis added a comment -

            print_badge_status_box(badge $badge) is quite clearly incorrect. Here is a fix that prevents the error being displayed.

            https://github.com/andyjdavis/moodle/compare/master...MDL-39480_badge

            For the teacher to get to this page (/badges/criteria.php?id=1&awards=2) I navigated there as admin, copied and pasted the URL, logged in as teacher then pasted the URL into the browser address bar. I'll fill out this issue using the testing steps I used. If anyone has a less hacky version using the capability system feel free to update the testing instructions.

            Show
            andyjdavis Andrew Davis added a comment - print_badge_status_box(badge $badge) is quite clearly incorrect. Here is a fix that prevents the error being displayed. https://github.com/andyjdavis/moodle/compare/master...MDL-39480_badge For the teacher to get to this page (/badges/criteria.php?id=1&awards=2) I navigated there as admin, copied and pasted the URL, logged in as teacher then pasted the URL into the browser address bar. I'll fill out this issue using the testing steps I used. If anyone has a less hacky version using the capability system feel free to update the testing instructions.
            Hide
            andyjdavis Andrew Davis added a comment -

            Oops. I just managed to accidently overwrite the original issue description with the testing instructions. Sorry

            Show
            andyjdavis Andrew Davis added a comment - Oops. I just managed to accidently overwrite the original issue description with the testing instructions. Sorry
            Hide
            andyjdavis Andrew Davis added a comment -

            I've updated the testing instructions. Its worth noting that once the user has the required capabilities for the link to the site badge page to appear in site administration they would never have got the error. I'm not sure how it arose in the first place. The error is however now fixed (assuming the capability checks on that page are correct).

            Show
            andyjdavis Andrew Davis added a comment - I've updated the testing instructions. Its worth noting that once the user has the required capabilities for the link to the site badge page to appear in site administration they would never have got the error. I'm not sure how it arose in the first place. The error is however now fixed (assuming the capability checks on that page are correct).
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Andrew,
            The patch looks good. However on further investigation, I noticed the page is open to everyone. All it does is a require_login() check. Although it doesn’t leak any information that the user cannot already see as far as I can tell. So I suggest either we link this page up with view.php or block it completely if the user doesn’t have permissions to modify the criteria for the badge.

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Andrew, The patch looks good. However on further investigation, I noticed the page is open to everyone. All it does is a require_login() check. Although it doesn’t leak any information that the user cannot already see as far as I can tell. So I suggest either we link this page up with view.php or block it completely if the user doesn’t have permissions to modify the criteria for the badge.
            Hide
            andyjdavis Andrew Davis added a comment -

            Oh wait, there is actually a capability check in index.php. Its just quite permissive (but not necessarily wrong).

            if (!has_any_capability(array(
                    'moodle/badges:viewawarded',
                    'moodle/badges:createbadge',
                    'moodle/badges:awardbadge',
                    'moodle/badges:configuremessages',
                    'moodle/badges:configuredetails',
                    'moodle/badges:deletebadge'), $PAGE->context)) {
                redirect($CFG->wwwroot);
            }

            Show
            andyjdavis Andrew Davis added a comment - Oh wait, there is actually a capability check in index.php. Its just quite permissive (but not necessarily wrong). if (!has_any_capability(array( 'moodle/badges:viewawarded', 'moodle/badges:createbadge', 'moodle/badges:awardbadge', 'moodle/badges:configuremessages', 'moodle/badges:configuredetails', 'moodle/badges:deletebadge'), $PAGE->context)) { redirect($CFG->wwwroot); }
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Yes Andrew, but there is none in criteria.php

            Show
            ankit_frenz Ankit Agarwal added a comment - Yes Andrew, but there is none in criteria.php
            Hide
            andyjdavis Andrew Davis added a comment -

            After much discussion I'm putting this up for peer review. There may well be issues still needing to be fixed but this error at least is resolved.

            Show
            andyjdavis Andrew Davis added a comment - After much discussion I'm putting this up for peer review. There may well be issues still needing to be fixed but this error at least is resolved.
            Hide
            damyon Damyon Wiese added a comment -

            This change looks good - integrated to master.

            One minor point is that it's not ideal to do capability checks inside the renderers (that is logic). It is better to do the checks before hand and pass that information to the renderer as a boolean.

            Show
            damyon Damyon Wiese added a comment - This change looks good - integrated to master. One minor point is that it's not ideal to do capability checks inside the renderers (that is logic). It is better to do the checks before hand and pass that information to the renderer as a boolean.
            Hide
            abgreeve Adrian Greeve added a comment -

            I followed the instructions and found no notices or warnings.
            Test passed.

            Show
            abgreeve Adrian Greeve added a comment - I followed the instructions and found no notices or warnings. Test passed.
            Hide
            abgreeve Adrian Greeve added a comment - - edited

            Just out of personal interest. I followed the instructions in the faq for "How can teachers award site badges?" with the thought that creating a badge with the badge criteria of "This badge has to be awarded by a user with the following role: - Teacher" would allow the teacher to award the badge to a student, but when navigating to [Site administration > Badges > Manage badges] and then click on the icon for awarding a badge I get the following message: "Your current role assignment is not among the roles that can manually issue this badge."

            So how do you actually award a site wide badge as a teacher in this situation?

            Edit: Thanks to Ankit for going through this with me and explaining what to do to get it to work. It was an issue with permissions and I didn't set up the badge to be awarded by a "site badge awarder" Once that was changed I could award badges site wide.

            Show
            abgreeve Adrian Greeve added a comment - - edited Just out of personal interest. I followed the instructions in the faq for "How can teachers award site badges?" with the thought that creating a badge with the badge criteria of "This badge has to be awarded by a user with the following role: - Teacher" would allow the teacher to award the badge to a student, but when navigating to [Site administration > Badges > Manage badges] and then click on the icon for awarding a badge I get the following message: "Your current role assignment is not among the roles that can manually issue this badge." So how do you actually award a site wide badge as a teacher in this situation? Edit: Thanks to Ankit for going through this with me and explaining what to do to get it to work. It was an issue with permissions and I didn't set up the badge to be awarded by a "site badge awarder" Once that was changed I could award badges site wide.
            Hide
            andyjdavis Andrew Davis added a comment -

            That's a good question to which I personally do not have a good answer.

            Show
            andyjdavis Andrew Davis added a comment - That's a good question to which I personally do not have a good answer.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Did you think this day was not going to arrive ever?

            Your patience has been rewarded, yay, sent upstream, thanks!

            Closing...ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Did you think this day was not going to arrive ever? Your patience has been rewarded, yay, sent upstream, thanks! Closing...ciao

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/May/13