Moodle
  1. Moodle
  2. MDL-39986

Allow non editing teachers and students with award badge privilege award badges from settings block

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1
    • Component/s: Badges
    • Labels:
    • Testing Instructions:
      Hide

      1. Add a non-editing teacher to the course (this role can award and view recipients by default).
      2. Login as this non-editing teacher.
      3. Go to the course.
      4. Badges navigation should appear under course administration.
      5. If there are any badges, only one action should be available in manage badge table - award a badge.
      5a. If there are no badges, add new badge button should not appear. User should see just a message that there are no badges.
      6. Try to award a badge.
      7a. If the role is not among criteria, you should get a message that it is not possible.
      7b. If the role among criteria, you should be able to award badge.

      Show
      1. Add a non-editing teacher to the course (this role can award and view recipients by default). 2. Login as this non-editing teacher. 3. Go to the course. 4. Badges navigation should appear under course administration. 5. If there are any badges, only one action should be available in manage badge table - award a badge. 5a. If there are no badges, add new badge button should not appear. User should see just a message that there are no badges. 6. Try to award a badge. 7a. If the role is not among criteria, you should get a message that it is not possible. 7b. If the role among criteria, you should be able to award badge.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull 2.5 Branch:
      MDL-39986_25_master
    • Rank:
      50727

      Description

      Fixes MDL-39830 and MDL-39831 by modifying to the lib/badgeslib to add a badges node under course administration ADMIN_Category

      //@nneko added else clause to add not for non editing users with award privilege
      function badges_add_course_navigation(navigation_node $coursenode, stdClass $course) {
      global $CFG, $SITE;

      ...

      if (!empty($CFG->enablebadges) && !empty($CFG->badges_allowcoursebadges) && !$isfrontpage) {
      if (has_capability('moodle/badges:configuredetails', $coursecontext)) {

      ...

      else {//Add Badge category for non editing teachers and students that have award badge permission
      if(has_capability('moodle/badges:awardbadge', $coursecontext))

      { $coursenode->add(get_string('coursebadges', 'badges'), null, navigation_node::TYPE_CONTAINER, null, 'coursebadges', new pix_icon('i/badge', get_string('coursebadges', 'badges'))); $url = new moodle_url($CFG->wwwroot . '/badges/index.php', array('type' => BADGE_TYPE_COURSE, 'id' => $course->id)); $coursenode->get('coursebadges')->add(get_string('managebadges', 'badges'), $url, navigation_node::TYPE_SETTING, null, 'coursebadges'); }

      }
      }
      }

        Issue Links

          Activity

          Hide
          Nneko Branche added a comment -

          Modified badges lib

          Show
          Nneko Branche added a comment - Modified badges lib
          Hide
          Nneko Branche added a comment -

          Proposed fix by updating badgeslib to add Badge node under course administration in the settings block.

          Show
          Nneko Branche added a comment - Proposed fix by updating badgeslib to add Badge node under course administration in the settings block.
          Hide
          Lindy Klein added a comment -

          Hi Nneko,
          I'm not a programmer, so am just checking my understanding here. This change seems to check if the permission "award badge" is set to "allow", and if so, display the Badges node in the Administration menu, and if not, don't. If this is correct, then it has my +! for integration as soon as possible!

          Thanks for the speedy response!

          Lindy

          Show
          Lindy Klein added a comment - Hi Nneko, I'm not a programmer, so am just checking my understanding here. This change seems to check if the permission "award badge" is set to "allow", and if so, display the Badges node in the Administration menu, and if not, don't. If this is correct, then it has my +! for integration as soon as possible! Thanks for the speedy response! Lindy
          Hide
          Nneko Branche added a comment - - edited

          Yes, Lindy your interpretation is correct. The original badgeslib only added the managebadges link if the user had configure badges privilege which meant a user would have to be able to add,create, and edit badges to view the badges/index.php page.

          Given, that this was the only place that presented the award badges button it meant that regular users with the award privilege but not the add, edit, delete badges privileges had no way to navigate to the award badges page and would have to enter the URL directly. This corrects that anomaly and as the badges index page verifies privileges before showing UI elements it should be safe.

          However, I am not an expert on Moodle code and only recently started so there maybe more elegant or better ways to achieve this but currently the attached patch to badgeslib does solve the problem.

          Nneko.

          Show
          Nneko Branche added a comment - - edited Yes, Lindy your interpretation is correct. The original badgeslib only added the managebadges link if the user had configure badges privilege which meant a user would have to be able to add,create, and edit badges to view the badges/index.php page. Given, that this was the only place that presented the award badges button it meant that regular users with the award privilege but not the add, edit, delete badges privileges had no way to navigate to the award badges page and would have to enter the URL directly. This corrects that anomaly and as the badges index page verifies privileges before showing UI elements it should be safe. However, I am not an expert on Moodle code and only recently started so there maybe more elegant or better ways to achieve this but currently the attached patch to badgeslib does solve the problem. Nneko.
          Hide
          Mary Cooch added a comment -

          Just adding Yuliya as a watcher since she's badges expert and might be able to advise you Nneko.

          Show
          Mary Cooch added a comment - Just adding Yuliya as a watcher since she's badges expert and might be able to advise you Nneko.
          Hide
          Yuliya Bozhko added a comment -

          Thank, Nneko! I am a bit behind on badges right now, but I will have a look at you contribution and will submit it as soon as possible if it applies.

          Show
          Yuliya Bozhko added a comment - Thank, Nneko! I am a bit behind on badges right now, but I will have a look at you contribution and will submit it as soon as possible if it applies.
          Hide
          Nneko Branche added a comment -
          Show
          Nneko Branche added a comment - Yulia you may pull change from here https://github.com/nneko/moodle/commit/8ef6fa6a5d3a20ff4a201b890bd6b5d821aad68e
          Hide
          Yuliya Bozhko added a comment -

          Thanks, Nneko! I changed your patch a little bit and also fixed some issue which I found while testing it

          Show
          Yuliya Bozhko added a comment - Thanks, Nneko! I changed your patch a little bit and also fixed some issue which I found while testing it
          Hide
          Dan Poltawski added a comment -

          I've converted this to a bug, since really it seems a bug to me that the capability isn't able to control it.

          Show
          Dan Poltawski added a comment - I've converted this to a bug, since really it seems a bug to me that the capability isn't able to control it.
          Hide
          Dan Poltawski added a comment -

          Hi Yuliya,

          I was going to try and get this one integrated and tested, but there seem to be a few problems, so sending it back to you:

          As a user without the capability to add a new badge (actually just non-editing teacher was how I discovered it, the 'add new badge' button was displayed:

          1. When there were no badges at all
          2. When there was a badge that I had the permission to award

          So I think there are a few places which need fixing up

          Show
          Dan Poltawski added a comment - Hi Yuliya, I was going to try and get this one integrated and tested, but there seem to be a few problems, so sending it back to you: As a user without the capability to add a new badge (actually just non-editing teacher was how I discovered it, the 'add new badge' button was displayed: When there were no badges at all When there was a badge that I had the permission to award So I think there are a few places which need fixing up
          Hide
          Yuliya Bozhko added a comment -

          Good catch! Not really a bug as users without permissions can't create a badge anyway, but I agree that it would look better if we prevent people from trying

          Fixed on index page and in renderer, couldn't find anywhere else where users can be adding a new badge.

          Show
          Yuliya Bozhko added a comment - Good catch! Not really a bug as users without permissions can't create a badge anyway, but I agree that it would look better if we prevent people from trying Fixed on index page and in renderer, couldn't find anywhere else where users can be adding a new badge.
          Hide
          Dan Poltawski added a comment -

          I've integrated and tested this now. Thanks Yuliya.

          I think we might need to consider the navigation adding a bit more, but rather than hold up this issue, i'm adding a new issue.

          Show
          Dan Poltawski added a comment - I've integrated and tested this now. Thanks Yuliya. I think we might need to consider the navigation adding a bit more, but rather than hold up this issue, i'm adding a new issue.
          Hide
          Dan Poltawski added a comment -

          MDL-40471 is the issue I was talking about. It seems like the 'manage' link needs to consider a lot more capabilities.

          Show
          Dan Poltawski added a comment - MDL-40471 is the issue I was talking about. It seems like the 'manage' link needs to consider a lot more capabilities.
          Hide
          Damyon Wiese added a comment -

          This issue is fixed! Hurray! Hurray!
          Your issue is fixed, what a wonderful day!

          Cheers, Damyon

          Show
          Damyon Wiese added a comment - This issue is fixed! Hurray! Hurray! Your issue is fixed, what a wonderful day! Cheers, Damyon

            People

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

              Dates

              • Created:
                Updated:
                Resolved: