From 16123c9a58b9929161d7cd14d134e2fd3fa0e402 Mon Sep 17 00:00:00 2001
From: Tim Abbott <tabbott@zulipchat.com>
Date: Sat, 2 Mar 2019 09:23:57 -0800
Subject: [PATCH] realm_logo: Fix synchronization of realm night logo.

The night logo synchronization on the settings page was perfect, but
the actual display logic had a few problems:

* We were including the realm_logo in context_processors, even though
  it is only used in home.py.
* We used different variable names for the templating in navbar.html
  than anywhere else the codebase.

* The behavior that the night logo would default to the day logo if
  only one was uploaded was not correctly implemented for the navbar
  position, either in the synchronization for updates code or the
  logic in the navbar.html templates.
---
 static/js/realm_logo.js            |  2 +-
 templates/zerver/app/navbar.html   |  2 +-
 zerver/context_processors.py       |  7 ----
 zerver/lib/events.py               | 13 +++++---
 zerver/tests/test_auth_backends.py |  2 --
 zerver/tests/test_home.py          | 52 +++++++++++++++++++++++++++++-
 zerver/views/auth.py               |  2 --
 zerver/views/home.py               | 13 +++++++-
 8 files changed, 73 insertions(+), 20 deletions(-)

diff --git a/static/js/realm_logo.js b/static/js/realm_logo.js
index f109c43506..6ee02852e3 100644
--- a/static/js/realm_logo.js
+++ b/static/js/realm_logo.js
@@ -75,7 +75,7 @@ var realm_logo = (function () {
             $("#realm-settings-night-logo").attr("src", page_params.realm_night_logo_url);
         }
 
-        if (page_params.night_mode) {
+        if (page_params.night_mode && page_params.realm_night_logo_source !== 'D') {
             $("#realm-logo").attr("src", page_params.realm_night_logo_url);
         } else {
             $("#realm-logo").attr("src", page_params.realm_logo_url);
diff --git a/templates/zerver/app/navbar.html b/templates/zerver/app/navbar.html
index 7f11a9da73..0b6ac827c4 100644
--- a/templates/zerver/app/navbar.html
+++ b/templates/zerver/app/navbar.html
@@ -31,7 +31,7 @@
     <nav class="header-main rightside-userlist" id="top_navbar">
         <div class="column-left">
             <a class="brand no-style" href="#">
-                <img id="realm-logo" src="{% if night_mode %} {{ realm_night_logo }}{% else %} {{ realm_logo }} {% endif %}" alt="" class="nav-logo no-drag"/>
+                <img id="realm-logo" src="{{ navbar_logo_url }}" alt="" class="nav-logo no-drag"/>
             </a>
         </div>
         <div class="column-middle" id="navbar-middle">
diff --git a/zerver/context_processors.py b/zerver/context_processors.py
index 477942fd17..9bf3a55bd4 100644
--- a/zerver/context_processors.py
+++ b/zerver/context_processors.py
@@ -15,7 +15,6 @@ from zerver.lib.bugdown import convert as bugdown_convert
 from zerver.lib.send_email import FromAddress
 from zerver.lib.subdomains import get_subdomain
 from zerver.lib.realm_icon import get_realm_icon_url
-from zerver.lib.realm_logo import get_realm_logo_url
 
 from version import ZULIP_VERSION, LATEST_RELEASE_VERSION, \
     LATEST_RELEASE_ANNOUNCEMENT, LATEST_MAJOR_VERSION
@@ -54,8 +53,6 @@ def zulip_default_context(request: HttpRequest) -> Dict[str, Any]:
         realm_uri = settings.ROOT_DOMAIN_URI
         realm_name = None
         realm_icon = None
-        realm_logo = None
-        realm_night_logo = None
         realm_description = None
         realm_invite_required = False
         realm_plan_type = 0
@@ -63,8 +60,6 @@ def zulip_default_context(request: HttpRequest) -> Dict[str, Any]:
         realm_uri = realm.uri
         realm_name = realm.name
         realm_icon = get_realm_icon_url(realm)
-        realm_logo = get_realm_logo_url(realm, night = False)
-        realm_night_logo = get_realm_logo_url(realm, night = True)
         realm_description_raw = realm.description or "The coolest place in the universe."
         realm_description = bugdown_convert(realm_description_raw, message_realm=realm)
         realm_invite_required = realm.invite_required
@@ -119,8 +114,6 @@ def zulip_default_context(request: HttpRequest) -> Dict[str, Any]:
         'realm_uri': realm_uri,
         'realm_name': realm_name,
         'realm_icon': realm_icon,
-        'realm_logo': realm_logo,
-        'realm_night_logo': realm_night_logo,
         'realm_description': realm_description,
         'realm_plan_type': realm_plan_type,
         'root_domain_uri': settings.ROOT_DOMAIN_URI,
diff --git a/zerver/lib/events.py b/zerver/lib/events.py
index edb64f1d98..54e57488a1 100644
--- a/zerver/lib/events.py
+++ b/zerver/lib/events.py
@@ -107,6 +107,13 @@ def get_raw_user_data(realm_id: int, client_gravatar: bool) -> Dict[int, Dict[st
         for row in user_dicts
     }
 
+def add_realm_logo_fields(state: Dict[str, Any], realm: Realm) -> None:
+    state['realm_logo_url'] = realm_logo_url(realm, night = False)
+    state['realm_logo_source'] = realm.logo_source
+    state['realm_night_logo_url'] = realm_logo_url(realm, night = True)
+    state['realm_night_logo_source'] = realm.night_logo_source
+    state['max_logo_file_size'] = settings.MAX_LOGO_FILE_SIZE
+
 def always_want(msg_type: str) -> bool:
     '''
     This function is used as a helper in
@@ -184,11 +191,7 @@ def fetch_initial_state_data(user_profile: UserProfile,
         state['realm_icon_url'] = realm_icon_url(realm)
         state['realm_icon_source'] = realm.icon_source
         state['max_icon_file_size'] = settings.MAX_ICON_FILE_SIZE
-        state['realm_logo_url'] = realm_logo_url(realm, night = False)
-        state['realm_logo_source'] = realm.logo_source
-        state['realm_night_logo_url'] = realm_logo_url(realm, night = True)
-        state['realm_night_logo_source'] = realm.night_logo_source
-        state['max_logo_file_size'] = settings.MAX_LOGO_FILE_SIZE
+        add_realm_logo_fields(state, realm)
         state['realm_bot_domain'] = realm.get_bot_domain()
         state['realm_uri'] = realm.uri
         state['realm_available_video_chat_providers'] = realm.VIDEO_CHAT_PROVIDERS
diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py
index f7e0b2dd4e..c50e41339d 100644
--- a/zerver/tests/test_auth_backends.py
+++ b/zerver/tests/test_auth_backends.py
@@ -1718,8 +1718,6 @@ class FetchAuthBackends(ZulipTestCase):
             ('realm_name', check_string),
             ('realm_description', check_string),
             ('realm_icon', check_string),
-            ('realm_logo', check_string),
-            ('realm_night_logo', check_string),
         ])
 
     def test_fetch_auth_backend_format(self) -> None:
diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py
index e2c7d4e395..8725e8d6d9 100644
--- a/zerver/tests/test_home.py
+++ b/zerver/tests/test_home.py
@@ -10,6 +10,8 @@ import urllib
 from typing import Any, Dict
 
 from zerver.lib.actions import do_create_user
+from zerver.lib.actions import do_change_logo_source
+from zerver.lib.events import add_realm_logo_fields
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import (
     HostRequestMock, queries_captured, get_user_messages
@@ -20,7 +22,7 @@ from zerver.models import (
     get_realm, get_stream, get_user, UserProfile,
     flush_per_request_caches, DefaultStream, Realm,
 )
-from zerver.views.home import home, sent_time_in_epoch_seconds
+from zerver.views.home import home, sent_time_in_epoch_seconds, compute_navbar_logo_url
 from corporate.models import Customer, CustomerPlan
 
 class HomeTest(ZulipTestCase):
@@ -722,6 +724,54 @@ class HomeTest(ZulipTestCase):
         html = result.content.decode('utf-8')
         self.assertIn('Apps for every platform.', html)
 
+    def test_compute_navbar_logo_url(self) -> None:
+        user_profile = self.example_user("hamlet")
+
+        page_params = {"night_mode": True}
+        add_realm_logo_fields(page_params, user_profile.realm)
+        self.assertEqual(compute_navbar_logo_url(page_params),
+                         "/static/images/logo/zulip-org-logo.png?version=0")
+
+        page_params = {"night_mode": False}
+        add_realm_logo_fields(page_params, user_profile.realm)
+        self.assertEqual(compute_navbar_logo_url(page_params),
+                         "/static/images/logo/zulip-org-logo.png?version=0")
+
+        do_change_logo_source(user_profile.realm, Realm.LOGO_UPLOADED, night=False)
+        page_params = {"night_mode": True}
+        add_realm_logo_fields(page_params, user_profile.realm)
+        self.assertEqual(compute_navbar_logo_url(page_params),
+                         "/user_avatars/1/realm/logo.png?version=2")
+
+        page_params = {"night_mode": False}
+        add_realm_logo_fields(page_params, user_profile.realm)
+        self.assertEqual(compute_navbar_logo_url(page_params),
+                         "/user_avatars/1/realm/logo.png?version=2")
+
+        do_change_logo_source(user_profile.realm, Realm.LOGO_UPLOADED, night=True)
+        page_params = {"night_mode": True}
+        add_realm_logo_fields(page_params, user_profile.realm)
+        self.assertEqual(compute_navbar_logo_url(page_params),
+                         "/user_avatars/1/realm/night_logo.png?version=2")
+
+        page_params = {"night_mode": False}
+        add_realm_logo_fields(page_params, user_profile.realm)
+        self.assertEqual(compute_navbar_logo_url(page_params),
+                         "/user_avatars/1/realm/logo.png?version=2")
+
+        # This configuration isn't super supported in the UI and is a
+        # weird choice, but we have a test for it anyway.
+        do_change_logo_source(user_profile.realm, Realm.LOGO_DEFAULT, night=False)
+        page_params = {"night_mode": True}
+        add_realm_logo_fields(page_params, user_profile.realm)
+        self.assertEqual(compute_navbar_logo_url(page_params),
+                         "/user_avatars/1/realm/night_logo.png?version=2")
+
+        page_params = {"night_mode": False}
+        add_realm_logo_fields(page_params, user_profile.realm)
+        self.assertEqual(compute_navbar_logo_url(page_params),
+                         "/static/images/logo/zulip-org-logo.png?version=0")
+
     def test_generate_204(self) -> None:
         email = self.example_email("hamlet")
         self.login(email)
diff --git a/zerver/views/auth.py b/zerver/views/auth.py
index 9175f9012a..949c2cc5d0 100644
--- a/zerver/views/auth.py
+++ b/zerver/views/auth.py
@@ -871,8 +871,6 @@ def api_get_server_settings(request: HttpRequest) -> HttpResponse:
             "realm_uri",
             "realm_name",
             "realm_icon",
-            "realm_logo",
-            "realm_night_logo",
             "realm_description"]:
         if context[settings_item] is not None:
             result[settings_item] = context[settings_item]
diff --git a/zerver/views/home.py b/zerver/views/home.py
index 62a6115234..c720910f31 100644
--- a/zerver/views/home.py
+++ b/zerver/views/home.py
@@ -1,4 +1,4 @@
-from typing import List, Dict, Optional
+from typing import Any, List, Dict, Optional
 
 from django.conf import settings
 from django.urls import reverse
@@ -73,6 +73,13 @@ def get_bot_types(user_profile: UserProfile) -> List[Dict[str, object]]:
         })
     return bot_types
 
+def compute_navbar_logo_url(page_params: Dict[str, Any]) -> str:
+    if page_params["night_mode"] and page_params["realm_night_logo_source"] != Realm.LOGO_DEFAULT:
+        navbar_logo_url = page_params["realm_night_logo_url"]
+    else:
+        navbar_logo_url = page_params["realm_logo_url"]
+    return navbar_logo_url
+
 def home(request: HttpRequest) -> HttpResponse:
     if (settings.DEVELOPMENT and not settings.TEST_SUITE and
             os.path.exists('var/handlebars-templates/compile.error')):
@@ -270,6 +277,9 @@ def home_real(request: HttpRequest) -> HttpResponse:
         # include (and thus how to display emojis in the emoji picker
         # and composebox typeahead).
         emojiset = UserProfile.GOOGLE_BLOB_EMOJISET
+
+    navbar_logo_url = compute_navbar_logo_url(page_params)
+
     response = render(request, 'zerver/app/index.html',
                       context={'user_profile': user_profile,
                                'emojiset': emojiset,
@@ -286,6 +296,7 @@ def home_real(request: HttpRequest) -> HttpResponse:
                                'is_admin': user_profile.is_realm_admin,
                                'is_guest': user_profile.is_guest,
                                'night_mode': user_profile.night_mode,
+                               'navbar_logo_url': navbar_logo_url,
                                'show_webathena': user_profile.realm.webathena_enabled,
                                'enable_feedback': settings.ENABLE_FEEDBACK,
                                'embedded': narrow_stream is not None,