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

Incorrect call to fullname() in mod/forum/lib.php; post object passed to fullname() instead of user object

    Details

      Description

      I was modifying the fullname function when I noticed an inconsistency.

      Current code:
      $fullname = fullname($post, has_capability('moodle/site:viewfullnames', $modcontext));

      Should be:
      $fullname = fullname($postuser, has_capability('moodle/site:viewfullnames', $modcontext));

      The post contains the user's name anyway so there is no real issue here unless you are in the fullname() function and expecting $user->id there to be the user's ID. The $user->id will be the post ID in this case.

      Patch attached.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting this issue.

            We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported.

            If you believe that this issue is still relevant to current versions (2.3 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

            Michael d;

            4d6f6f646c6521

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.3 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d; 4d6f6f646c6521
            Hide
            cfulton Charles Fulton added a comment -

            The offending code appears to still be there on 2.3 (it's inside forum_print_discussion_header). I'm curious about the downstream effect of this.

            Show
            cfulton Charles Fulton added a comment - The offending code appears to still be there on 2.3 (it's inside forum_print_discussion_header ). I'm curious about the downstream effect of this.
            Hide
            phalacee Jason Fowler added a comment -

            [Y] Syntax
            [-] Output
            [Y] Whitespace
            [-] Language
            [-] Databases
            [N] Testing – This issue will need testing instructions before it can be integrated.
            [-] Security
            [-] Documentation
            [Y] Git
            [Y] Sanity check

            This issue will probably need to back ported as far back as 2.3

            Show
            phalacee Jason Fowler added a comment - [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [N] Testing – This issue will need testing instructions before it can be integrated. [-] Security [-] Documentation [Y] Git [Y] Sanity check This issue will probably need to back ported as far back as 2.3
            Hide
            jacks92 Jayesh Anandani added a comment -

            Can you please elaborate it? I am new to this testing part! So i don't know much of what you wrote!
            Is something wrong with code?

            Show
            jacks92 Jayesh Anandani added a comment - Can you please elaborate it? I am new to this testing part! So i don't know much of what you wrote! Is something wrong with code?
            Hide
            cfulton Charles Fulton added a comment -

            Jayesh Anandani: He just needs some instructions for verifying that your patch works as advertised (which is simple in this case). I've added some. You might also want to reformat your commit message to include the issue name and affected component; e.g. "MDL-26120 mod_forum: use correct object" or some such.

            Show
            cfulton Charles Fulton added a comment - Jayesh Anandani : He just needs some instructions for verifying that your patch works as advertised (which is simple in this case). I've added some. You might also want to reformat your commit message to include the issue name and affected component; e.g. " MDL-26120 mod_forum: use correct object" or some such.
            Hide
            jacks92 Jayesh Anandani added a comment -

            Thanks Charles Fulton!
            Understood and modified the commit message!

            Show
            jacks92 Jayesh Anandani added a comment - Thanks Charles Fulton! Understood and modified the commit message!
            Hide
            salvetore Michael de Raadt added a comment - - edited

            As suggested by Jason, could you please provide a branch for 2.3 and 2.4 so it can go to integration.

            Show
            salvetore Michael de Raadt added a comment - - edited As suggested by Jason, could you please provide a branch for 2.3 and 2.4 so it can go to integration.
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for adding the additional branch fixes, Jayesh.

            Show
            salvetore Michael de Raadt added a comment - Thanks for adding the additional branch fixes, Jayesh.
            Hide
            salvetore Michael de Raadt added a comment -

            Jason: It would be good if you could peer-review this again.

            Show
            salvetore Michael de Raadt added a comment - Jason: It would be good if you could peer-review this again.
            Hide
            phalacee Jason Fowler added a comment -

            Looks as good this time as it did the last time, pushing for integration.

            Show
            phalacee Jason Fowler added a comment - Looks as good this time as it did the last time, pushing for integration.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (23, 24, 25 & master), thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (23, 24, 25 & master), thanks!
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            There are no visible changes, as name was displayed correctly before as well.
            Passing the test.
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - There are no visible changes, as name was displayed correctly before as well. Passing the test. Thanks
            Hide
            damyon Damyon Wiese added a comment -

            Thanks for your contribution! This issue has been reviewed, integrated, tested and now released to everyone.

            Closing as Fixed!

            Show
            damyon Damyon Wiese added a comment - Thanks for your contribution! This issue has been reviewed, integrated, tested and now released to everyone. Closing as Fixed!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/13