diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js index a32915371fe..a3aad4e2b42 100644 --- a/src/__tests__/lib/exampleData.js +++ b/src/__tests__/lib/exampleData.js @@ -293,7 +293,14 @@ export const crossRealmBot: CrossRealmBot = makeCrossRealmBot({ const randStreamId: () => number = makeUniqueRandInt('stream IDs', 1000); export const makeStream = ( - args: {| stream_id?: number, name?: string, description?: string |} = Object.freeze({}), + args: {| + stream_id?: number, + name?: string, + description?: string, + invite_only?: boolean, + is_web_public?: boolean, + history_public_to_subscribers?: boolean, + |} = Object.freeze({}), ): Stream => { const name = args.name ?? randString(); return deepFreeze({ @@ -301,10 +308,10 @@ export const makeStream = ( name, description: args.description ?? `On the ${randString()} of ${name}`, rendered_description: args.description ?? `

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

`, - invite_only: false, + invite_only: args.invite_only ?? false, is_announcement_only: false, - is_web_public: false, - history_public_to_subscribers: true, + is_web_public: args.is_web_public ?? false, + history_public_to_subscribers: args.history_public_to_subscribers ?? true, first_message_id: null, }); }; diff --git a/src/api/eventTypes.js b/src/api/eventTypes.js index 1a8444e6dc0..015512a4284 100644 --- a/src/api/eventTypes.js +++ b/src/api/eventTypes.js @@ -149,21 +149,91 @@ type StreamListEvent = $ReadOnly<{| streams: $ReadOnlyArray, |}>; +type StreamUpdateEventBase = $ReadOnly<{| + ...EventCommon, + type: typeof EventTypes.stream, + op: 'update', + stream_id: number, + name: string, +|}>; + +// https://zulip.com/api/get-events#stream-update +export type StreamUpdateEvent = + | {| ...StreamUpdateEventBase, +property: 'name', +value: $PropertyType |} + | {| + ...StreamUpdateEventBase, + +property: 'description', + +value: $PropertyType, + +rendered_description: $PropertyType, + |} + // TODO(server-4.0): New in FL 30. + | {| + ...StreamUpdateEventBase, + +property: 'date_created', + +value: $PropertyType, + |} + | {| + ...StreamUpdateEventBase, + +property: 'invite_only', + +value: $PropertyType, + +history_public_to_subscribers: $PropertyType, + + // TODO(server-5.0): New in FL 71. + +is_web_public?: $PropertyType, + |} + | {| + ...StreamUpdateEventBase, + +property: 'rendered_description', + +value: $PropertyType, + |} + | {| + ...StreamUpdateEventBase, + +property: 'is_web_public', + +value: $PropertyType, + |} + // TODO(server-3.0): New in FL 1; expect is_announcement_only from older + // servers. + | {| + ...StreamUpdateEventBase, + +property: 'stream_post_policy', + +value: $PropertyType, + |} + // 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. + | {| + ...StreamUpdateEventBase, + +property: 'message_retention_days', + +value: $PropertyType, + |} + | {| + ...StreamUpdateEventBase, + +property: 'history_public_to_subscribers', + +value: $PropertyType, + |} + | {| + ...StreamUpdateEventBase, + +property: 'first_message_id', + +value: $PropertyType, + |} + // TODO(server-3.0): Deprecated at FL 1; expect stream_post_policy from + // newer servers. + | {| + ...StreamUpdateEventBase, + +property: 'is_announcement_only', + +value: $PropertyType, + |}; + // prettier-ignore export type StreamEvent = | {| ...StreamListEvent, +op: 'create', |} | {| ...StreamListEvent, +op: 'delete', |} | {| ...StreamListEvent, +op: 'occupy', |} | {| ...StreamListEvent, +op: 'vacate', |} - | $ReadOnly<{| - ...EventCommon, - type: typeof EventTypes.stream, - op: 'update', - stream_id: number, - name: string, - property: string, - value: string, - |}>; + | StreamUpdateEvent; export type UpdateMessageFlagsEvent = $ReadOnly<{| ...EventCommon, diff --git a/src/streams/__tests__/streamsReducer-test.js b/src/streams/__tests__/streamsReducer-test.js index f9dcb67ed4c..9b8b85c3a36 100644 --- a/src/streams/__tests__/streamsReducer-test.js +++ b/src/streams/__tests__/streamsReducer-test.js @@ -10,13 +10,9 @@ import streamsReducer from '../streamsReducer'; describe('streamsReducer', () => { describe('ACCOUNT_SWITCH', () => { test('resets state to initial state', () => { - const initialState = deepFreeze([eg.makeStream({ name: 'some_stream' })]); - - const expectedState = []; - - const actualState = streamsReducer(initialState, eg.action.account_switch); - - expect(actualState).toEqual(expectedState); + expect( + streamsReducer([eg.makeStream({ name: 'some_stream' })], eg.action.account_switch), + ).toEqual([]); }); }); @@ -25,23 +21,20 @@ describe('streamsReducer', () => { const stream1 = eg.makeStream({ name: 'some stream', stream_id: 1 }); const stream2 = eg.makeStream({ name: 'some other stream', stream_id: 2 }); - const initialState = deepFreeze([]); - - const action = deepFreeze({ - type: EVENT, - event: { - id: 0, - type: EventTypes.stream, - op: 'create', - streams: [stream1, stream2], - }, - }); - - const expectedState = [stream1, stream2]; - - const newState = streamsReducer(initialState, action); - - expect(newState).toEqual(expectedState); + expect( + streamsReducer( + [], + deepFreeze({ + type: EVENT, + event: { + id: 0, + type: EventTypes.stream, + op: 'create', + streams: [stream1, stream2], + }, + }), + ), + ).toEqual([stream1, stream2]); }); test('if stream already exist, do not add it', () => { @@ -52,23 +45,20 @@ describe('streamsReducer', () => { }); const stream2 = eg.makeStream({ name: 'some other stream', stream_id: 2 }); - const initialState = deepFreeze([stream1]); - - const action = deepFreeze({ - type: EVENT, - event: { - id: 0, - type: EventTypes.stream, - op: 'create', - streams: [stream1, stream2], - }, - }); - - const expectedState = [stream1, stream2]; - - const newState = streamsReducer(initialState, action); - - expect(newState).toEqual(expectedState); + expect( + streamsReducer( + [stream1], + deepFreeze({ + type: EVENT, + event: { + id: 0, + type: EventTypes.stream, + op: 'create', + streams: [stream1, stream2], + }, + }), + ), + ).toEqual([stream1, stream2]); }); }); @@ -90,46 +80,40 @@ describe('streamsReducer', () => { name: 'third stream', }); - const initialState = deepFreeze([stream1, stream2, stream3]); - - const action = deepFreeze({ - type: EVENT, - event: { - id: 0, - type: EventTypes.stream, - op: 'delete', - streams: [stream1, stream2], - }, - }); - - const expectedState = [stream3]; - - const newState = streamsReducer(initialState, action); - - expect(newState).toEqual(expectedState); + const newState = streamsReducer( + [stream1, stream2, stream3], + deepFreeze({ + type: EVENT, + event: { + id: 0, + type: EventTypes.stream, + op: 'delete', + streams: [stream1, stream2], + }, + }), + ); + + expect(newState).toEqual([stream3]); }); test('removes streams that exist, do nothing if not', () => { const stream1 = eg.makeStream({ name: 'some stream', stream_id: 1 }); const stream2 = eg.makeStream({ name: 'some other stream', stream_id: 2 }); - const initialState = deepFreeze([stream1]); - - const action = deepFreeze({ - type: EVENT, - event: { - id: 0, - type: EventTypes.stream, - op: 'delete', - streams: [stream1, stream2], - }, - }); - - const expectedState = []; - - const newState = streamsReducer(initialState, action); - - expect(newState).toEqual(expectedState); + expect( + streamsReducer( + [stream1], + deepFreeze({ + type: EVENT, + event: { + id: 0, + type: EventTypes.stream, + op: 'delete', + streams: [stream1, stream2], + }, + }), + ), + ).toEqual([]); }); }); @@ -139,26 +123,23 @@ describe('streamsReducer', () => { const stream67 = eg.makeStream({ stream_id: 67, name: 'design' }); const stream53 = eg.makeStream({ stream_id: 53, name: 'mobile' }); - const initialState = deepFreeze([stream123, stream67, stream53]); - - const action = deepFreeze({ - type: EVENT, - event: { - id: 2, - type: EventTypes.stream, - op: 'update', - stream_id: 123, - name: 'competition', - property: 'name', - value: 'real competition', - }, - }); - - const expectedState = [{ ...stream123, name: 'real competition' }, stream67, stream53]; - - const actualState = streamsReducer(initialState, action); - - expect(actualState).toEqual(expectedState); + expect( + streamsReducer( + [stream123, stream67, stream53], + deepFreeze({ + type: EVENT, + event: { + id: 2, + type: EventTypes.stream, + op: 'update', + stream_id: 123, + name: 'competition', + property: 'name', + value: 'real competition', + }, + }), + ), + ).toEqual([{ ...stream123, name: 'real competition' }, stream67, stream53]); }); test('Change the description property', () => { @@ -174,26 +155,73 @@ describe('streamsReducer', () => { }); const stream53 = eg.makeStream({ stream_id: 53, name: 'mobile', description: 'android' }); - const initialState = deepFreeze([stream123, stream67, stream53]); - - const action = deepFreeze({ - type: EVENT, - event: { - id: 2, - type: EventTypes.stream, - op: 'update', - stream_id: 53, - name: 'mobile', - property: 'description', - value: 'iOS + android', - }, - }); - - const expectedState = [stream123, stream67, { ...stream53, description: 'iOS + android' }]; + expect( + streamsReducer( + [stream123, stream67, stream53], + deepFreeze({ + type: EVENT, + event: { + id: 2, + type: EventTypes.stream, + op: 'update', + stream_id: 53, + name: 'mobile', + property: 'description', + value: 'iOS + android', + rendered_description: '

iOS + android

', + }, + }), + ), + ).toEqual([ + stream123, + stream67, + { ...stream53, description: 'iOS + android', rendered_description: '

iOS + android

' }, + ]); + }); - const actualState = streamsReducer(initialState, action); + test('Change the invite_only property', () => { + const stream1 = eg.makeStream({ + stream_id: 1, + name: 'web public stream', + invite_only: false, + is_web_public: true, + history_public_to_subscribers: true, + }); + const stream2 = eg.makeStream({ + stream_id: 2, + name: 'invite only stream', + invite_only: true, + is_web_public: false, + history_public_to_subscribers: true, + }); - expect(actualState).toEqual(expectedState); + expect( + streamsReducer( + [stream1, stream2], + deepFreeze({ + type: EVENT, + event: { + id: 0, + type: EventTypes.stream, + op: 'update', + stream_id: stream1.stream_id, + name: stream1.name, + property: 'invite_only', + value: true, + is_web_public: false, + history_public_to_subscribers: false, + }, + }), + ), + ).toEqual([ + { + ...stream1, + invite_only: true, + is_web_public: false, + history_public_to_subscribers: false, + }, + stream2, + ]); }); }); }); diff --git a/src/streams/streamsReducer.js b/src/streams/streamsReducer.js index 3473809b83a..2da46fd72d9 100644 --- a/src/streams/streamsReducer.js +++ b/src/streams/streamsReducer.js @@ -1,6 +1,7 @@ /* @flow strict-local */ import { EventTypes } from '../api/eventTypes'; -import type { PerAccountApplicableAction, StreamsState } from '../types'; +import type { PerAccountApplicableAction, StreamsState, StreamUpdateEvent } from '../types'; +import type { Stream, Subscription } from '../api/modelTypes'; import { ensureUnreachable } from '../types'; import { LOGOUT, ACCOUNT_SWITCH, EVENT, REGISTER_COMPLETE } from '../actionConstants'; import { NULL_ARRAY } from '../nullObjects'; @@ -8,6 +9,48 @@ import { filterArray } from '../utils/immutability'; const initialState: StreamsState = NULL_ARRAY; +export function updateStreamProperties( + stream: S, + event: StreamUpdateEvent, +): S { + switch (event.property) { + case ('name': 'name'): + return { ...stream, [event.property]: event.value }; + case ('description': 'description'): + return { + ...stream, + [event.property]: event.value, + rendered_description: event.rendered_description, + }; + case ('date_created': 'date_created'): + return { ...stream, [event.property]: event.value }; + case ('invite_only': 'invite_only'): + return { + ...stream, + [event.property]: event.value, + history_public_to_subscribers: event.history_public_to_subscribers, + is_web_public: event.is_web_public, + }; + case ('rendered_description': 'rendered_description'): + return { ...stream, [event.property]: event.value }; + case ('is_web_public': 'is_web_public'): + return { ...stream, [event.property]: event.value }; + case ('stream_post_policy': 'stream_post_policy'): + return { ...stream, [event.property]: event.value }; + case ('message_retention_days': 'message_retention_days'): + return { ...stream, [event.property]: event.value }; + case ('history_public_to_subscribers': 'history_public_to_subscribers'): + return { ...stream, [event.property]: event.value }; + case ('first_message_id': 'first_message_id'): + return { ...stream, [event.property]: event.value }; + case ('is_announcement_only': 'is_announcement_only'): + return { ...stream, [event.property]: event.value }; + default: + ensureUnreachable(event.property); + return stream; + } +} + export default ( state: StreamsState = initialState, action: PerAccountApplicableAction, @@ -37,14 +80,13 @@ export default ( ); case 'update': - return state.map(stream => - stream.stream_id === event.stream_id - ? { - ...stream, - [event.property]: event.value, - } - : stream, - ); + return state.map(stream => { + if (stream.stream_id !== event.stream_id) { + return stream; + } + + return updateStreamProperties(stream, event); + }); case 'occupy': case 'vacate': diff --git a/src/subscriptions/subscriptionsReducer.js b/src/subscriptions/subscriptionsReducer.js index 3b28221f08d..e8a5ee7fb4b 100644 --- a/src/subscriptions/subscriptionsReducer.js +++ b/src/subscriptions/subscriptionsReducer.js @@ -2,6 +2,7 @@ import { EventTypes } from '../api/eventTypes'; import type { SubscriptionsState, PerAccountApplicableAction } from '../types'; import { ensureUnreachable } from '../types'; +import { updateStreamProperties } from '../streams/streamsReducer'; import { LOGOUT, LOGIN_SUCCESS, @@ -15,11 +16,6 @@ import { filterArray } from '../utils/immutability'; const initialState: SubscriptionsState = NULL_ARRAY; -const updateSubscription = (state, event) => - state.map(sub => - sub.stream_id === event.stream_id ? { ...sub, [event.property]: event.value } : sub, - ); - export default ( state: SubscriptionsState = initialState, action: PerAccountApplicableAction, @@ -46,7 +42,9 @@ export default ( ); case 'update': - return updateSubscription(state, action); + return state.map(sub => + sub.stream_id === action.stream_id ? { ...sub, [action.property]: action.value } : sub, + ); case 'peer_add': case 'peer_remove': @@ -64,7 +62,12 @@ export default ( case EventTypes.stream: switch (event.op) { case 'update': - return updateSubscription(state, event); + return state.map(sub => + // If inlining `updateStreamProperties` with plans to change + // its logic, note that it has test coverage at its other + // callsite, but not here, as of 2022-04-19. + sub.stream_id === event.stream_id ? updateStreamProperties(sub, event) : sub, + ); case 'delete': return filterArray(