From 65ddf3fc55717b1cefa053ff13f8074cf4182263 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 14 Apr 2022 15:20:07 -0700 Subject: [PATCH 01/16] api.updateStream [nfc]: Accept multiple properties at once --- src/api/streams/updateStream.js | 8 ++------ src/streams/streamsActions.js | 6 +++--- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/api/streams/updateStream.js b/src/api/streams/updateStream.js index 47d2a6bb66e..608472f6762 100644 --- a/src/api/streams/updateStream.js +++ b/src/api/streams/updateStream.js @@ -13,9 +13,5 @@ import { apiPatch } from '../apiFetch'; export default ( auth: Auth, id: number, - property: string, - value: string | boolean, -): Promise => - apiPatch(auth, `streams/${id}`, { - [property]: value, - }); + params: $ReadOnly<{| description?: string, new_name?: string, is_private?: boolean |}>, +): Promise => apiPatch(auth, `streams/${id}`, params); diff --git a/src/streams/streamsActions.js b/src/streams/streamsActions.js index b6b4414362f..0aa3316ef21 100644 --- a/src/streams/streamsActions.js +++ b/src/streams/streamsActions.js @@ -20,12 +20,12 @@ export const updateExistingStream = ( const auth = getAuth(state); if (initialValues.name !== newValues.name) { - await api.updateStream(auth, id, 'new_name', maybeEncode(newValues.name)); + await api.updateStream(auth, id, { new_name: maybeEncode(newValues.name) }); } if (initialValues.description !== newValues.description) { - await api.updateStream(auth, id, 'description', maybeEncode(newValues.description)); + await api.updateStream(auth, id, { description: maybeEncode(newValues.description) }); } if (initialValues.invite_only !== newValues.isPrivate) { - await api.updateStream(auth, id, 'is_private', newValues.isPrivate); + await api.updateStream(auth, id, { is_private: newValues.isPrivate }); } }; From be12766bc2451afda2ff99b26b219b617c9055be Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 14 Apr 2022 15:43:06 -0700 Subject: [PATCH 02/16] streamsActions: Send one update-stream request for all changed properties Instead of up to three in sequence, one per changed property. --- src/streams/streamsActions.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/streams/streamsActions.js b/src/streams/streamsActions.js index 0aa3316ef21..328318a2760 100644 --- a/src/streams/streamsActions.js +++ b/src/streams/streamsActions.js @@ -19,13 +19,18 @@ export const updateExistingStream = ( getZulipFeatureLevel(state) >= 64 ? value : JSON.stringify(value); const auth = getAuth(state); + const updates = {}; if (initialValues.name !== newValues.name) { - await api.updateStream(auth, id, { new_name: maybeEncode(newValues.name) }); + updates.new_name = maybeEncode(newValues.name); } if (initialValues.description !== newValues.description) { - await api.updateStream(auth, id, { description: maybeEncode(newValues.description) }); + updates.description = maybeEncode(newValues.description); } if (initialValues.invite_only !== newValues.isPrivate) { - await api.updateStream(auth, id, { is_private: newValues.isPrivate }); + updates.is_private = newValues.isPrivate; + } + + if (Object.keys(updates).length > 0) { + await api.updateStream(auth, id, updates); } }; From 3fc7aa3b5be39a1596721f59da53b1f98ca6bec4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 15 Apr 2022 12:28:05 -0700 Subject: [PATCH 03/16] api types [nfc]: Express updateStream's params by referring to Stream type --- src/api/streams/updateStream.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/api/streams/updateStream.js b/src/api/streams/updateStream.js index 608472f6762..d9c58a4f546 100644 --- a/src/api/streams/updateStream.js +++ b/src/api/streams/updateStream.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import type { ApiResponse, Auth } from '../transportTypes'; +import type { Stream } from '../modelTypes'; import { apiPatch } from '../apiFetch'; /** @@ -13,5 +14,11 @@ import { apiPatch } from '../apiFetch'; export default ( auth: Auth, id: number, - params: $ReadOnly<{| description?: string, new_name?: string, is_private?: boolean |}>, + params: $ReadOnly<{| + description?: $PropertyType, + new_name?: $PropertyType, + + // controls the invite_only property + is_private?: $PropertyType, + |}>, ): Promise => apiPatch(auth, `streams/${id}`, params); From eb5027119c3cf20ca2b18c9c8a356b535c7e473f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 15 Apr 2022 17:05:30 -0700 Subject: [PATCH 04/16] modelTypes [nfc]: Order Stream type's properties to follow a doc --- src/api/modelTypes.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/api/modelTypes.js b/src/api/modelTypes.js index 3549116910d..60cccc62315 100644 --- a/src/api/modelTypes.js +++ b/src/api/modelTypes.js @@ -330,15 +330,19 @@ export type MutedUser = $ReadOnly<{| // export type Stream = {| + // Property ordering follows https://zulip.com/api/register-queue . + +stream_id: number, - +description: string, +name: string, + +description: string, +invite_only: boolean, - +is_announcement_only: boolean, + // TODO(server-2.1): is_web_public was added in Zulip version 2.1; // absence implies the stream is not web-public. +is_web_public?: boolean, + +history_public_to_subscribers: boolean, + +is_announcement_only: boolean, |}; export type Subscription = {| From dd6fea1e2e9d3f3aac5f67b1a3195fdfa3640b6d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 15 Apr 2022 15:06:55 -0700 Subject: [PATCH 05/16] modelTypes: Update Stream type, to match `streams` initial data at FL 121 --- src/__tests__/lib/exampleData.js | 2 ++ src/api/modelTypes.js | 25 ++++++++++++++++++++++++- src/nullObjects.js | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js index 9810063ab70..a32915371fe 100644 --- a/src/__tests__/lib/exampleData.js +++ b/src/__tests__/lib/exampleData.js @@ -300,10 +300,12 @@ export const makeStream = ( stream_id: args.stream_id ?? randStreamId(), name, description: args.description ?? `On the ${randString()} of ${name}`, + rendered_description: args.description ?? `

On the ${randString()} of ${name}

`, invite_only: false, is_announcement_only: false, is_web_public: false, history_public_to_subscribers: true, + first_message_id: null, }); }; diff --git a/src/api/modelTypes.js b/src/api/modelTypes.js index 60cccc62315..97397ecd270 100644 --- a/src/api/modelTypes.js +++ b/src/api/modelTypes.js @@ -330,18 +330,41 @@ export type MutedUser = $ReadOnly<{| // export type Stream = {| - // Property ordering follows https://zulip.com/api/register-queue . + // Based on `streams` in the /register response, current to FL 121: + // https://zulip.com/api/register-queue + // Property ordering follows that doc. +stream_id: number, +name: string, +description: string, + + // TODO(server-4.0): New in FL 30. + +date_created?: number, + + // Note: updateStream controls this with its `is_private` param. +invite_only: boolean, + +rendered_description: string, + // TODO(server-2.1): is_web_public was added in Zulip version 2.1; // absence implies the stream is not web-public. +is_web_public?: boolean, + // TODO(server-3.0): Added in FL 1; if absent, use is_announcement_only. + +stream_post_policy?: 1 | 2 | 3 | 4, + + // Special values are: + // * null: default; inherits from org-level setting + // * -1: unlimited retention + // These special values differ from updateStream's and createStream's params; see + // https://chat.zulip.org/#narrow/stream/412-api-documentation/topic/message_retention_days/near/1367895 + // TODO(server-3.0): New in FL 17. + +message_retention_days?: number | null, + +history_public_to_subscribers: boolean, + +first_message_id: number | null, + + // TODO(server-3.0): Deprecated at FL 1; use stream_post_policy instead +is_announcement_only: boolean, |}; diff --git a/src/nullObjects.js b/src/nullObjects.js index 9fae293dba7..0ac3887ab03 100644 --- a/src/nullObjects.js +++ b/src/nullObjects.js @@ -34,6 +34,7 @@ export const NULL_SUBSCRIPTION: Subscription = { audible_notifications: false, color: 'gray', description: '', + rendered_description: '', desktop_notifications: false, email_address: '', in_home_view: false, @@ -47,4 +48,5 @@ export const NULL_SUBSCRIPTION: Subscription = { is_announcement_only: false, is_web_public: false, history_public_to_subscribers: false, + first_message_id: null, }; From 0c56883ef5afb63dad9dc03992d0c3123f1d21aa Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 15 Apr 2022 17:21:24 -0700 Subject: [PATCH 06/16] api types: Sync updateStream's params with API doc at FL 121 --- src/api/streams/updateStream.js | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/api/streams/updateStream.js b/src/api/streams/updateStream.js index d9c58a4f546..5e1acaaa17b 100644 --- a/src/api/streams/updateStream.js +++ b/src/api/streams/updateStream.js @@ -6,19 +6,42 @@ import { apiPatch } from '../apiFetch'; /** * https://zulip.com/api/update-stream * - * NB the caller must adapt for old-server compatibility; see comment. + * NB the caller must adapt for old-server compatibility; see comments. */ -// TODO(#4659): Once we pass the feature level to API methods, this one -// should encapsulate a switch at feature level 64. See its call sites. -// TODO(server-4.0): Simplify that away. +// Current to FL 121; property ordering follows the doc. export default ( auth: Auth, id: number, params: $ReadOnly<{| + // TODO(#4659): Once we pass the feature level to API methods, this one + // should encapsulate a switch at FL 64 related to these two parameters. + // See call sites. description?: $PropertyType, new_name?: $PropertyType, // controls the invite_only property is_private?: $PropertyType, + + // TODO(server-5.0): New in FL 98. + is_web_public?: $PropertyType, + + // TODO(server-3.0): New in FL 1; for older servers, pass is_announcement_only. + stream_post_policy?: $PropertyType, + + history_public_to_subscribers?: $PropertyType, + + // Doesn't take the same special values as Stream.is_announcement_only! + // https://chat.zulip.org/#narrow/stream/412-api-documentation/topic/message_retention_days/near/1367895 + // TODO(server-3.0): New in FL 17 + message_retention_days?: + | number + | 'realm_default' + // TODO(server-5.0): Added in FL 91, replacing 'forever'. + | 'unlimited' + // TODO(server-5.0): Replaced in FL 91 by 'unlimited'. + | 'forever', + + // TODO(server-3.0): Replaced in FL 1 by 'stream_post_policy'. + is_announcement_only?: $PropertyType, |}>, ): Promise => apiPatch(auth, `streams/${id}`, params); From 2668bdeb74ed3b3905beb31236c25cb61634d09d Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 31 Mar 2022 12:31:25 -0700 Subject: [PATCH 07/16] stream [nfc]: Assimilate to name "invite_only" in create/edit stream logic We'll be adding more stream properties here soon, and it'll be best to avoid multiplying the number of different names. --- src/api/streams/createStream.js | 4 ++-- src/streams/CreateStreamScreen.js | 4 ++-- src/streams/EditStreamCard.js | 18 +++++++++--------- src/streams/EditStreamScreen.js | 4 ++-- src/streams/streamsActions.js | 6 +++--- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/api/streams/createStream.js b/src/api/streams/createStream.js index d8248d15a4e..9df1da78da1 100644 --- a/src/api/streams/createStream.js +++ b/src/api/streams/createStream.js @@ -9,12 +9,12 @@ export default ( description?: string = '', // TODO(server-3.0): Send numeric user IDs (#3764), not emails. principals?: $ReadOnlyArray = [], - inviteOnly?: boolean = false, + invite_only?: boolean = false, announce?: boolean = false, ): Promise => apiPost(auth, 'users/me/subscriptions', { subscriptions: JSON.stringify([{ name, description }]), principals: JSON.stringify(principals), - invite_only: inviteOnly, + invite_only, announce, }); diff --git a/src/streams/CreateStreamScreen.js b/src/streams/CreateStreamScreen.js index 62d8f66cebe..a6d668d6db1 100644 --- a/src/streams/CreateStreamScreen.js +++ b/src/streams/CreateStreamScreen.js @@ -22,8 +22,8 @@ export default function CreateStreamScreen(props: Props): Node { const ownEmail = useSelector(getOwnEmail); const handleComplete = useCallback( - (name: string, description: string, isPrivate: boolean) => { - api.createStream(auth, name, description, [ownEmail], isPrivate); + (name: string, description: string, invite_only: boolean) => { + api.createStream(auth, name, description, [ownEmail], invite_only); NavigationService.dispatch(navigateBack()); }, [auth, ownEmail], diff --git a/src/streams/EditStreamCard.js b/src/streams/EditStreamCard.js index 72f9c437358..857d6ff8f45 100644 --- a/src/streams/EditStreamCard.js +++ b/src/streams/EditStreamCard.js @@ -24,26 +24,26 @@ type Props = $ReadOnly<{| description: string, invite_only: boolean, |}, - onComplete: (name: string, description: string, isPrivate: boolean) => void, + onComplete: (name: string, description: string, invite_only: boolean) => void, |}>; type State = {| name: string, description: string, - isPrivate: boolean, + invite_only: boolean, |}; export default class EditStreamCard extends PureComponent { state: State = { name: this.props.initialValues.name, description: this.props.initialValues.description, - isPrivate: this.props.initialValues.invite_only, + invite_only: this.props.initialValues.invite_only, }; handlePerformAction: () => void = () => { const { onComplete } = this.props; - const { name, description, isPrivate } = this.state; - onComplete(name, description, isPrivate); + const { name, description, invite_only } = this.state; + onComplete(name, description, invite_only); }; handleNameChange: string => void = name => { @@ -54,8 +54,8 @@ export default class EditStreamCard extends PureComponent { this.setState({ description }); }; - handleIsPrivateChange: boolean => void = isPrivate => { - this.setState({ isPrivate }); + handleInviteOnlyChange: boolean => void = invite_only => { + this.setState({ invite_only }); }; render(): Node { @@ -83,8 +83,8 @@ export default class EditStreamCard extends PureComponent { style={componentStyles.switchRow} Icon={IconPrivate} label="Private" - value={this.state.isPrivate} - onValueChange={this.handleIsPrivateChange} + value={this.state.invite_only} + onValueChange={this.handleInviteOnlyChange} /> getStreamForId(state, props.route.params.streamId)); const handleComplete = useCallback( - (name: string, description: string, isPrivate: boolean) => { - dispatch(updateExistingStream(stream.stream_id, stream, { name, description, isPrivate })); + (name: string, description: string, invite_only: boolean) => { + dispatch(updateExistingStream(stream.stream_id, stream, { name, description, invite_only })); NavigationService.dispatch(navigateBack()); }, [stream, dispatch], diff --git a/src/streams/streamsActions.js b/src/streams/streamsActions.js index 328318a2760..aaa63d62372 100644 --- a/src/streams/streamsActions.js +++ b/src/streams/streamsActions.js @@ -6,7 +6,7 @@ import { getAuth, getZulipFeatureLevel } from '../selectors'; export const updateExistingStream = ( id: number, initialValues: Stream, - newValues: {| name: string, description: string, isPrivate: boolean |}, + newValues: {| name: string, description: string, invite_only: boolean |}, ): ThunkAction> => async (dispatch, getState) => { const state = getState(); @@ -26,8 +26,8 @@ export const updateExistingStream = ( if (initialValues.description !== newValues.description) { updates.description = maybeEncode(newValues.description); } - if (initialValues.invite_only !== newValues.isPrivate) { - updates.is_private = newValues.isPrivate; + if (initialValues.invite_only !== newValues.invite_only) { + updates.is_private = newValues.invite_only; } if (Object.keys(updates).length > 0) { From 08c06970d6022f12617b88689077425aef9ee64a Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 11:40:06 -0700 Subject: [PATCH 08/16] api [nfc]: Have createStream use params objects One for stream attributes, and one for miscellaneous options. The create-stream API is a little awkward in that it piggy-backs on the subscribe-to-stream API. Here's the doc: https://zulip.com/api/create-stream > You can create a stream using Zulip's REST API by submitting a > [subscribe](/api/subscribe) request with a stream name that > doesn't yet exist and passing appropriate parameters to define the > initial configuration of the new stream. So, make our code a bit clearer by bundling the attributes of the to-be-created stream into an object. We're about to add more attributes, to sync with the API doc. While we're at it, order the attributes by their appearance in the doc. --- src/api/streams/createStream.js | 29 ++++++++++++++++++++--------- src/streams/CreateStreamScreen.js | 2 +- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/api/streams/createStream.js b/src/api/streams/createStream.js index 9df1da78da1..b0dcc7648d2 100644 --- a/src/api/streams/createStream.js +++ b/src/api/streams/createStream.js @@ -5,16 +5,27 @@ import { apiPost } from '../apiFetch'; /** See https://zulip.com/api/create-stream */ export default ( auth: Auth, - name: string, - description?: string = '', - // TODO(server-3.0): Send numeric user IDs (#3764), not emails. - principals?: $ReadOnlyArray = [], - invite_only?: boolean = false, - announce?: boolean = false, -): Promise => - apiPost(auth, 'users/me/subscriptions', { + streamAttributes: $ReadOnly<{| + // Ordered by their appearance in the doc. + + name: string, + description?: string, + invite_only?: boolean, + |}>, + options?: $ReadOnly<{| + // TODO(server-3.0): Send numeric user IDs (#3764), not emails. + principals?: $ReadOnlyArray, + + announce?: boolean, + |}>, +): Promise => { + const { name, description = '', invite_only = false } = streamAttributes; + const { principals = [], announce = false } = options ?? {}; + return apiPost(auth, 'users/me/subscriptions', { subscriptions: JSON.stringify([{ name, description }]), - principals: JSON.stringify(principals), invite_only, + + principals: JSON.stringify(principals), announce, }); +}; diff --git a/src/streams/CreateStreamScreen.js b/src/streams/CreateStreamScreen.js index a6d668d6db1..43287603906 100644 --- a/src/streams/CreateStreamScreen.js +++ b/src/streams/CreateStreamScreen.js @@ -23,7 +23,7 @@ export default function CreateStreamScreen(props: Props): Node { const handleComplete = useCallback( (name: string, description: string, invite_only: boolean) => { - api.createStream(auth, name, description, [ownEmail], invite_only); + api.createStream(auth, { name, description, invite_only }, { principals: [ownEmail] }); NavigationService.dispatch(navigateBack()); }, [auth, ownEmail], From 94d7a59d2503b30e9a892e9b73807484764b81f7 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 11:47:09 -0700 Subject: [PATCH 09/16] api [nfc]: Remove some param-defaulting within our api.createStream binding I think the only good reason to have such fallback code at this layer would be that we don't trust the server to use reasonable fallbacks itself. There are certainly awkward things about this endpoint -- it's doing double duty as "subscribe-to-stream" and "create-a-stream" -- but I see no reason to distrust the server's defaults for these params, which set attributes on a created stream. Its choices currently match ours, but they could reasonably change, and we'd be fine deferring to the server if they did. NFC because, currently, this binding's only caller also applies the same fallbacks. That might be a bad choice, but at least now that choice isn't forced by the code that's designed to interface directly with the server. --- src/api/streams/createStream.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/streams/createStream.js b/src/api/streams/createStream.js index b0dcc7648d2..ee5605c55d7 100644 --- a/src/api/streams/createStream.js +++ b/src/api/streams/createStream.js @@ -19,7 +19,7 @@ export default ( announce?: boolean, |}>, ): Promise => { - const { name, description = '', invite_only = false } = streamAttributes; + const { name, description, invite_only } = streamAttributes; const { principals = [], announce = false } = options ?? {}; return apiPost(auth, 'users/me/subscriptions', { subscriptions: JSON.stringify([{ name, description }]), From 9148ab3122c22f325e5b6a1ffffbacf3634b93ee Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 13:19:56 -0700 Subject: [PATCH 10/16] api: Remove more param-defaulting within our api.createStream binding With similar reasoning as in the previous commit, except these are in `options` instead of `streamAttributes`. This time, one of the properties doesn't have a default applied by the single caller: `announce`. So this is a behavior change: we're now sometimes omitting `announce`, which lets the server decide what behavior to fall back on. Currently, it's documented as falling back to `false` (and that should apply for all non-ancient servers): https://zulip.com/api/subscribe#parameter-announce --- src/api/streams/createStream.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/streams/createStream.js b/src/api/streams/createStream.js index ee5605c55d7..2e70ee7e149 100644 --- a/src/api/streams/createStream.js +++ b/src/api/streams/createStream.js @@ -20,7 +20,7 @@ export default ( |}>, ): Promise => { const { name, description, invite_only } = streamAttributes; - const { principals = [], announce = false } = options ?? {}; + const { principals, announce } = options ?? {}; return apiPost(auth, 'users/me/subscriptions', { subscriptions: JSON.stringify([{ name, description }]), invite_only, From ffc44bc89b89aaa5438199a7809e0221305dc57f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 13:26:14 -0700 Subject: [PATCH 11/16] CreateStreamScreen: Let server apply its own fallback for `principals` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This seems like a fine place to let the server control the behavior, and it's documented to give the same behavior anyway. See https://zulip.com/api/subscribe#parameter-announce : > […] If not provided, then the requesting user/bot is subscribed. We can start passing `principals` again when we eventually have a UI for choosing multiple users to be subscribed, as the web app does. For now, it's redundant and unnecessary for us to request the same behavior the server would give anyway. Especially since we're asking for it in a way that's discouraged: we use an email instead of a user ID. --- src/streams/CreateStreamScreen.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/streams/CreateStreamScreen.js b/src/streams/CreateStreamScreen.js index 43287603906..db081016e35 100644 --- a/src/streams/CreateStreamScreen.js +++ b/src/streams/CreateStreamScreen.js @@ -7,7 +7,7 @@ import type { AppNavigationProp } from '../nav/AppNavigator'; import * as NavigationService from '../nav/NavigationService'; import { useSelector } from '../react-redux'; import { navigateBack } from '../actions'; -import { getAuth, getOwnEmail } from '../selectors'; +import { getAuth } from '../selectors'; import Screen from '../common/Screen'; import EditStreamCard from './EditStreamCard'; import * as api from '../api'; @@ -19,14 +19,13 @@ type Props = $ReadOnly<{| export default function CreateStreamScreen(props: Props): Node { const auth = useSelector(getAuth); - const ownEmail = useSelector(getOwnEmail); const handleComplete = useCallback( (name: string, description: string, invite_only: boolean) => { - api.createStream(auth, { name, description, invite_only }, { principals: [ownEmail] }); + api.createStream(auth, { name, description, invite_only }); NavigationService.dispatch(navigateBack()); }, - [auth, ownEmail], + [auth], ); return ( From 2a5bcd206616b20efdf7b94f26f1f9936e130f94 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 13:35:16 -0700 Subject: [PATCH 12/16] userSelectors [nfc]: Bring a comment up-to-date Grepping shows that we actually use this in several places now. --- src/users/userSelectors.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/users/userSelectors.js b/src/users/userSelectors.js index 11bef03d9b1..334cecfed0f 100644 --- a/src/users/userSelectors.js +++ b/src/users/userSelectors.js @@ -80,8 +80,8 @@ export const getSortedUsers: Selector<$ReadOnlyArray> = createSelector(get * * See also `getOwnEmail` and `getOwnUser`. */ -// Not currently used, but should replace uses of `getOwnEmail` (e.g. inside -// `getOwnUser`). See #3764. +// Should replace uses of `getOwnEmail` (e.g. inside `getOwnUser`). See +// #3764. export const getOwnUserId = (state: PerAccountState): UserId => { const { user_id } = state.realm; if (user_id === undefined) { From aba93eefc3e15b860898c16cc4a479e65d6ee3f4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 13:34:45 -0700 Subject: [PATCH 13/16] userSelectors [nfc]: Remove now-unused getOwnEmail Related: #3764 --- src/users/userSelectors.js | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/src/users/userSelectors.js b/src/users/userSelectors.js index 334cecfed0f..a4effcb6e2c 100644 --- a/src/users/userSelectors.js +++ b/src/users/userSelectors.js @@ -78,10 +78,8 @@ export const getSortedUsers: Selector<$ReadOnlyArray> = createSelector(get * * Throws if we have no data from the server. * - * See also `getOwnEmail` and `getOwnUser`. + * See also `getOwnUser`. */ -// Should replace uses of `getOwnEmail` (e.g. inside `getOwnUser`). See -// #3764. export const getOwnUserId = (state: PerAccountState): UserId => { const { user_id } = state.realm; if (user_id === undefined) { @@ -90,21 +88,6 @@ export const getOwnUserId = (state: PerAccountState): UserId => { return user_id; }; -/** - * The user's own email in the active account. - * - * Throws if we have no data from the server. - * - * Prefer using `getOwnUserId` or `getOwnUser`; see #3764. - */ -export const getOwnEmail = (state: PerAccountState): string => { - const { email } = state.realm; - if (email === undefined) { - throw new Error('No server data found'); - } - return email; -}; - /** * The person using the app, as represented by a `User` object. * @@ -114,7 +97,7 @@ export const getOwnEmail = (state: PerAccountState): string => { * * Throws if we have no such information. * - * See also `getOwnUserId` and `getOwnEmail`. + * See also `getOwnUserId`. */ export const getOwnUser = (state: PerAccountState): User => { const ownUser = getUsersById(state).get(getOwnUserId(state)); From 7fee95bbdc8e70209b71f31c60176f9794b4d4a9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 13:39:28 -0700 Subject: [PATCH 14/16] api types [nfc]: Express createStream's params by referring to Stream type --- src/api/streams/createStream.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/api/streams/createStream.js b/src/api/streams/createStream.js index 2e70ee7e149..604e6bbc471 100644 --- a/src/api/streams/createStream.js +++ b/src/api/streams/createStream.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import type { ApiResponse, Auth } from '../transportTypes'; +import type { Stream } from '../modelTypes'; import { apiPost } from '../apiFetch'; /** See https://zulip.com/api/create-stream */ @@ -8,9 +9,9 @@ export default ( streamAttributes: $ReadOnly<{| // Ordered by their appearance in the doc. - name: string, - description?: string, - invite_only?: boolean, + name: $PropertyType, + description?: $PropertyType, + invite_only?: $PropertyType, |}>, options?: $ReadOnly<{| // TODO(server-3.0): Send numeric user IDs (#3764), not emails. From d002af06eb7f0ed44115fc0f97a750b1c6de5a9c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 14:05:50 -0700 Subject: [PATCH 15/16] api [nfc]: Use spreads, to reduce duplication --- src/api/streams/createStream.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/api/streams/createStream.js b/src/api/streams/createStream.js index 604e6bbc471..640f298cbcb 100644 --- a/src/api/streams/createStream.js +++ b/src/api/streams/createStream.js @@ -20,13 +20,13 @@ export default ( announce?: boolean, |}>, ): Promise => { - const { name, description, invite_only } = streamAttributes; - const { principals, announce } = options ?? {}; + const { name, description, ...restAttributes } = streamAttributes; + const { principals, ...restOptions } = options ?? {}; return apiPost(auth, 'users/me/subscriptions', { subscriptions: JSON.stringify([{ name, description }]), - invite_only, + ...restAttributes, principals: JSON.stringify(principals), - announce, + ...restOptions, }); }; From 6eff27afe89486ff79f947284944c27300320976 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Apr 2022 13:42:31 -0700 Subject: [PATCH 16/16] api types: Sync createStream's params with API doc at FL 121 --- src/api/streams/createStream.js | 34 ++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/api/streams/createStream.js b/src/api/streams/createStream.js index 640f298cbcb..db43d529b46 100644 --- a/src/api/streams/createStream.js +++ b/src/api/streams/createStream.js @@ -3,7 +3,14 @@ import type { ApiResponse, Auth } from '../transportTypes'; import type { Stream } from '../modelTypes'; import { apiPost } from '../apiFetch'; -/** See https://zulip.com/api/create-stream */ +/** + * See https://zulip.com/api/create-stream + * + * NB the caller must adapt for old-server compatibility; see comments. + */ +// Current to FL 121. +// TODO(#4659): Once we pass the feature level to API methods, this one +// should encapsulate various feature-level switches; see comments. export default ( auth: Auth, streamAttributes: $ReadOnly<{| @@ -12,11 +19,36 @@ export default ( name: $PropertyType, description?: $PropertyType, invite_only?: $PropertyType, + + // TODO(server-5.0): New in FL 98 + is_web_public?: $PropertyType, + + history_public_to_subscribers?: $PropertyType, + + // TODO(server-3.0): New in FL 1; for older servers, pass is_announcement_only. + stream_post_policy?: $PropertyType, + + // Doesn't take the same special values as Stream.is_announcement_only! + // https://chat.zulip.org/#narrow/stream/412-api-documentation/topic/message_retention_days/near/1367895 + // TODO(server-3.0): New in FL 17 + message_retention_days?: + | number + | 'realm_default' + // TODO(server-5.0): New in FL 91, replacing 'forever'. + | 'unlimited' + // TODO(server-5.0): Replaced in FL 91 by 'unlimited'. + | 'forever', + + // TODO(server-3.0): Replaced in FL 1 by 'stream_post_policy'. + // Commented out because this isn't actually in the doc. It probably + // exists though? Copied from api.updateStream. + // is_announcement_only?: $PropertyType, |}>, options?: $ReadOnly<{| // TODO(server-3.0): Send numeric user IDs (#3764), not emails. principals?: $ReadOnlyArray, + authorization_errors_fatal?: boolean, announce?: boolean, |}>, ): Promise => {