Moodle
  1. Moodle
  2. MDL-41339

Entering an event tag from the block displays duplicate value error

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.4.7, 2.5.3, 2.6
    • Component/s: Tags
    • Labels:
    • Testing Instructions:
      Hide

      enable developer level of debugging output
      enable course tags
      create a course
      add the tag block to it
      add a tags: fred
      log on as a different user
      go to the same course and the same tag

      Before patch is applied, you should see the PHP warning

      Show
      enable developer level of debugging output enable course tags create a course add the tag block to it add a tags: fred log on as a different user go to the same course and the same tag Before patch is applied, you should see the PHP warning
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-mdl-41339

      Description

      If you enable tags for courses, add the tags block, and then enter in the same tag with different users, you get the following error:

      Did you remember to make the first column something unique in your call to get_records? Duplicate value '4' found in column 'id'.

      line 771 of /lib/dml/pgsql_native_moodle_database.php: call to debugging()
      line 336 of /tag/lib.php: call to pgsql_native_moodle_database->get_records_sql()
      line 405 of /tag/lib.php: call to tag_get_tags()
      line 291 of /tag/coursetagslib.php: call to tag_get_tags_ids()
      line 51 of /tag/coursetags_add.php: call to coursetag_store_keywords()

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Jenny Gray added a comment -

            This error occurs from the tag_get_tags() function who's SQL returns tg.id, tg.tagtype, tg.name, tg.rawname, tg.flag, ti.ordering where tg is the tag table and ti is tag_instance.

            It is possible for there to be multiple entries in ti with the same data but for different user accounts - as Adrian noted, this is the correct data when more than one person uses the same tag term for the same thing (course/blog post/whatever).

            The error is thrown because the first column requested contains the tag id, which is rightly not unique.

            The solution is, I think, to add ti.id to be the first column returned. It won't be used by any of the myriad functions which call tag_get_tags but it won't get in their way either.

            I considered alternatives of stripping out ti.ordering and making the result set distinct on tag id but tag_get_tag_ids uses ordering, so this doesn't seem appropriate. None of the current columns contain unique values and so the ordering of the returned columns cannot be altered to fix the bug.

            Show
            Jenny Gray added a comment - This error occurs from the tag_get_tags() function who's SQL returns tg.id, tg.tagtype, tg.name, tg.rawname, tg.flag, ti.ordering where tg is the tag table and ti is tag_instance. It is possible for there to be multiple entries in ti with the same data but for different user accounts - as Adrian noted, this is the correct data when more than one person uses the same tag term for the same thing (course/blog post/whatever). The error is thrown because the first column requested contains the tag id, which is rightly not unique. The solution is, I think, to add ti.id to be the first column returned. It won't be used by any of the myriad functions which call tag_get_tags but it won't get in their way either. I considered alternatives of stripping out ti.ordering and making the result set distinct on tag id but tag_get_tag_ids uses ordering, so this doesn't seem appropriate. None of the current columns contain unique values and so the ordering of the returned columns cannot be altered to fix the bug.
            Hide
            Adrian Greeve added a comment -

            Hello Jenny,

            I like your solution. Just a couple of things before this heads to integration.

            Could you please add some testing instructions, and also add branches for 2.5 and 2.4?

            Thank you.

            Show
            Adrian Greeve added a comment - Hello Jenny, I like your solution. Just a couple of things before this heads to integration. Could you please add some testing instructions, and also add branches for 2.5 and 2.4? Thank you.
            Hide
            Jenny Gray added a comment -

            Rebased to current master. Created 2.4 and 2.5 branches and added testing instructions.

            Show
            Jenny Gray added a comment - Rebased to current master. Created 2.4 and 2.5 branches and added testing instructions.
            Hide
            Jenny Gray added a comment -

            Ready for integration I think, if you're happy Adrian?

            Show
            Jenny Gray added a comment - Ready for integration I think, if you're happy Adrian?
            Hide
            Adrian Greeve added a comment -

            Yes, of course. I think this is ready.

            Show
            Adrian Greeve added a comment - Yes, of course. I think this is ready.
            Hide
            Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            Damyon Wiese added a comment -

            Thanks Jenny. I have integrated this to 24, 25 and master.

            Note: I added one extra commit to change the SQL to use 'AS' instead of as.

            See point 2 in the general section here:

            http://docs.moodle.org/dev/SQL_coding_style

            Show
            Damyon Wiese added a comment - Thanks Jenny. I have integrated this to 24, 25 and master. Note: I added one extra commit to change the SQL to use 'AS' instead of as. See point 2 in the general section here: http://docs.moodle.org/dev/SQL_coding_style
            Hide
            David Monllaó added a comment -

            It passes; I can add again the same tag without any warning. Tested in 24, 25 and master.

            Show
            David Monllaó added a comment - It passes; I can add again the same tag without any warning. Tested in 24, 25 and master.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            "Aequam memento rebus in arduis servare mentem"

            Many thanks for your hard work, this is now part of "Moodle, the LMS". Closing!

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - "Aequam memento rebus in arduis servare mentem" Many thanks for your hard work, this is now part of "Moodle, the LMS". Closing! Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: