Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: Sync updateStream and createStream with their API docs #5341

Merged
merged 16 commits into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
65ddf3f
api.updateStream [nfc]: Accept multiple properties at once
chrisbobbe Apr 14, 2022
be12766
streamsActions: Send one update-stream request for all changed proper…
chrisbobbe Apr 14, 2022
3fc7aa3
api types [nfc]: Express updateStream's params by referring to Stream…
chrisbobbe Apr 15, 2022
eb50271
modelTypes [nfc]: Order Stream type's properties to follow a doc
chrisbobbe Apr 16, 2022
dd6fea1
modelTypes: Update Stream type, to match `streams` initial data at FL…
chrisbobbe Apr 15, 2022
0c56883
api types: Sync updateStream's params with API doc at FL 121
chrisbobbe Apr 16, 2022
2668bde
stream [nfc]: Assimilate to name "invite_only" in create/edit stream …
gnprice Mar 31, 2022
08c0697
api [nfc]: Have createStream use params objects
chrisbobbe Apr 18, 2022
94d7a59
api [nfc]: Remove some param-defaulting within our api.createStream b…
chrisbobbe Apr 18, 2022
9148ab3
api: Remove more param-defaulting within our api.createStream binding
chrisbobbe Apr 18, 2022
ffc44bc
CreateStreamScreen: Let server apply its own fallback for `principals`
chrisbobbe Apr 18, 2022
2a5bcd2
userSelectors [nfc]: Bring a comment up-to-date
chrisbobbe Apr 18, 2022
aba93ee
userSelectors [nfc]: Remove now-unused getOwnEmail
chrisbobbe Apr 18, 2022
7fee95b
api types [nfc]: Express createStream's params by referring to Stream…
chrisbobbe Apr 18, 2022
d002af0
api [nfc]: Use spreads, to reduce duplication
chrisbobbe Apr 18, 2022
6eff27a
api types: Sync createStream's params with API doc at FL 121
chrisbobbe Apr 18, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/__tests__/lib/exampleData.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? `<p>On the ${randString()} of ${name}</p>`,
invite_only: false,
is_announcement_only: false,
is_web_public: false,
history_public_to_subscribers: true,
first_message_id: null,
});
};

Expand Down
31 changes: 29 additions & 2 deletions src/api/modelTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,15 +330,42 @@ export type MutedUser = $ReadOnly<{|
//

export type Stream = {|
// 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,
+description: string,
+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,
+is_announcement_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,
|};

export type Subscription = {|
Expand Down
66 changes: 55 additions & 11 deletions src/api/streams/createStream.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,64 @@
/* @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 */
/**
* 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,
name: string,
description?: string = '',
// TODO(server-3.0): Send numeric user IDs (#3764), not emails.
principals?: $ReadOnlyArray<string> = [],
inviteOnly?: boolean = false,
announce?: boolean = false,
): Promise<ApiResponse> =>
apiPost(auth, 'users/me/subscriptions', {
streamAttributes: $ReadOnly<{|
// Ordered by their appearance in the doc.

name: $PropertyType<Stream, 'name'>,
description?: $PropertyType<Stream, 'description'>,
invite_only?: $PropertyType<Stream, 'invite_only'>,

// TODO(server-5.0): New in FL 98
is_web_public?: $PropertyType<Stream, 'is_web_public'>,

history_public_to_subscribers?: $PropertyType<Stream, 'history_public_to_subscribers'>,

// TODO(server-3.0): New in FL 1; for older servers, pass is_announcement_only.
stream_post_policy?: $PropertyType<Stream, 'stream_post_policy'>,

// 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<Stream, 'is_announcement_only'>,
Comment on lines +42 to +45
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, curious.

|}>,
options?: $ReadOnly<{|
// TODO(server-3.0): Send numeric user IDs (#3764), not emails.
principals?: $ReadOnlyArray<string>,

authorization_errors_fatal?: boolean,
announce?: boolean,
|}>,
): Promise<ApiResponse> => {
const { name, description, ...restAttributes } = streamAttributes;
const { principals, ...restOptions } = options ?? {};
return apiPost(auth, 'users/me/subscriptions', {
subscriptions: JSON.stringify([{ name, description }]),
...restAttributes,

principals: JSON.stringify(principals),
invite_only: inviteOnly,
announce,
...restOptions,
});
};
46 changes: 36 additions & 10 deletions src/api/streams/updateStream.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,47 @@
/* @flow strict-local */
import type { ApiResponse, Auth } from '../transportTypes';
import type { Stream } from '../modelTypes';
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,
property: string,
value: string | boolean,
): Promise<ApiResponse> =>
apiPatch(auth, `streams/${id}`, {
[property]: value,
});
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<Stream, 'description'>,
new_name?: $PropertyType<Stream, 'name'>,

// controls the invite_only property
is_private?: $PropertyType<Stream, 'invite_only'>,

// TODO(server-5.0): New in FL 98.
is_web_public?: $PropertyType<Stream, 'is_web_public'>,

// TODO(server-3.0): New in FL 1; for older servers, pass is_announcement_only.
stream_post_policy?: $PropertyType<Stream, 'stream_post_policy'>,

history_public_to_subscribers?: $PropertyType<Stream, 'history_public_to_subscribers'>,

// 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<Stream, 'is_announcement_only'>,
|}>,
): Promise<ApiResponse> => apiPatch(auth, `streams/${id}`, params);
2 changes: 2 additions & 0 deletions src/nullObjects.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
};
9 changes: 4 additions & 5 deletions src/streams/CreateStreamScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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, isPrivate: boolean) => {
api.createStream(auth, name, description, [ownEmail], isPrivate);
(name: string, description: string, invite_only: boolean) => {
api.createStream(auth, { name, description, invite_only });
NavigationService.dispatch(navigateBack());
},
[auth, ownEmail],
[auth],
);

return (
Expand Down
18 changes: 9 additions & 9 deletions src/streams/EditStreamCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<Props, State> {
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 => {
Expand All @@ -54,8 +54,8 @@ export default class EditStreamCard extends PureComponent<Props, State> {
this.setState({ description });
};

handleIsPrivateChange: boolean => void = isPrivate => {
this.setState({ isPrivate });
handleInviteOnlyChange: boolean => void = invite_only => {
this.setState({ invite_only });
};

render(): Node {
Expand Down Expand Up @@ -83,8 +83,8 @@ export default class EditStreamCard extends PureComponent<Props, State> {
style={componentStyles.switchRow}
Icon={IconPrivate}
label="Private"
value={this.state.isPrivate}
onValueChange={this.handleIsPrivateChange}
value={this.state.invite_only}
onValueChange={this.handleInviteOnlyChange}
/>
<ZulipButton
style={styles.marginTop}
Expand Down
4 changes: 2 additions & 2 deletions src/streams/EditStreamScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export default function EditStreamScreen(props: Props): Node {
const stream = useSelector(state => 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],
Expand Down
15 changes: 10 additions & 5 deletions src/streams/streamsActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<Promise<void>> => async (dispatch, getState) => {
const state = getState();

Expand All @@ -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);
if (initialValues.invite_only !== newValues.invite_only) {
updates.is_private = newValues.invite_only;
}

if (Object.keys(updates).length > 0) {
await api.updateStream(auth, id, updates);
}
};
21 changes: 2 additions & 19 deletions src/users/userSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,8 @@ export const getSortedUsers: Selector<$ReadOnlyArray<User>> = createSelector(get
*
* Throws if we have no data from the server.
*
* See also `getOwnEmail` and `getOwnUser`.
* See also `getOwnUser`.
*/
// Not currently used, but 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) {
Expand All @@ -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.
*
Expand All @@ -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));
Expand Down