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

Searching for users in messaging with multiple roles generates a warning

          Details

          • Type: Bug
          • Status: Closed
          • Priority: Minor
          • Resolution: Fixed
          • Affects Version/s: 2.1.5, 2.2.2, 2.3
          • Fix Version/s: 2.1.6, 2.2.3
          • Component/s: Messages
          • Labels:
          • Database:
            Any
          • Testing Instructions:
            Hide

            Enrol a user in a course (if they arent already)
            Go to the enrolled users screen for that course and give that user a second role (click the + sign in the role column)
            Go to messages and go to advanced search
            Tick "Only in my courses" and search for the name of the user with 2 roles.
            Check that you don't get any error.

            Remove the user's 2nd role and repeat the search. Check that you still don't get any error.

            (suggestion: test on more than 1 DB type)

            Show
            Enrol a user in a course (if they arent already) Go to the enrolled users screen for that course and give that user a second role (click the + sign in the role column) Go to messages and go to advanced search Tick "Only in my courses" and search for the name of the user with 2 roles. Check that you don't get any error. Remove the user's 2nd role and repeat the search. Check that you still don't get any error. (suggestion: test on more than 1 DB type)
          • Affected Branches:
            MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
          • Fixed Branches:
            MOODLE_21_STABLE, MOODLE_22_STABLE
          • Pull Master Branch:
            MDL-31869_message_search_roles
          • Pull Master Diff URL:

            Description

            If you search for user x and they have multiple roles like student and teacher you will get this error relating to their user id a duplicate in the returning SQL.

            Did you remember to make the first column something unique in your call to get_records? Duplicate value '4' found in column 'id'.
            line 705 of /lib/dml/pgsql_native_moodle_database.php: call to debugging()
            line 1515 of /message/lib.php: call to pgsql_native_moodle_database->get_records_sql()
            line 1034 of /message/lib.php: call to message_search_users()
            line 653 of /message/lib.php: call to message_print_search_results()
            line 300 of /message/index.php: call to message_print_search()

              Gliffy Diagrams

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  andyjdavis Andrew Davis added a comment -

                  Updated testing instructions.

                  Show
                  andyjdavis Andrew Davis added a comment - Updated testing instructions.
                  Hide
                  Permalink
                  andyjdavis Andrew Davis added a comment -

                  Looks good. Adding master branch and putting up for peer review.

                  Show
                  andyjdavis Andrew Davis added a comment - Looks good. Adding master branch and putting up for peer review.
                  Hide
                  Permalink
                  poltawski Dan Poltawski added a comment - - edited

                  One thing to watch out for with adding DISTINCT to queries is any field with text columns (see MDL-26371 )as this is non cross-db compatible.

                  As this query is using only ids and user field needing for user picture [which dont include the TEXT description] this looks good - Thanks!

                  Show
                  poltawski Dan Poltawski added a comment - - edited One thing to watch out for with adding DISTINCT to queries is any field with text columns (see MDL-26371 )as this is non cross-db compatible. As this query is using only ids and user field needing for user picture [which dont include the TEXT description] this looks good - Thanks!
                  Hide
                  Permalink
                  andyjdavis Andrew Davis added a comment -

                  Submitting for integration.

                  Show
                  andyjdavis Andrew Davis added a comment - Submitting for integration.
                  Hide
                  Permalink
                  nebgor Aparup Banerjee added a comment -

                  This looks good.

                  Dan - i've suggested MDL-32170.

                  As this a SQL change , i suggest Tester to test across more than 1 DB.

                  Show
                  nebgor Aparup Banerjee added a comment - This looks good. Dan - i've suggested MDL-32170 . As this a SQL change , i suggest Tester to test across more than 1 DB.
                  Hide
                  Permalink
                  nebgor Aparup Banerjee added a comment -

                  This has now been integrated and is ready for testing.

                  Show
                  nebgor Aparup Banerjee added a comment - This has now been integrated and is ready for testing.
                  Hide
                  Permalink
                  abgreeve Adrian Greeve added a comment -

                  Tested on all branches. I also tested it on MSSQL. No problems.
                  Test passed.
                  Thanks.

                  Show
                  abgreeve Adrian Greeve added a comment - Tested on all branches. I also tested it on MSSQL. No problems. Test passed. Thanks.
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  And this has landed upstream, finally! Yay!

                  תודה רבה && شكرا جزيلا



                  Closing, ciao

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - And this has landed upstream, finally! Yay! תודה רבה && شكرا جزيلا Closing, ciao

                    People

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

                      Dates

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