mirror of https://github.com/zulip/zulip.git
home: Don't send /register response for spectators.
In very large communities, computing page_params can be quite expensive. Because we've moved the homepage for communities with web public streams enabled to be the Zulip app, and it's common for automation to frequently poll the homepage of a Zulip organization, we'd like to keep those homepages cheap (as the login pages are). We address this by prototyping something we may end up wanting to do anyway -- having the web application do a `POST /register` API call in order to fetch most page_params, and merging those with the mostly webapp configuration page_params that we leave in the / response for convenience. This exact implementation is messy in a few ways: * We rely on the assumption that ui_init.initialize_everything happens before all code that needs to inspect the page_params properties we are fetching via /register. This is likely mostly true, but nothing in the implementation enforces it. * The bundle of ~25 keys that are in page_params ideally would be considered individually, with some moved to the /register API response and perhaps others eliminated or namespaced inside a webapp_settings object. * It's weird to have the spectators network sequence different that from logged-in users, and potentially a maintainability risk. * We might be able to arrange that the initial `/` response be cacheable, now that we're no longer embedding our metadata inside it. We've made no effort to do that as of yet. Despite those issues, this commit solves an immediate problem and will give us helpful experience with a model closer to the one we'll want in order to happily support a web client that can be run locally against a production Zulip server's data. Co-authored-by: Anders Kaseorg <anders@zulip.com>
This commit is contained in:
Tim Abbott
parent
44fcb6009b
commit
24f527cb59
|
|
@ -93,6 +93,7 @@
|
|||
"no-undef-init": "error",
|
||||
"no-unneeded-ternary": ["error", {"defaultAssignment": false}],
|
||||
"no-unused-expressions": "error",
|
||||
"no-unused-vars": ["error", {"ignoreRestSiblings": true}],
|
||||
"no-use-before-define": ["error", {"functions": false}],
|
||||
"no-useless-concat": "error",
|
||||
"no-useless-constructor": "error",
|
||||
|
|
@ -188,7 +189,7 @@
|
|||
"@typescript-eslint/no-non-null-assertion": "off",
|
||||
"@typescript-eslint/no-parameter-properties": "error",
|
||||
"@typescript-eslint/no-unnecessary-qualifier": "error",
|
||||
"@typescript-eslint/no-unused-vars": ["error", {"varsIgnorePattern": "^_"}],
|
||||
"@typescript-eslint/no-unused-vars": ["error", {"ignoreRestSiblings": true}],
|
||||
"@typescript-eslint/no-unsafe-argument": "off",
|
||||
"@typescript-eslint/no-unsafe-assignment": "off",
|
||||
"@typescript-eslint/no-unsafe-call": "off",
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import * as activity from "./activity";
|
|||
import * as alert_words from "./alert_words";
|
||||
import * as blueslip from "./blueslip";
|
||||
import * as bot_data from "./bot_data";
|
||||
import * as channel from "./channel";
|
||||
import * as click_handlers from "./click_handlers";
|
||||
import * as common from "./common";
|
||||
import * as compose from "./compose";
|
||||
|
|
@ -683,7 +684,25 @@ export function initialize_everything() {
|
|||
$("#app-loading").addClass("loaded");
|
||||
}
|
||||
|
||||
$(() => {
|
||||
$(async () => {
|
||||
if (page_params.is_spectator) {
|
||||
const data = {
|
||||
apply_markdown: true,
|
||||
client_capabilities: JSON.stringify({
|
||||
notification_settings_null: true,
|
||||
bulk_message_deletion: true,
|
||||
user_avatar_url_field_optional: true,
|
||||
// Set this to true when stream typing notifications are implemented.
|
||||
stream_typing_notifications: false,
|
||||
user_settings_object: true,
|
||||
}),
|
||||
client_gravatar: false,
|
||||
};
|
||||
const {result, msg, ...state} = await new Promise((success, error) => {
|
||||
channel.post({url: "/json/register", data, success, error});
|
||||
});
|
||||
Object.assign(page_params, state);
|
||||
}
|
||||
blueslip.measure_time("initialize_everything", () => {
|
||||
initialize_everything();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -153,32 +153,21 @@ def build_page_params_for_home_page_load(
|
|||
narrow=narrow,
|
||||
include_streams=False,
|
||||
)
|
||||
default_language = register_ret["user_settings"]["default_language"]
|
||||
else:
|
||||
# Since events for spectator is not implemented, we only fetch the data
|
||||
# at the time of request and don't register for any events.
|
||||
# TODO: Implement events for spectator.
|
||||
from zerver.lib.events import fetch_initial_state_data, post_process_state
|
||||
|
||||
register_ret = fetch_initial_state_data(
|
||||
user_profile,
|
||||
realm=realm,
|
||||
event_types=None,
|
||||
queue_id=None,
|
||||
client_gravatar=False,
|
||||
user_avatar_url_field_optional=client_capabilities["user_avatar_url_field_optional"],
|
||||
user_settings_object=client_capabilities["user_settings_object"],
|
||||
slim_presence=False,
|
||||
include_subscribers=False,
|
||||
include_streams=False,
|
||||
)
|
||||
|
||||
post_process_state(user_profile, register_ret, False)
|
||||
# The spectator client will be fetching the /register response
|
||||
# for spectators via the API. But we still need to set the
|
||||
# values not presence in that object.
|
||||
register_ret = {
|
||||
"queue_id": None,
|
||||
}
|
||||
default_language = realm.default_language
|
||||
|
||||
furthest_read_time = get_furthest_read_time(user_profile)
|
||||
|
||||
request_language = get_and_set_request_language(
|
||||
request,
|
||||
register_ret["user_settings"]["default_language"],
|
||||
default_language,
|
||||
translation.get_language_from_path(request.path_info),
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -329,38 +329,36 @@ class HomeTest(ZulipTestCase):
|
|||
page_params = self._get_page_params(result)
|
||||
self.assertEqual(page_params["is_spectator"], True)
|
||||
actual_keys = sorted(str(k) for k in page_params.keys())
|
||||
removed_keys = [
|
||||
"custom_profile_field_types",
|
||||
"custom_profile_fields",
|
||||
"last_event_id",
|
||||
"narrow",
|
||||
"narrow_stream",
|
||||
expected_keys = [
|
||||
"apps_page_url",
|
||||
"bot_types",
|
||||
"corporate_enabled",
|
||||
"development_environment",
|
||||
"first_in_realm",
|
||||
"furthest_read_time",
|
||||
"insecure_desktop_app",
|
||||
"is_spectator",
|
||||
"language_list",
|
||||
"login_page",
|
||||
"needs_tutorial",
|
||||
"no_event_queue",
|
||||
"promote_sponsoring_zulip",
|
||||
"prompt_for_invites",
|
||||
"queue_id",
|
||||
"realm_rendered_description",
|
||||
"request_language",
|
||||
"search_pills_enabled",
|
||||
"show_billing",
|
||||
"show_plans",
|
||||
"show_webathena",
|
||||
"test_suite",
|
||||
"translation_data",
|
||||
"two_fa_enabled",
|
||||
"two_fa_enabled_user",
|
||||
"warn_no_email",
|
||||
"webpack_public_path",
|
||||
]
|
||||
expected_keys = set(self.expected_page_params_keys + ["realm_rendered_description"]) - set(
|
||||
removed_keys
|
||||
)
|
||||
self.assertEqual(set(actual_keys), set(expected_keys))
|
||||
|
||||
# Test information passed to client about users.
|
||||
page_params = self._get_page_params(result)
|
||||
self.assertEqual(
|
||||
sorted(page_params["realm_users"][0].keys()),
|
||||
[
|
||||
"avatar_url",
|
||||
"avatar_version",
|
||||
"date_joined",
|
||||
"email",
|
||||
"full_name",
|
||||
"is_admin",
|
||||
"is_bot",
|
||||
"is_guest",
|
||||
"is_owner",
|
||||
"role",
|
||||
"user_id",
|
||||
],
|
||||
)
|
||||
date_length = len("YYYY-MM-DD")
|
||||
self.assert_length(page_params["realm_users"][0]["date_joined"], date_length)
|
||||
self.assertEqual(actual_keys, expected_keys)
|
||||
|
||||
def test_home_under_2fa_without_otp_device(self) -> None:
|
||||
with self.settings(TWO_FACTOR_AUTHENTICATION_ENABLED=True):
|
||||
|
|
|
|||
Loading…
Reference in New Issue