From 23e0ceca1658709db5850a0dc0a944c2a1a9bf15 Mon Sep 17 00:00:00 2001
From: Ryan Wyllie <ryan@moodle.com>
Date: Mon, 9 Sep 2019 15:44:48 +0800
Subject: [PATCH] MDL-64821 mod_forum: don't show name of deleted user

When a user account is deleted don't render the user's name in
the forum. Instead we render "Deleted user".
---
 .../local/data_mappers/legacy/author.php      |  1 +
 mod/forum/classes/local/entities/author.php   | 13 ++++
 mod/forum/classes/local/exporters/author.php  | 71 ++++++++++++-------
 mod/forum/classes/local/exporters/post.php    |  6 +-
 mod/forum/classes/local/factories/entity.php  |  1 +
 .../classes/local/vaults/discussion_list.php  |  4 +-
 .../vaults/preprocessors/extract_user.php     |  2 +-
 mod/forum/lang/en/forum.php                   |  1 +
 ...orum_discussion_modern_first_post.mustache | 14 ++--
 ...orum_discussion_modern_post_reply.mustache |  6 +-
 mod/forum/tests/entities_author_test.php      |  2 +
 .../entities_discussion_summary_test.php      |  6 +-
 mod/forum/tests/exporters_author_test.php     |  9 ++-
 mod/forum/tests/externallib_test.php          | 12 ++++
 14 files changed, 102 insertions(+), 46 deletions(-)

diff --git a/mod/forum/classes/local/data_mappers/legacy/author.php b/mod/forum/classes/local/data_mappers/legacy/author.php
index a8ddb12f1b137..b62e5a6f5dae7 100644
--- a/mod/forum/classes/local/data_mappers/legacy/author.php
+++ b/mod/forum/classes/local/data_mappers/legacy/author.php
@@ -51,6 +51,7 @@ public function to_legacy_objects(array $authors) : array {
                 'lastname' => $author->get_last_name(),
                 'fullname' => $author->get_full_name(),
                 'email' => $author->get_email(),
+                'deleted' => $author->is_deleted(),
                 'middlename' => $author->get_middle_name(),
                 'firstnamephonetic' => $author->get_first_name_phonetic(),
                 'lastnamephonetic' => $author->get_last_name_phonetic(),
diff --git a/mod/forum/classes/local/entities/author.php b/mod/forum/classes/local/entities/author.php
index d2ed541cd689d..527ecedaa2df4 100644
--- a/mod/forum/classes/local/entities/author.php
+++ b/mod/forum/classes/local/entities/author.php
@@ -45,6 +45,8 @@ class author {
     private $fullname;
     /** @var string $email Email */
     private $email;
+    /** @var bool $deleted Deleted */
+    private $deleted;
     /** @var string $middlename Middle name */
     private $middlename;
     /** @var string $firstnamephonetic Phonetic spelling of first name */
@@ -78,6 +80,7 @@ public function __construct(
         string $lastname,
         string $fullname,
         string $email,
+        bool $deleted,
         string $middlename = null,
         string $firstnamephonetic = null,
         string $lastnamephonetic = null,
@@ -90,6 +93,7 @@ public function __construct(
         $this->lastname = $lastname;
         $this->fullname = $fullname;
         $this->email = $email;
+        $this->deleted = $deleted;
         $this->middlename = $middlename;
         $this->firstnamephonetic = $firstnamephonetic;
         $this->lastnamephonetic = $lastnamephonetic;
@@ -151,6 +155,15 @@ public function get_email() : string {
         return $this->email;
     }
 
+    /**
+     * Is the author deleted?
+     *
+     * @return bool
+     */
+    public function is_deleted() : bool {
+        return !empty($this->deleted);
+    }
+
     /**
      * Return the middle name.
      *
diff --git a/mod/forum/classes/local/exporters/author.php b/mod/forum/classes/local/exporters/author.php
index 7beb5d56564d5..21508abd94f74 100644
--- a/mod/forum/classes/local/exporters/author.php
+++ b/mod/forum/classes/local/exporters/author.php
@@ -91,6 +91,12 @@ protected static function define_other_properties() {
                 'default' => null,
                 'null' => NULL_ALLOWED
             ],
+            'isdeleted' => [
+                'type' => PARAM_BOOL,
+                'optional' => true,
+                'default' => null,
+                'null' => NULL_ALLOWED
+            ],
             'groups' => [
                 'multiple' => true,
                 'optional' => true,
@@ -143,41 +149,56 @@ protected function get_other_values(renderer_base $output) {
         $context = $this->related['context'];
 
         if ($this->canview) {
-            $groups = array_map(function($group) use ($urlfactory, $context) {
-                $imageurl = null;
-                $groupurl = null;
-                if (!$group->hidepicture) {
-                    $imageurl = get_group_picture_url($group, $group->courseid, true);
-                }
-                if (course_can_view_participants($context)) {
-                    $groupurl = $urlfactory->get_author_group_url($group);
-                }
-
+            if ($author->is_deleted()) {
                 return [
-                    'id' => $group->id,
-                    'name' => $group->name,
+                    'id' => $author->get_id(),
+                    'fullname' => get_string('deleteduser', 'mod_forum'),
+                    'isdeleted' => true,
+                    'groups' => [],
                     'urls' => [
-                        'image' => $imageurl ? $imageurl->out(false) : null,
-                        'group' => $groupurl ? $groupurl->out(false) : null
-
+                        'profile' => ($urlfactory->get_author_profile_url($author))->out(false),
+                        'profileimage' => ($urlfactory->get_author_profile_image_url($author, $authorcontextid))->out(false)
                     ]
                 ];
-            }, $this->authorgroups);
+            } else {
+                $groups = array_map(function($group) use ($urlfactory, $context) {
+                    $imageurl = null;
+                    $groupurl = null;
+                    if (!$group->hidepicture) {
+                        $imageurl = get_group_picture_url($group, $group->courseid, true);
+                    }
+                    if (course_can_view_participants($context)) {
+                        $groupurl = $urlfactory->get_author_group_url($group);
+                    }
 
-            return [
-                'id' => $author->get_id(),
-                'fullname' => $author->get_full_name(),
-                'groups' => $groups,
-                'urls' => [
-                    'profile' => ($urlfactory->get_author_profile_url($author))->out(false),
-                    'profileimage' => ($urlfactory->get_author_profile_image_url($author, $authorcontextid))->out(false)
-                ]
-            ];
+                    return [
+                        'id' => $group->id,
+                        'name' => $group->name,
+                        'urls' => [
+                            'image' => $imageurl ? $imageurl->out(false) : null,
+                            'group' => $groupurl ? $groupurl->out(false) : null
+
+                        ]
+                    ];
+                }, $this->authorgroups);
+
+                return [
+                    'id' => $author->get_id(),
+                    'fullname' => $author->get_full_name(),
+                    'isdeleted' => false,
+                    'groups' => $groups,
+                    'urls' => [
+                        'profile' => ($urlfactory->get_author_profile_url($author))->out(false),
+                        'profileimage' => ($urlfactory->get_author_profile_image_url($author, $authorcontextid))->out(false)
+                    ]
+                ];
+            }
         } else {
             // The author should be anonymised.
             return [
                 'id' => null,
                 'fullname' => get_string('forumauthorhidden', 'mod_forum'),
+                'isdeleted' => null,
                 'groups' => [],
                 'urls' => [
                     'profile' => null,
diff --git a/mod/forum/classes/local/exporters/post.php b/mod/forum/classes/local/exporters/post.php
index 0d23672d37830..f854e4613f38b 100644
--- a/mod/forum/classes/local/exporters/post.php
+++ b/mod/forum/classes/local/exporters/post.php
@@ -386,7 +386,7 @@ protected function get_other_values(renderer_base $output) {
             $author,
             $authorcontextid,
             $authorgroups,
-            ($canview && !$isdeleted),
+            $canview,
             $this->related
         );
         $exportedauthor = $authorexporter->export($output);
@@ -402,10 +402,6 @@ protected function get_other_values(renderer_base $output) {
             $subject = $isdeleted ? get_string('forumsubjectdeleted', 'forum') : get_string('forumsubjecthidden', 'forum');
             $message = $isdeleted ? get_string('forumbodydeleted', 'forum') : get_string('forumbodyhidden', 'forum');
             $timecreated = null;
-
-            if ($isdeleted) {
-                $exportedauthor->fullname = null;
-            }
         }
 
         $replysubject = $subject;
diff --git a/mod/forum/classes/local/factories/entity.php b/mod/forum/classes/local/factories/entity.php
index 55263f025c0f7..e40e59634feae 100644
--- a/mod/forum/classes/local/factories/entity.php
+++ b/mod/forum/classes/local/factories/entity.php
@@ -172,6 +172,7 @@ public function get_author_from_stdclass(stdClass $record) : author_entity {
             $record->lastname,
             fullname($record),
             $record->email,
+            $record->deleted,
             $record->middlename,
             $record->firstnamephonetic,
             $record->lastnamephonetic,
diff --git a/mod/forum/classes/local/vaults/discussion_list.php b/mod/forum/classes/local/vaults/discussion_list.php
index 6cfcebf2f58ef..a4a345b76e647 100644
--- a/mod/forum/classes/local/vaults/discussion_list.php
+++ b/mod/forum/classes/local/vaults/discussion_list.php
@@ -120,8 +120,8 @@ protected function generate_get_records_sql(string $wheresql = null, ?string $so
         // - Most recent editor.
         $thistable = new dml_table(self::TABLE, $alias, $alias);
         $posttable = new dml_table('forum_posts', 'fp', 'p_');
-        $firstauthorfields = \user_picture::fields('fa', null, self::FIRST_AUTHOR_ID_ALIAS, self::FIRST_AUTHOR_ALIAS);
-        $latestuserfields = \user_picture::fields('la', null, self::LATEST_AUTHOR_ID_ALIAS, self::LATEST_AUTHOR_ALIAS);
+        $firstauthorfields = \user_picture::fields('fa', ['deleted'], self::FIRST_AUTHOR_ID_ALIAS, self::FIRST_AUTHOR_ALIAS);
+        $latestuserfields = \user_picture::fields('la', ['deleted'], self::LATEST_AUTHOR_ID_ALIAS, self::LATEST_AUTHOR_ALIAS);
 
         $fields = implode(', ', [
             $thistable->get_field_select(),
diff --git a/mod/forum/classes/local/vaults/preprocessors/extract_user.php b/mod/forum/classes/local/vaults/preprocessors/extract_user.php
index ae6aa6b9a80d4..ae01ce21fbda1 100644
--- a/mod/forum/classes/local/vaults/preprocessors/extract_user.php
+++ b/mod/forum/classes/local/vaults/preprocessors/extract_user.php
@@ -65,7 +65,7 @@ public function execute(array $records) : array {
         $alias = $this->alias;
 
         return array_map(function($record) use ($idalias, $alias) {
-            return user_picture::unalias($record, null, $idalias, $alias);
+            return user_picture::unalias($record, ['deleted'], $idalias, $alias);
         }, $records);
     }
 }
\ No newline at end of file
diff --git a/mod/forum/lang/en/forum.php b/mod/forum/lang/en/forum.php
index 71c312f3568e3..be2f700003223 100644
--- a/mod/forum/lang/en/forum.php
+++ b/mod/forum/lang/en/forum.php
@@ -146,6 +146,7 @@
 $string['deleteddiscussion'] = 'The discussion topic has been deleted';
 $string['deletedpost'] = 'The post has been deleted';
 $string['deletedposts'] = 'Those posts have been deleted';
+$string['deleteduser'] = 'Deleted user';
 $string['deletesure'] = 'Are you sure you want to delete this post?';
 $string['deletesureplural'] = 'Are you sure you want to delete this post and all replies? ({$a} posts)';
 $string['digestmailheader'] = 'This is your daily digest of new posts from the {$a->sitename} forums. To change your default forum email preferences, go to {$a->userprefs}.';
diff --git a/mod/forum/templates/forum_discussion_modern_first_post.mustache b/mod/forum/templates/forum_discussion_modern_first_post.mustache
index 7abcf15bb3ee6..3df50fabec1b9 100644
--- a/mod/forum/templates/forum_discussion_modern_first_post.mustache
+++ b/mod/forum/templates/forum_discussion_modern_first_post.mustache
@@ -236,14 +236,14 @@
                             {{/readonly}}
                             </div>
                     </div>
-                    {{$subject}}
-                        <h2
-                            class="h1 font-weight-bold post-subject mt-n2"
-                            data-region-content="forum-post-core-subject"
-                            data-reply-subject="{{replysubject}}"
-                        >{{{subject}}}</h2>
-                    {{/subject}}
                 {{/isdeleted}}
+                {{$subject}}
+                    <h2
+                        class="h1 font-weight-bold post-subject mt-n2"
+                        data-region-content="forum-post-core-subject"
+                        data-reply-subject="{{replysubject}}"
+                    >{{{subject}}}</h2>
+                {{/subject}}
                 {{#hasreplycount}}
                     <span class="sr-only">{{#str}} numberofreplies, mod_forum, {{replycount}} {{/str}}</span>
                 {{/hasreplycount}}
diff --git a/mod/forum/templates/forum_discussion_modern_post_reply.mustache b/mod/forum/templates/forum_discussion_modern_post_reply.mustache
index 0d7209fe609ac..4bdf5ef45ee8b 100644
--- a/mod/forum/templates/forum_discussion_modern_post_reply.mustache
+++ b/mod/forum/templates/forum_discussion_modern_post_reply.mustache
@@ -31,7 +31,11 @@
 }}
 {{< mod_forum/forum_discussion_modern_first_post }}
     {{$subject}}
-        <h3 class="sr-only" data-region-content="forum-post-core-subject">{{{subject}}}</h3>
+        <h3
+            {{#isdeleted}}class="h6 font-weight-bold"{{/isdeleted}}
+            {{^isdeleted}}class="sr-only"{{/isdeleted}}
+            data-region-content="forum-post-core-subject"
+        >{{{subject}}}</h3>
     {{/subject}}
     {{$footer}}
         {{^isdeleted}}
diff --git a/mod/forum/tests/entities_author_test.php b/mod/forum/tests/entities_author_test.php
index 7e44f89d932c8..198566fbb2463 100644
--- a/mod/forum/tests/entities_author_test.php
+++ b/mod/forum/tests/entities_author_test.php
@@ -47,6 +47,7 @@ public function test_entity() {
             'person',
             'test person',
             'test@example.com',
+            false,
             'middle',
             'tteeeeest',
             'ppppeeerssson',
@@ -60,6 +61,7 @@ public function test_entity() {
         $this->assertEquals('person', $author->get_last_name());
         $this->assertEquals('test person', $author->get_full_name());
         $this->assertEquals('test@example.com', $author->get_email());
+        $this->assertEquals(false, $author->is_deleted());
         $this->assertEquals('middle', $author->get_middle_name());
         $this->assertEquals('tteeeeest', $author->get_first_name_phonetic());
         $this->assertEquals('ppppeeerssson', $author->get_last_name_phonetic());
diff --git a/mod/forum/tests/entities_discussion_summary_test.php b/mod/forum/tests/entities_discussion_summary_test.php
index 5674237829a31..5475d5251dbd4 100644
--- a/mod/forum/tests/entities_discussion_summary_test.php
+++ b/mod/forum/tests/entities_discussion_summary_test.php
@@ -49,7 +49,8 @@ public function test_entity() {
             'test',
             'person',
             'test person',
-            'test@example.com'
+            'test@example.com',
+            false
         );
         $lastauthor = new author_entity(
             2,
@@ -57,7 +58,8 @@ public function test_entity() {
             'test 2',
             'person 2',
             'test 2 person 2',
-            'test2@example.com'
+            'test2@example.com',
+            false
         );
         $discussion = new discussion_entity(
             1,
diff --git a/mod/forum/tests/exporters_author_test.php b/mod/forum/tests/exporters_author_test.php
index 9942e7f26e1a3..3b5dc5755c308 100644
--- a/mod/forum/tests/exporters_author_test.php
+++ b/mod/forum/tests/exporters_author_test.php
@@ -57,7 +57,8 @@ public function test_export_author() {
             'test',
             'user',
             'test user',
-            'test@example.com'
+            'test@example.com',
+            false
         );
 
         $exporter = new author_exporter($author, 1, [], true, [
@@ -95,7 +96,8 @@ public function test_export_author_with_groups() {
             'test',
             'user',
             'test user',
-            'test@example.com'
+            'test@example.com',
+            false
         );
 
         $group = $datagenerator->create_group(['courseid' => $course->id]);
@@ -132,7 +134,8 @@ public function test_export_author_no_view_capability() {
             'test',
             'user',
             'test user',
-            'test@example.com'
+            'test@example.com',
+            false
         );
 
         $group = $datagenerator->create_group(['courseid' => $course->id]);
diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php
index 28cafe887c76a..84428a5a59071 100644
--- a/mod/forum/tests/externallib_test.php
+++ b/mod/forum/tests/externallib_test.php
@@ -548,6 +548,7 @@ public function test_mod_forum_get_discussion_posts() {
         $exporteduser2 = [
             'id' => (int) $user2->id,
             'fullname' => fullname($user2),
+            'isdeleted' => false,
             'groups' => [],
             'urls' => [
                 'profile' => $urlfactory->get_author_profile_url($user2entity),
@@ -562,6 +563,7 @@ public function test_mod_forum_get_discussion_posts() {
             'id' => (int) $user3->id,
             'fullname' => fullname($user3),
             'groups' => [],
+            'isdeleted' => false,
             'urls' => [
                 'profile' => $urlfactory->get_author_profile_url($user3entity),
                 'profileimage' => $urlfactory->get_author_profile_image_url($user3entity),
@@ -642,6 +644,16 @@ public function test_mod_forum_get_discussion_posts() {
 
         // Delete one user, to test that we still receive posts by this user.
         delete_user($user3);
+        $exporteduser3 = [
+            'id' => (int) $user3->id,
+            'fullname' => get_string('deleteduser', 'mod_forum'),
+            'groups' => [],
+            'isdeleted' => true,
+            'urls' => [
+                'profile' => $urlfactory->get_author_profile_url($user3entity),
+                'profileimage' => $urlfactory->get_author_profile_image_url($user3entity),
+            ]
+        ];
 
         // Create what we expect to be returned when querying the discussion.
         $expectedposts = array(