Moodle
  1. Moodle
  2. MDL-39480

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker 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
    • Rank:
      50147

      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.
      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
          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
          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 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 Agarwal added a comment - In that case the page should not be shown to teachers at all. Should throw a no permission error.
          Hide
          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
          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
          Mary Cooch added a comment -

          Site badges in Navigation block screenshot added

          Show
          Mary Cooch added a comment - Site badges in Navigation block screenshot added
          Hide
          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 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
          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
          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
          Andrew Davis added a comment -

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

          Show
          Andrew Davis added a comment - Oops. I just managed to accidently overwrite the original issue description with the testing instructions. Sorry
          Hide
          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
          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 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 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
          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
          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 Agarwal added a comment -

          Yes Andrew, but there is none in criteria.php

          Show
          Ankit Agarwal added a comment - Yes Andrew, but there is none in criteria.php
          Hide
          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
          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 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 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
          Adrian Greeve added a comment -

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

          Show
          Adrian Greeve added a comment - I followed the instructions and found no notices or warnings. Test passed.
          Hide
          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
          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
          Andrew Davis added a comment -

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

          Show
          Andrew Davis added a comment - That's a good question to which I personally do not have a good answer.
          Hide
          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
          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: