diff --git a/src/api/eventTypes.js b/src/api/eventTypes.js index c980b77e9fc..5cb504fd92f 100644 --- a/src/api/eventTypes.js +++ b/src/api/eventTypes.js @@ -298,17 +298,19 @@ export type UserSettingsUpdateEvent = $ReadOnly<{| * * See also `messageMoved` in `misc.js`. */ -// This is current to feature level 109. +// This is current to feature level 132. // NB if this ever gains a feature of moving PMs, `messageMoved` needs updating. export type UpdateMessageEvent = $ReadOnly<{| ...EventCommon, type: typeof EventTypes.update_message, - // Future servers might send `null` here: - // https://chat.zulip.org/#narrow/stream/378-api-design/topic/.60update_message.60.20event/near/1309241 - // TODO(server-5.0): Update this and/or simplify. + // Before FL 114, can be absent with the same meaning as null. + // TODO(server-5.0): Make required. user_id?: UserId | null, + // TODO(server-5.0): New in FL 114. On old servers, infer from `user_id`. + rendering_only?: boolean, + // Any content changes apply to just message_id. message_id: number, @@ -320,9 +322,16 @@ export type UpdateMessageEvent = $ReadOnly<{| message_ids: $ReadOnlyArray, flags: $ReadOnlyArray, + + // TODO(server-5.0): Always present as of FL 114; make required. edit_timestamp?: number, + stream_name?: string, + + // As of FL 112, present for all stream-message updates. + // TODO(server-5.0): Remove comment but keep optional; absent for PMs. stream_id?: number, + new_stream_id?: number, propagate_mode?: PropagateMode, orig_subject?: string, diff --git a/src/api/messages/getMessages.js b/src/api/messages/getMessages.js index ff8d40e6e1e..be5ac66f725 100644 --- a/src/api/messages/getMessages.js +++ b/src/api/messages/getMessages.js @@ -2,7 +2,7 @@ import type { Auth, ApiResponseSuccess } from '../transportTypes'; import type { Identity } from '../../types'; import type { Message, ApiNarrow } from '../apiTypes'; -import type { PmMessage, StreamMessage, Reaction, UserId } from '../modelTypes'; +import type { PmMessage, StreamMessage, Reaction, UserId, MessageEdit } from '../modelTypes'; import { apiGet } from '../apiFetch'; import { identityOfAuth } from '../../account/accountMisc'; import { AvatarURL } from '../../utils/avatar'; @@ -62,7 +62,7 @@ type ServerMessageOf = $ReadOnly<{| ...$Exact, avatar_url: string | null, reactions: $ReadOnlyArray, - edit_history: $ReadOnlyArray, + edit_history?: $ReadOnlyArray, |}>; export type ServerMessage = ServerMessageOf | ServerMessageOf; @@ -100,7 +100,7 @@ export const migrateMessages = ( /* eslint-disable operator-linebreak */ zulipFeatureLevel >= 118 ? // $FlowIgnore[incompatible-cast] - See MessageEdit type - (message.edit_history: Message['edit_history']) + (message.edit_history: $ReadOnlyArray | void) : null, })); diff --git a/src/api/modelTypes.js b/src/api/modelTypes.js index 1f624ddb014..4e1b9d16ec9 100644 --- a/src/api/modelTypes.js +++ b/src/api/modelTypes.js @@ -749,9 +749,38 @@ type MessageBase = $ReadOnly<{| content_type: 'text/html', // display_recipient handled on PmMessage and StreamMessage separately - // Optional because we only store it in Redux if it's coming from a server - // with FL >=118; see comment on MessageEdit. - edit_history: $ReadOnlyArray | null, + /** + * A possibly incomplete view of the message's edit history. + * + * This is null if it's coming from a server with FL <118; see comment on + * MessageEdit. + * + * Missing/undefined if the message had no edit history when we added it + * to the state. + * + * Missing/undefined if, at the time we added it to the state, the realm + * didn't allow viewing edit history. + * + * Stale if the message was updated or moved after we added it to the state. + */ + // TODO: Keep reasonably current: + // - Use `null` for the `allow_edit_history`-false case, to distinguish it + // from the message-never-edited case. (Handle everywhere we add/update + // Messages in Redux: the get-messages response, new-message events, + // update-message events.) + // - Handle changes to the allow_edit_history setting: + // - Treat an allow_edit_history change similar to a restart event (in + // particular, a restart event where the server version changed, which + // could mean any number of subtle changes in server behavior), and do + // a re-fetch of server data soon after it. + // - Handle update-message events, writing the FL 118+ shape. + // + // TODO(server-5.0): Remove FL <118 condition + // + // (Why optional and `| void`? We convert missing to undefined at the + // edge, while doing the FL <118 condition, but that's incidental and + // could easily change.) + edit_history?: $ReadOnlyArray | null | void, id: number, is_me_message: boolean, diff --git a/src/events/eventToAction.js b/src/events/eventToAction.js index 74d2c4bf9ea..4c945e32ec1 100644 --- a/src/events/eventToAction.js +++ b/src/events/eventToAction.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import { EventTypes, type EventType, type RealmUserUpdateEventRaw } from '../api/eventTypes'; import * as logging from '../utils/logging'; -import type { PerAccountState, EventAction, Message } from '../types'; +import type { PerAccountState, EventAction, MessageEdit } from '../types'; import { EVENT_ALERT_WORDS, EVENT_NEW_MESSAGE, @@ -120,7 +120,7 @@ export default (state: PerAccountState, event: $FlowFixMe): EventAction | null = edit_history: // Why FL 118 condition? See MessageEdit type. zulipFeatureLevel >= 118 - ? (event.message.edit_history: $NonMaybeType) + ? (event.message.edit_history: $ReadOnlyArray | void) : null, }, local_message_id: event.local_message_id, diff --git a/src/message/__tests__/messagesReducer-test.js b/src/message/__tests__/messagesReducer-test.js index 6501bd8a0c4..3e0dffbe0f3 100644 --- a/src/message/__tests__/messagesReducer-test.js +++ b/src/message/__tests__/messagesReducer-test.js @@ -292,6 +292,10 @@ describe('messagesReducer', () => { ]), ); }); + + test.todo('multiple messages moved in one event'); + + test.todo("edited one message's content + multiple messages moved in one event"); }); test('when a message exists in state, it is updated', () => { diff --git a/src/realm/__tests__/realmReducer-test.js b/src/realm/__tests__/realmReducer-test.js index 4f3307bc22f..a445fcaeff5 100644 --- a/src/realm/__tests__/realmReducer-test.js +++ b/src/realm/__tests__/realmReducer-test.js @@ -57,6 +57,7 @@ describe('realmReducer', () => { createWebPublicStreamPolicy: action.data.realm_create_web_public_stream_policy, enableSpectatorAccess: action.data.realm_enable_spectator_access, waitingPeriodThreshold: action.data.realm_waiting_period_threshold, + allowEditHistory: action.data.realm_allow_edit_history, // // InitialDataRealmUser @@ -457,6 +458,14 @@ describe('realmReducer', () => { check(90, 90); check(90, 30); }); + + describe('allowEditHistory / allow_edit_history', () => { + const check = mkCheck('allowEditHistory', 'allow_edit_history'); + check(true, true); + check(true, false); + check(false, true); + check(false, false); + }); }); }); }); diff --git a/src/realm/realmReducer.js b/src/realm/realmReducer.js index 6c2a39b15fa..3ee5a980a05 100644 --- a/src/realm/realmReducer.js +++ b/src/realm/realmReducer.js @@ -51,6 +51,7 @@ const initialState = { createWebPublicStreamPolicy: CreateWebPublicStreamPolicy.Nobody, enableSpectatorAccess: false, waitingPeriodThreshold: 90, + allowEditHistory: false, // // InitialDataRealmUser @@ -149,6 +150,7 @@ export default ( action.data.realm_create_web_public_stream_policy ?? CreateWebPublicStreamPolicy.Nobody, enableSpectatorAccess: action.data.realm_enable_spectator_access ?? false, waitingPeriodThreshold: action.data.realm_waiting_period_threshold, + allowEditHistory: action.data.realm_allow_edit_history, // // InitialDataRealmUser @@ -240,6 +242,9 @@ export default ( if (data.waiting_period_threshold !== undefined) { result.waitingPeriodThreshold = data.waiting_period_threshold; } + if (data.allow_edit_history !== undefined) { + result.allowEditHistory = data.allow_edit_history; + } return result; } diff --git a/src/reduxTypes.js b/src/reduxTypes.js index a910f92b541..42a861c0dd6 100644 --- a/src/reduxTypes.js +++ b/src/reduxTypes.js @@ -303,6 +303,7 @@ export type RealmState = {| +createWebPublicStreamPolicy: CreateWebPublicStreamPolicyT, +enableSpectatorAccess: boolean, +waitingPeriodThreshold: number, + +allowEditHistory: boolean, // // InitialDataRealmUser diff --git a/src/storage/__tests__/migrations-test.js b/src/storage/__tests__/migrations-test.js index 74af2f1f380..83bb1573701 100644 --- a/src/storage/__tests__/migrations-test.js +++ b/src/storage/__tests__/migrations-test.js @@ -87,7 +87,7 @@ describe('migrations', () => { // What `base` becomes after all migrations. const endBase = { ...base37, - migrations: { version: 49 }, + migrations: { version: 50 }, }; for (const [desc, before, after] of [ @@ -110,8 +110,8 @@ describe('migrations', () => { // redundant with this one, because none of the migration steps notice // whether any properties outside `storeKeys` are present or not. [ - 'check dropCache at 49', - { ...endBase, migrations: { version: 48 }, mute: [], nonsense: [1, 2, 3] }, + 'check dropCache at 50', + { ...endBase, migrations: { version: 49 }, mute: [], nonsense: [1, 2, 3] }, endBase, ], diff --git a/src/storage/migrations.js b/src/storage/migrations.js index 6938185536f..861337c0a21 100644 --- a/src/storage/migrations.js +++ b/src/storage/migrations.js @@ -437,6 +437,9 @@ const migrationsInner: {| [string]: (LessPartialState) => LessPartialState |} = // Add defaultExternalAccounts to state.realm. '49': dropCache, + // Add allowEditHistory to state.realm. + '50': dropCache, + // TIP: When adding a migration, consider just using `dropCache`. // (See its jsdoc for guidance on when that's the right answer.) };