From 3d6e9f573c39cadc3af2f3eff89eee2c4af68b34 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Mon, 3 Jul 2023 08:15:36 +0000
Subject: [PATCH] fix errors and warnings, fixes #2803

Closes #2803

Merge request studip/studip!1890
---
 app/controllers/admin/user.php                |  9 ++---
 app/controllers/course/messenger.php          |  4 ++-
 app/controllers/jsupdater.php                 |  3 +-
 app/controllers/profile.php                   | 36 +++++++++++++++----
 app/views/admin/user/new.php                  |  6 ++--
 .../course/ilias_interface/add_object.php     |  2 +-
 app/views/my_ilias_accounts/add_object.php    |  4 +--
 lib/classes/ProfileModel.php                  |  5 ++-
 lib/functions.php                             | 13 +++----
 lib/models/UserDomain.php                     | 10 ++++--
 lib/user_visible.inc.php                      |  5 +--
 11 files changed, 64 insertions(+), 33 deletions(-)

diff --git a/app/controllers/admin/user.php b/app/controllers/admin/user.php
index 8932f48d8ba..33d56a5080a 100644
--- a/app/controllers/admin/user.php
+++ b/app/controllers/admin/user.php
@@ -891,16 +891,14 @@ class Admin_UserController extends AuthenticatedController
         }
 
         if ($GLOBALS['perm']->have_perm('root')) {
-            $sql
-                     = "SELECT Institut_id, Name, 1 AS is_fak
+            $sql = "SELECT Institut_id, Name, 1 AS is_fak
                     FROM Institute
                     WHERE Institut_id=fakultaets_id
                     ORDER BY Name";
             $faks    = DBManager::get()->query($sql)->fetchAll(PDO::FETCH_ASSOC);
             $domains = UserDomain::getUserDomains();
         } else {
-            $sql
-                       = "SELECT a.Institut_id, Name, b.Institut_id = b.fakultaets_id AS is_fak
+            $sql = "SELECT a.Institut_id, Name, b.Institut_id = b.fakultaets_id AS is_fak
                     FROM user_inst a
                     LEFT JOIN Institute b USING (Institut_id)
                     WHERE a.user_id = ? AND a.inst_perms = 'admin'
@@ -911,8 +909,7 @@ class Admin_UserController extends AuthenticatedController
             $domains = UserDomain::getUserDomainsForUser(User::findCurrent()->id);
         }
 
-        $query
-                   = "SELECT Institut_id, Name
+        $query = "SELECT Institut_id, Name
                   FROM Institute
                   WHERE fakultaets_id = ? AND institut_id != fakultaets_id
                   ORDER BY Name";
diff --git a/app/controllers/course/messenger.php b/app/controllers/course/messenger.php
index 8acd24b135e..ca0ec3cdada 100644
--- a/app/controllers/course/messenger.php
+++ b/app/controllers/course/messenger.php
@@ -12,7 +12,9 @@ class Course_MessengerController extends AuthenticatedController
 
     public function course_action($thread_id = null)
     {
-        PageLayout::setTitle(Context::get()->getFullname() . ' - ' . _('Blubber'));
+        if (Context::get()) {
+            PageLayout::setTitle(Context::get()->getFullname() . ' - ' . _('Blubber'));
+        }
 
         if (Navigation::hasItem('/course/blubber')) {
             Navigation::activateItem('/course/blubber');
diff --git a/app/controllers/jsupdater.php b/app/controllers/jsupdater.php
index d0cd7219c57..0d657fe66c5 100644
--- a/app/controllers/jsupdater.php
+++ b/app/controllers/jsupdater.php
@@ -262,6 +262,7 @@ class JsupdaterController extends AuthenticatedController
 
     private function getCoursewareClipboardUpdates($pageInfo)
     {
-        return count(\Courseware\Clipboard::findUsersClipboards($GLOBALS["user"])) != $pageInfo['coursewareclipboard']['counter'];
+        $counter = $pageInfo['coursewareclipboard']['counter'] ?? 0;
+        return \Courseware\Clipboard::countByUser_id($GLOBALS['user']->id) != $counter;
     }
 }
diff --git a/app/controllers/profile.php b/app/controllers/profile.php
index 74e0b436c57..a1776172af1 100644
--- a/app/controllers/profile.php
+++ b/app/controllers/profile.php
@@ -28,19 +28,41 @@ class ProfileController extends AuthenticatedController
         URLHelper::addLinkParam('username', Request::username('username'));
         PageLayout::setHelpKeyword('Basis.Homepage');
 
-        $this->user         = User::findCurrent(); // current logged in user
-        $this->perm         = $GLOBALS['perm']; // perms of current logged in user
-        $this->current_user = User::findByUsername(Request::username('username', $this->user->username)); // current selected user
+        // current logged in user
+        $this->user = User::findCurrent();
+        // perms of current logged in user
+        $this->perm = $GLOBALS['perm'];
+        // current selected user
+        $this->current_user = User::findByUsername(Request::username(
+            'username',
+            $this->user ? $this->user->username : null
+        ));
         // get additional informations to selected user
-        $this->profile = new ProfileModel($this->current_user->user_id, $this->user->user_id);
+        $this->profile = new ProfileModel(
+            $this->current_user ? $this->current_user->id : null,
+            $this->user ? $this->user->id : null
+        );
 
         // set the page title depending on user selection
-        if ($this->current_user['user_id'] == $this->user->id && !$this->current_user['locked']) {
+        if (
+            $this->user
+            && $this->current_user->id === $this->user->id
+            && !$this->current_user->locked
+        ) {
             PageLayout::setTitle(_('Mein Profil'));
             UserConfig::get($this->user->id)->store('PROFILE_LAST_VISIT', time());
-        } elseif ($this->current_user['user_id'] && ($this->perm->have_perm('root') || (!$this->current_user['locked'] && get_visibility_by_id($this->current_user['user_id'])))) {
+        } elseif (
+            $this->current_user->id
+            && (
+                $this->perm->have_perm('root')
+                || (
+                    !$this->current_user->locked
+                    && get_visibility_by_id($this->current_user->id)
+                )
+            )
+        ) {
             PageLayout::setTitle(_('Profil von') . ' ' . $this->current_user->getFullname());
-            object_add_view($this->current_user->user_id);
+            object_add_view($this->current_user->id);
         } else {
             PageLayout::setTitle(_('Profil'));
             $action = 'not_available';
diff --git a/app/views/admin/user/new.php b/app/views/admin/user/new.php
index 49fd9ed29a2..8ceb12fe1c8 100644
--- a/app/views/admin/user/new.php
+++ b/app/views/admin/user/new.php
@@ -171,16 +171,16 @@ use Studip\Button, Studip\LinkButton;
                 <option value="" class="is-placeholder">
                     <?= _('-- Bitte Einrichtung auswählen --') ?>
                 </option>
-                <? foreach ($faks as $fak) : ?>
+            <? foreach ($faks as $fak) : ?>
                 <option value="<?= $fak['Institut_id'] ?>" <?= ($user['institute'] == $fak['Institut_id']) ? 'selected' : '' ?> class="<?= $fak['is_fak'] ? 'nested-item-header' : 'nested-item'; ?>">
                     <?= htmlReady($fak['Name']) ?>
                 </option>
-                    <? foreach ($fak['institutes'] as $institute) : ?>
+                <? foreach ($fak['institutes'] ?? [] as $institute) : ?>
                     <option value="<?= $institute['Institut_id'] ?>" <?= ($user['institute'] == $institute['Institut_id']) ? 'selected' : '' ?> class="nested-item">
                         <?= htmlReady($institute['Name']) ?>
                     </option>
-                    <? endforeach ?>
                 <? endforeach ?>
+            <? endforeach ?>
             </select>
         </label>
 
diff --git a/app/views/course/ilias_interface/add_object.php b/app/views/course/ilias_interface/add_object.php
index 356a82dcb16..85503377e00 100644
--- a/app/views/course/ilias_interface/add_object.php
+++ b/app/views/course/ilias_interface/add_object.php
@@ -122,7 +122,7 @@
     </table>
     <? endif ?>
     <footer data-dialog-button>
-    <? if ($ilias->isActive() && $submit_text) : ?>
+    <? if ($ilias && $ilias->isActive() && $submit_text) : ?>
         <?= Studip\Button::create($submit_text, 'submit', ($dialog && $keep_dialog) ? ['data-dialog' => 'size=auto;reload-on-close'] : []) ?>
     <? endif ?>
         <?= Studip\Button::createCancel(_('Schließen'), 'cancel', $dialog ? ['data-dialog' => 'close'] : []) ?>
diff --git a/app/views/my_ilias_accounts/add_object.php b/app/views/my_ilias_accounts/add_object.php
index e90beea1060..21540409d69 100644
--- a/app/views/my_ilias_accounts/add_object.php
+++ b/app/views/my_ilias_accounts/add_object.php
@@ -1,7 +1,7 @@
 <form class="default" action="<?= $controller->url_for('my_ilias_accounts/redirect/'.$ilias_index.'/new/'.$ilias_ref_id) ?>" method="post" target="_blank">
     <?= CSRFProtection::tokenTag() ?>
-	<input type="hidden" name="ilias_target" value="new">
-	<input type="hidden" name="ilias_ref_id" value="<?=$ilias_ref_id?>">
+    <input type="hidden" name="ilias_target" value="new">
+    <input type="hidden" name="ilias_ref_id" value="<?=$ilias_ref_id?>">
     <label>
         <span class="required"><?= _('Art des Lernobjekts') ?></span>
         <select name="ilias_module_type" required>
diff --git a/lib/classes/ProfileModel.php b/lib/classes/ProfileModel.php
index 663d8ab3ff2..6827ba62a4f 100644
--- a/lib/classes/ProfileModel.php
+++ b/lib/classes/ProfileModel.php
@@ -54,7 +54,10 @@ class ProfileModel
      */
     public function getHomepageVisibilities()
     {
-        $visibilities = get_local_visibility_by_id($this->current_user->user_id, 'homepage');
+        $visibilities = get_local_visibility_by_id(
+            $this->current_user ? $this->current_user->id : null,
+            'homepage'
+        );
         if (is_array(json_decode($visibilities, true))) {
             return json_decode($visibilities, true);
         }
diff --git a/lib/functions.php b/lib/functions.php
index 144b6f580ff..20066c1cc3b 100644
--- a/lib/functions.php
+++ b/lib/functions.php
@@ -302,21 +302,22 @@ function my_substr($what, $start, $end)
  */
 function get_fullname($user_id = "", $format = "full" , $htmlready = false)
 {
-    static $cache;
-    global $user, $_fullname_sql;
+    static $cache = [];
+
+    $current_user = User::findCurrent();
 
     if (!$user_id) {
-        $user_id = $user->id;
+        $user_id = $current_user ? $current_user->id : null;
     }
 
-    if (User::findCurrent()->id === $user_id) {
-        $fullname = User::findCurrent()->getFullName($format);
+    if ($current_user && $current_user->id === $user_id) {
+        $fullname = $current_user->getFullName($format);
         return $htmlready ? htmlReady($fullname) : $fullname;
     }
 
     $hash = md5($user_id . $format);
     if (!isset($cache[$hash])) {
-        $query = "SELECT {$_fullname_sql[$format]}
+        $query = "SELECT {$GLOBALS['_fullname_sql'][$format]}
                   FROM auth_user_md5
                   LEFT JOIN user_info USING (user_id)
                   WHERE user_id = ?";
diff --git a/lib/models/UserDomain.php b/lib/models/UserDomain.php
index 8179082a031..49abbbc75ba 100644
--- a/lib/models/UserDomain.php
+++ b/lib/models/UserDomain.php
@@ -107,10 +107,14 @@ class UserDomain extends SimpleORMap
      * Get an array of all user domains for a specific user.
      * Returns an array of UserDomain objects.
      */
-    public static function getUserDomainsForUser ($user_id)
+    public static function getUserDomainsForUser($user_id)
     {
-        $domains = User::find($user_id)->domains;
-        return $domains ? $domains->getArrayCopy() : [];
+        $user = User::find($user_id);
+        if (!$user) {
+            return [];
+        }
+
+        return $user->domains ? $user->domains->getArrayCopy() : [];
     }
 
     /**
diff --git a/lib/user_visible.inc.php b/lib/user_visible.inc.php
index c6b56860cd6..fbb64c73820 100644
--- a/lib/user_visible.inc.php
+++ b/lib/user_visible.inc.php
@@ -82,14 +82,15 @@ function get_visibility_by_state ($state, $user_id) {
         return true;
     }
 
+    $user = User::findCurrent();
     $other_user = User::find($user_id);
-    if (!$other_user) {
+    if (!$user || !$other_user) {
         return false;
     }
 
     $same_domain = UserDomain::checkUserVisibility(
         $other_user->domains,
-        User::findCurrent()->domains
+        $user->domains
     );
 
     switch ($state) {
-- 
GitLab