home: Remove now-unnecessary page_params_core_fields duplication.

Also, we update the documentation to make the overall system a bit
clearer.

Fixes #4628.
This commit is contained in:
Tim Abbott
2017-05-13 22:49:35 -07:00
parent 157e0623f8
commit 97abaae9af
4 changed files with 25 additions and 105 deletions

View File

@@ -236,6 +236,9 @@ checking that all the calls to `send_event` are covered. Someday
we'll add automation that verifies this directly by inspecting the we'll add automation that verifies this directly by inspecting the
coverage data. coverage data.
In the Zulip webapp, the data returned by the `register` API is
available via the `page_params` parameter.
### Messages ### Messages
One exception to the protocol described in the last section is the One exception to the protocol described in the last section is the

View File

@@ -191,8 +191,8 @@ initial state. Subsequently, clients subscribe to "events," which can
(among other things) indicate that settings have changed. For the (among other things) indicate that settings have changed. For the
backend piece, we will need our action to make a call to `send_event` backend piece, we will need our action to make a call to `send_event`
to send the event to clients that are active. We will also need to to send the event to clients that are active. We will also need to
modify `fetch_initial_state_data` so that future clients see the new modify `fetch_initial_state_data` so that the new field is passed to
changes. See [our event system docs](events-system.html) for all the clients. See [our event system docs](events-system.html) for all the
gory details. gory details.
Anyway, getting back to implementation details... Anyway, getting back to implementation details...
@@ -258,9 +258,14 @@ to explicitly update this field and send an event.
### Update application state ### Update application state
You then need to add code that will handle the event and update the You then need to add code to ensure that your new setting is included
application state. The `fetch_initial_state` and `apply_event` in the data sent down to clients, both when a new client is loaded,
functions in `zerver/lib/events.py` do this. and when changes happen. The `fetch_initial_state_data` function is
responsible for the former (data added to the `state` here will be
available both in `page_params` in the browser, as well as to API
clients like the mobile apps). The `apply_event` function in
`zerver/lib/events.py` is important for making sure the `state` is
always correct, even in the event of rare race conditions.
# zerver/lib/events.py # zerver/lib/events.py
@@ -283,11 +288,12 @@ functions in `zerver/lib/events.py` do this.
If you are adding a realm property that fits the `property_types` If you are adding a realm property that fits the `property_types`
framework, you don't need to change `fetch_initial_state_data` or framework, you don't need to change `fetch_initial_state_data` or
`apply_event` because there is already code to get the initial data and `apply_event` because there is already code to get the initial data
the realm update event type. However, if you are adding a property and handle the realm update event type. However, if you are adding a
that is handled separately, you will need to explicitly add the property property that is handled separately, you will need to explicitly add
to the `state` dictionary in the `fetch_initial_state_data` function. the property to the `state` dictionary in the
Ex, for `authentication_methods`: `fetch_initial_state_data` function. E.g., for
`authentication_methods`:
def fetch_initial_state_data(user_profile, event_types, queue_id, include_subscribers=True): def fetch_initial_state_data(user_profile, event_types, queue_id, include_subscribers=True):
# ... # ...
@@ -298,8 +304,8 @@ Ex, for `authentication_methods`:
For this setting, one won't need to change `apply_event` since its For this setting, one won't need to change `apply_event` since its
default code for `realm` event types handles this case correctly, but default code for `realm` event types handles this case correctly, but
for a totally new type of feature, usually a few lines in that for a totally new type of feature, a few lines in that function may be
function are needed. needed.
### Add a new view ### Add a new view
@@ -311,19 +317,6 @@ already a view that does this for related flags: `update_realm` in
`zerver/views/realm.py`. So in this case, we can add our code to the `zerver/views/realm.py`. So in this case, we can add our code to the
existing view instead of creating a new one. existing view instead of creating a new one.
First, add the new feature to the `page_params_core_fields` list
in `zerver/views/home.py`.
def home_real(request):
# ...
page_params_core_fields = [
# ...
'realm_icon_url',
'realm_invite_by_admins_only',
'realm_inline_image_preview',
# ...
)
Since this feature adds a checkbox to the admin page and a new property Since this feature adds a checkbox to the admin page and a new property
to the Realm model that can be modified from there, you need to add a to the Realm model that can be modified from there, you need to add a
parameter for the new field to the `update_realm` function in parameter for the new field to the `update_realm` function in

View File

@@ -48,6 +48,7 @@ class HomeTest(ZulipTestCase):
"avatar_url_medium", "avatar_url_medium",
"can_create_streams", "can_create_streams",
"cross_realm_bots", "cross_realm_bots",
"custom_profile_fields",
"debug_mode", "debug_mode",
"default_desktop_notifications", "default_desktop_notifications",
"default_language", "default_language",

View File

@@ -220,87 +220,10 @@ def home_real(request):
has_mobile_devices = num_push_devices_for_user(user_profile) > 0, has_mobile_devices = num_push_devices_for_user(user_profile) > 0,
) )
# These fields will be automatically copied from register_ret into undesired_register_ret_fields = [
# page_params. It is a goal to move more of the page_params list 'streams',
# into this sort of cleaner structure.
page_params_core_fields = [
'alert_words',
'attachments',
'autoscroll_forever',
'avatar_source',
'avatar_url',
'avatar_url_medium',
'can_create_streams',
'default_desktop_notifications',
'default_language',
'email',
'emoji_alt_code',
'emojiset',
'emojiset_choices',
'enable_desktop_notifications',
'enable_digest_emails',
'enable_offline_email_notifications',
'enable_offline_push_notifications',
'enable_online_push_notifications',
'enable_sounds',
'enable_stream_desktop_notifications',
'enable_stream_sounds',
'enter_sends',
'full_name',
'hotspots',
'is_admin',
'last_event_id',
'left_side_userlist',
'max_icon_file_size',
'max_message_id',
'muted_topics',
'never_subscribed',
'pm_content_in_desktop_notifications',
'pointer',
'presences',
'queue_id',
'realm_add_emoji_by_admins_only',
'realm_allow_message_editing',
'realm_authentication_methods',
'realm_bot_domain',
'realm_bots',
'realm_create_stream_by_admins_only',
'realm_default_language',
'realm_default_streams',
'realm_domains',
'realm_email_changes_disabled',
'realm_emoji',
'realm_filters',
'realm_icon_source',
'realm_icon_url',
'realm_invite_by_admins_only',
'realm_inline_image_preview',
'realm_inline_url_embed_preview',
'realm_invite_required',
'realm_is_zephyr_mirror_realm',
'realm_mandatory_topics',
'realm_message_content_edit_limit_seconds',
'realm_message_retention_days',
'realm_name',
'realm_description',
'realm_name_changes_disabled',
'realm_password_auth_enabled',
'realm_presence_disabled',
'realm_restricted_to_domain',
'realm_show_digest_email',
'realm_uri',
'realm_users',
'realm_waiting_period_threshold',
'referrals',
'subscriptions',
'unsubscribed',
'timezone',
'twenty_four_hour_time',
'user_id',
'zulip_version',
] ]
for field_name in set(register_ret.keys()) - set(undesired_register_ret_fields):
for field_name in page_params_core_fields:
page_params[field_name] = register_ret[field_name] page_params[field_name] = register_ret[field_name]
if narrow_stream is not None: if narrow_stream is not None: