From 51a607419df8243d62f73cdb5e7abce914ea8c98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Philipp=20Sch=C3=BCle?= <p.schuele@metaways.de>
Date: Tue, 1 Dec 2020 12:12:48 +0100
Subject: [PATCH] fix(Courses/EditDialog): preserve memberships after
 addNewMember
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Change-Id: Id9d52c3971b154feede8bbe4092adc21e92a058b
Reviewed-on: http://gerrit.tine20.com/customers/18613
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
Tested-by: Philipp Schüle <p.schuele@metaways.de>
---
 tests/tine20/Courses/JsonTest.php     | 20 ++++++++++++++++++--
 tine20/Courses/Controller/Course.php  | 12 +++++++++---
 tine20/Courses/js/CourseEditDialog.js | 21 +++++++++++++++------
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/tests/tine20/Courses/JsonTest.php b/tests/tine20/Courses/JsonTest.php
index e76dffdd937..cc4d2b412c0 100644
--- a/tests/tine20/Courses/JsonTest.php
+++ b/tests/tine20/Courses/JsonTest.php
@@ -228,9 +228,25 @@ public function testUpdateCourseSetAdditionalMemberships($checkWithNoRight = fal
         $courseUpdated = $this->_json->saveCourse($courseData);
         self::assertTrue(isset($courseUpdated['members'][0]['additionalGroups']));
         if ($checkWithNoRight) {
-            self::assertEquals([], $courseUpdated['members'][0]['additionalGroups']);
+            self::assertEquals([], $courseUpdated['members'][0]['additionalGroups'],
+                print_r($courseUpdated['members'], true));
         } else {
-            self::assertEquals($memberships, $courseUpdated['members'][0]['additionalGroups']);
+            self::assertEquals($memberships, $courseUpdated['members'][0]['additionalGroups'],
+                print_r($courseUpdated['members'], true));
+        }
+
+        // add new member - assert that this api returns the additionalGroups, too
+        $result = $this->_json->addNewMember(array(
+            'accountFirstName' => 'jams',
+            'accountLastName'  => 'hot',
+        ), $courseUpdated);
+        $teacher = Tinebase_Translation::getTranslation('Courses')->_('Teacher');
+        foreach ($result['results'] as $member) {
+            if (! $checkWithNoRight && strpos($member['name'], $teacher) !== false) {
+                self::assertEquals($memberships, $member['additionalGroups'], print_r($member, true));
+            } else {
+                self::assertEquals([], $member['additionalGroups'], print_r($member, true));
+            }
         }
 
         // remove memberships
diff --git a/tine20/Courses/Controller/Course.php b/tine20/Courses/Controller/Course.php
index 853f064a61e..ff51c2fd80d 100644
--- a/tine20/Courses/Controller/Course.php
+++ b/tine20/Courses/Controller/Course.php
@@ -296,9 +296,15 @@ public function getAdditionalGroupMemberships()
         $groupIds = Courses_Config::getInstance()->get(Courses_Config::ADDITIONAL_GROUP_MEMBERSHIPS, []);
         $additionalGroups = new Tinebase_Record_RecordSet(Tinebase_Model_Group::class);
         foreach ($groupIds as $groupId) {
-            $group = Tinebase_Group::getInstance()->getGroupById($groupId);
-            $group->members = Tinebase_Group::getInstance()->getGroupMembers($groupId);
-            $additionalGroups->addRecord($group);
+            try {
+                $group = Tinebase_Group::getInstance()->getGroupById($groupId);
+                $group->members = Tinebase_Group::getInstance()->getGroupMembers($groupId);
+                $additionalGroups->addRecord($group);
+            } catch (Tinebase_Exception_Record_NotDefined $ternd) {
+                if (Tinebase_Core::isLogLevel(Zend_Log::NOTICE)) Tinebase_Core::getLogger()->notice(
+                    __METHOD__ . '::' . __LINE__ . ' Skipping group: '
+                    . $ternd->getMessage());
+            }
         }
         return $additionalGroups;
     }
diff --git a/tine20/Courses/js/CourseEditDialog.js b/tine20/Courses/js/CourseEditDialog.js
index d4b748f0f3e..40aea8c4ee9 100644
--- a/tine20/Courses/js/CourseEditDialog.js
+++ b/tine20/Courses/js/CourseEditDialog.js
@@ -122,7 +122,7 @@ Tine.Courses.CourseEditDialog = Ext.extend(Tine.widgets.dialog.EditDialog, {
         
         var members = (response.responseText) ? Ext.util.JSON.decode(response.responseText) : response;
         if (members.results.length > 0) {
-            this.membersStore.loadData({results: members.results});
+            this.loadMembersIntoStore(members.results);
         }
         this.hideLoadMask();
     },
@@ -132,12 +132,13 @@ Tine.Courses.CourseEditDialog = Ext.extend(Tine.widgets.dialog.EditDialog, {
      */
     updateToolbars: function() {
     },
-    
+
     /**
-     * onRecordLoad
+     * load members into members store / handle additional groups
+     *
+     * @param Array members
      */
-    onRecordLoad: function() {
-        let members = this.record.get('members') || [];
+    loadMembersIntoStore: function(members) {
         if (members.length > 0) {
             _.each(members, function(member) {
                 _.each(member.additionalGroups, function(groupId) {
@@ -146,7 +147,15 @@ Tine.Courses.CourseEditDialog = Ext.extend(Tine.widgets.dialog.EditDialog, {
             });
             this.membersStore.loadData({results: members});
         }
-        
+    },
+
+    /**
+     * onRecordLoad
+     */
+    onRecordLoad: function() {
+        let members = this.record.get('members') || [];
+        this.loadMembersIntoStore(members);
+
         // only activate import and ok buttons if editing existing course / user has the appropriate right
         var disabled = ! this.record.get('id') 
             || ! Tine.Tinebase.common.hasRight('manage', 'Admin', 'accounts')