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

    • Rank:
      15719

      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.

      1. mod_forum_lib.patch
        0.5 kB
        Stephen Mc Guinness (Enovation)

        Activity

        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        Jayesh Anandani added a comment -

        Thanks Charles Fulton!
        Understood and modified the commit message!

        Show
        Jayesh Anandani added a comment - Thanks Charles Fulton! Understood and modified the commit message!
        Hide
        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
        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
        Michael de Raadt added a comment -

        Thanks for adding the additional branch fixes, Jayesh.

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

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

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

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

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

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

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

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

        Show
        Ankit Agarwal added a comment - There are no visible changes, as name was displayed correctly before as well. Passing the test. Thanks
        Hide
        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 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: