Moodle
  1. Moodle
  2. MDL-39049

Error when awarding badge immediately after adding it

    Details

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

      1. Log in as a teacher and go to Settings > Course admin > Badges > Add a new badge
      2. Create a new badge
      3. Add criteria "Manual issue by role". Select "teacher". Save criteria.
      4. Enable access.
      5. Go to "Recipients" tab and click "Award badge" button.
      6. Select students on the right to award badge.
      7. Click "Award badge".
      8. Selected users should appear in the list to the left. No error message should appear.
      9. Go back to the Recipients tab. Previously selected users should in the table of badge recipients.

      Show
      1. Log in as a teacher and go to Settings > Course admin > Badges > Add a new badge 2. Create a new badge 3. Add criteria "Manual issue by role". Select "teacher". Save criteria. 4. Enable access. 5. Go to "Recipients" tab and click "Award badge" button. 6. Select students on the right to award badge. 7. Click "Award badge". 8. Selected users should appear in the list to the left. No error message should appear. 9. Go back to the Recipients tab. Previously selected users should in the table of badge recipients.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
      git@github.com:totara/openbadges.git
    • Pull Master Branch:
      MDL-39049_master
    • Rank:
      49170

      Description

      As a teacher in a course if you add a new badge and then try to award it from the Recipients tab you get the following error message.

      Debug info: 
      Error code: missingparam
      Stack trace:
      line 476 of /lib/setuplib.php: moodle_exception thrown
      line 532 of /lib/moodlelib.php: call to print_error()
      line 31 of /badges/award.php: call to required_param()
      

      If however, you save it and go back to Course admin>Manage badges and award by clicking the award icon, you are then able to award the badge.

      Steps to reproduce:
      1. As a teacher go to Settings>Course admin>Badges>Add a new badge
      2. Add a new badge
      3. Add criteria "manual issue by role>teacher" and enable access.
      4. Click the "Recipients" tab and the "award badge" button.
      5. Add a student from the right over to the left. Error message appears.
      6. Go back to Settings>Course admin>Badges>Manage badges
      7. Click the "award badge" icon to get back to that same screen and try to award a student the badge again. It works.

        Issue Links

          Activity

          Hide
          Yuliya Bozhko added a comment -

          Well done spotting this one! I pushed a fix.

          Show
          Yuliya Bozhko added a comment - Well done spotting this one! I pushed a fix.
          Hide
          Michael de Raadt added a comment -

          This will need some testing steps before it can be peer reviewed.

          Show
          Michael de Raadt added a comment - This will need some testing steps before it can be peer reviewed.
          Hide
          Jason Fowler added a comment -

          [Y] Syntax
          [-] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [Y] Testing
          [-] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          Code looks cleaner now that the issuerrole assignment has been split. Good work Yuliya.

          Show
          Jason Fowler added a comment - [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Code looks cleaner now that the issuerrole assignment has been split. Good work Yuliya.
          Hide
          Dan Poltawski added a comment -

          I assume the isuserole assignment was causing E_STRICT warning?

          Show
          Dan Poltawski added a comment - I assume the isuserole assignment was causing E_STRICT warning?
          Hide
          Yuliya Bozhko added a comment -

          Yep, you are very much right

          Show
          Yuliya Bozhko added a comment - Yep, you are very much right
          Hide
          Dan Poltawski added a comment -

          Integrated to master - thanks a lot Yuliya

          Show
          Dan Poltawski added a comment - Integrated to master - thanks a lot Yuliya
          Hide
          Mark Nelson added a comment - - edited

          When I select the users and click on the 'Award badge' button I am taken back to the page where I am told to select a 'role to award this badge' with the data never being processed, ie the badge is never awarded to those students.

          Show
          Mark Nelson added a comment - - edited When I select the users and click on the 'Award badge' button I am taken back to the page where I am told to select a 'role to award this badge' with the data never being processed, ie the badge is never awarded to those students.
          Hide
          Yuliya Bozhko added a comment -

          I think by fixing one issue I might have caused another Getting this problem too. It's fixed when I add required parameter to URL. I updated tracker with a new patch

          Show
          Yuliya Bozhko added a comment - I think by fixing one issue I might have caused another Getting this problem too. It's fixed when I add required parameter to URL. I updated tracker with a new patch
          Hide
          Mark Nelson added a comment -

          Hi Yuliya, I applied your recent commit which fixes the issue. Thanks. Once an integrator has applied your fix I can pass this test.

          Show
          Mark Nelson added a comment - Hi Yuliya, I applied your recent commit which fixes the issue. Thanks. Once an integrator has applied your fix I can pass this test.
          Hide
          Damyon Wiese added a comment -

          Hi Yuliya,

          While the patch fixes this specific issue - I spotted a logic error.

          A user can have multiple roles in a course and you are only checking the first one returned from "get_user_roles" - you need to loop through that list and only print an error if none of the user roles are in the accepted roles list.

          How to reproduce: Create a badge that can be awarded by the Teacher role. Give a user teacher + course creator roles in a course. Attempt to award the badge as that teacher (should be allowed).

          Can you fix this?

          Thanks, Damyon

          Show
          Damyon Wiese added a comment - Hi Yuliya, While the patch fixes this specific issue - I spotted a logic error. A user can have multiple roles in a course and you are only checking the first one returned from "get_user_roles" - you need to loop through that list and only print an error if none of the user roles are in the accepted roles list. How to reproduce: Create a badge that can be awarded by the Teacher role. Give a user teacher + course creator roles in a course. Attempt to award the badge as that teacher (should be allowed). Can you fix this? Thanks, Damyon
          Hide
          Yuliya Bozhko added a comment -

          Yeah, it was a part of initial architecture: admin was supposed to select a role they would like to use, all others are awarding badges with their highest role in the context. I can have a look and rework it to take into account multiple roles even if a user is not an admin.

          Show
          Yuliya Bozhko added a comment - Yeah, it was a part of initial architecture: admin was supposed to select a role they would like to use, all others are awarding badges with their highest role in the context. I can have a look and rework it to take into account multiple roles even if a user is not an admin.
          Hide
          Damyon Wiese added a comment -

          Passing this re Marks comment (the extra fix has been pushed to master). Will open a new issue for the multiple roles problem.

          Show
          Damyon Wiese added a comment - Passing this re Marks comment (the extra fix has been pushed to master). Will open a new issue for the multiple roles problem.
          Hide
          Dan Poltawski added a comment -

          Blooming Marvelous! It's time for a knees up - your changes are upstream!

          Thanks for making Moodle better!

          Toodle pip

          Show
          Dan Poltawski added a comment - Blooming Marvelous! It's time for a knees up - your changes are upstream! Thanks for making Moodle better! Toodle pip

            People

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

              Dates

              • Created:
                Updated:
                Resolved: