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

Error when awarding badge immediately after adding it

    Details

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

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            ybozhko Yuliya Bozhko added a comment -

            Well done spotting this one! I pushed a fix.

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

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

            Show
            salvetore Michael de Raadt added a comment - This will need some testing steps before it can be peer reviewed.
            Hide
            phalacee 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
            phalacee 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
            poltawski Dan Poltawski added a comment -

            I assume the isuserole assignment was causing E_STRICT warning?

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

            Yep, you are very much right

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

            Integrated to master - thanks a lot Yuliya

            Show
            poltawski Dan Poltawski added a comment - Integrated to master - thanks a lot Yuliya
            Hide
            markn 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
            markn 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
            ybozhko 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
            ybozhko 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
            markn 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
            markn 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 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 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
            ybozhko 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
            ybozhko 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 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 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
            poltawski 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
            poltawski 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:
                  Fix Release Date:
                  14/May/13