From 9dfbaeca99f4096388fd1c048a9e6d1d0a86743e Mon Sep 17 00:00:00 2001 From: Nick Misasi Date: Thu, 13 Aug 2026 07:27:23 -0400 Subject: [PATCH] Add weekly recurring scheduled posts (#37746) * Add weekly recurring scheduled posts. Extend scheduled posts so users can schedule weekly repeats and keep the series healthy across sends, reschedules, and UI updates instead of falling back to one-shot behavior. Made-with: Cursor * Add Playwright coverage for recurring scheduled posts. Cover weekly recurring scheduled messages in the scheduled-messages spec so the recurring UI and reschedule flow stay protected without adding a separate test surface. Made-with: Cursor * Fix recurring scheduled post CI failures. Resolve the initial lint and formatting issues and renumber the new scheduled-post migration so it no longer collides with master during Postgres-backed test setup. Made-with: Cursor * Sync recurring scheduled post translation files. Regenerate the affected English translation catalogs so the recurring scheduled post strings match the source extraction order expected by CI. Made-with: Cursor * Fix recurring scheduled post review follow-ups. Preserve overdue cleanup behavior during weekly catch-up, defer delete websocket events until deletion succeeds, and address the remaining migration and UI review nits. Made-with: Cursor * Address recurring scheduled post review feedback Made-with: Cursor * Fix recurring scheduled post CI checks Made-with: Cursor * Make pending scheduled post keyset cursor index-scannable EXPLAIN ANALYZE on a 5M-row ScheduledPosts table showed the pure OR-form cursor forced Postgres to scan idx_scheduledposts_pending_scheduled_at_id from the top on every page (~383ms/page, ~2M rows filtered). Keeping the ScheduledAt <= beforeTime bound outside the tie-break restores the index boundary (~0.12ms/page). Adds storetest coverage for cursor pagination. Co-authored-by: nick.misasi * Address code quality review findings for recurring scheduled posts Server: - Replace silent-fallback AdvanceWeeklyScheduledNextOccurrence with error-returning ScheduledPost.ComputeNextScheduledAt; a recurring post whose timezone fails to load is now routed through the failed-post path instead of being reposted every job run or silently deleted - Add ScheduledPost.IsRecurring and partition batches in processScheduledPostBatch; advance/delete now run independently so a store failure in one path can't cause reposts in the other - Drop dead generality in UpdateRecurringScheduledPosts (only ScheduledAt varies per row; ErrorCode/ProcessedAt are constants) - Simplify redundant repeat-type condition in GetPendingScheduledPosts Webapp: - Add shared isRecurringScheduledPost helper, replacing six scattered repeat_type === 'weekly' literals - Recurrence timezone is now simply the scheduler's current timezone; removes initialRepeatTimezone/effectiveTimezone plumbing and the modal-label/picker timezone mismatch - Single enforcement point for hiding send-now on recurring posts - Use canonical getTeamIdByChannelId at both SCHEDULED_POST_UPDATED dispatch sites; rewrite errorsByTeamId update case as remove-then-add and add reducer tests Co-authored-by: nick.misasi * Simplify recurring scheduled post code per review - End a recurring series when its channel no longer exists instead of advancing it forever (matches the one-shot channel-not-found handling) - Collapse the errorsByTeamId SCHEDULED_POST_UPDATED case into the identical SINGLE_SCHEDULED_POST_RECEIVED case (a scheduled post's team can't change) and combine duplicate byId cases; preserves state references on no-op updates - Drop no-op timezone conversions in ComputeNextScheduledAt - Remove redundant checkbox aria-label (label htmlFor already names it) - Schedule new job tests in the past instead of sleeping a real second - Remove redundant test assignment and e2e positional boolean Co-authored-by: nick.misasi * Remove slop from recurring scheduled post changes - Drop the business-rule CHECK constraint from the recurrence migration; no other migration enforces model-layer validation in the database, and BaseIsValid plus the job's failure handling already own it - Fold the standalone repeat-validation test file into the existing TestScheduledPostBaseIsValid, matching its conventions - Revert unrelated benchmark modernization in utils_test.go - Drop an unneeded cast, unused fixture fields, and naming/assertion inconsistencies in tests Co-authored-by: nick.misasi * Regenerate ScheduledPostStore mock with mockery ordering Co-authored-by: nick.misasi * Address CodeRabbit review feedback - Reject the host-dependent 'Local' value for RepeatTimezone; recurring schedules need a fixed zone (UTC or IANA name) - Hide reschedule for deactivated DMs, matching send-now eligibility Co-authored-by: nick.misasi * Resolve scheduled post team bucket from existing state when channel is unloaded An admin can reschedule before fetchMissingChannels resolves, in which case deriving the team from the channel returns undefined and the update was misfiled under directChannels. getScheduledPostTeamId falls back to the byTeamId bucket that already holds the post. Co-authored-by: nick.misasi * Retrigger CI after transient enterprise npm network failure Co-authored-by: nick.misasi * Clear hover/focus before asserting scheduled post header details in e2e The drafts panel hides its timestamp/tag info section while hovered or focus-within; after the reschedule modal closes, focus returns to the row and the 'Repeats weekly' tag assertion saw a hidden element. Co-authored-by: nick.misasi * Move recurrence columns into baseColumns and dispatch ComputeNextScheduledAt on repeat type Co-authored-by: nick.misasi * Preserve recurrence when scheduled post updates omit repeat fields Co-authored-by: nick.misasi * Disallow file attachments on recurring scheduled posts Co-authored-by: nick.misasi * Stop pinning the channel indicator for recurring-only scheduled posts Co-authored-by: nick.misasi * Re-home nullable-Type comment onto baseColumns Co-authored-by: nick.misasi * Gate recurring scheduled posts behind a default-off feature flag Co-authored-by: nick.misasi * Move presence-preservation rationale to the copy site Co-authored-by: nick.misasi * Extract draftHasAttachments helper and require allowRecurring at single-caller layers Co-authored-by: nick.misasi * Return null from the indicator selector when nothing should show Co-authored-by: nick.misasi * Gate only recurrence transitions and preserve existing series in the modal Co-authored-by: nick.misasi * Avoid err shadowing in scheduled post update handler Co-authored-by: nick.misasi * Disable the repeat weekly checkbox with a tooltip when the message has attachments Co-authored-by: nick.misasi * Return an explicit disposition from postScheduledPost The batch loop inferred 'channel permanently gone' from the one return path with a nil error and an error code set - an invariant a future change could silently break, deleting recurring series by accident. postScheduledPost now returns posted/failed/unsendable explicitly, the switch refuses to delete on an unhandled disposition, and a test pins that both recurring and one-shot posts in a nonexistent channel are permanently deleted. Co-authored-by: nick.misasi * Fix Repeat weekly attachments tooltip centering on the modal row (#37927) Shrink-wrap the repeat checkbox row so WithTooltip anchors to the control/label instead of the full modal body width. Co-authored-by: Cursor Agent --------- Co-authored-by: Cursor Agent --- api/v4/source/definitions.yaml | 8 + api/v4/source/scheduled_post.yaml | 12 + e2e-tests/.ci/server.generate.sh | 1 + .../lib/src/containers/env_baseline.ts | 1 + .../channels/schedule_message_modal.ts | 16 +- .../ui/components/channels/scheduled_post.ts | 2 + .../playwright/lib/src/ui/pages/channels.ts | 18 +- .../lib/src/ui/pages/scheduled_posts.ts | 10 +- .../scheduled_messages.spec.ts | 128 ++++++++- server/channels/api4/scheduled_post.go | 28 +- server/channels/api4/scheduled_post_test.go | 269 +++++++++++++++++- server/channels/app/scheduled_post.go | 26 ++ server/channels/app/scheduled_post_job.go | 138 +++++++-- .../channels/app/scheduled_post_job_test.go | 223 +++++++++++++++ server/channels/app/scheduled_post_test.go | 4 +- server/channels/db/migrations/migrations.list | 4 + ...212_add_scheduled_post_recurrence.down.sql | 2 + ...00212_add_scheduled_post_recurrence.up.sql | 2 + ..._add_scheduled_post_pending_index.down.sql | 2 + ...13_add_scheduled_post_pending_index.up.sql | 4 + .../channels/store/retrylayer/retrylayer.go | 21 ++ .../store/sqlstore/scheduled_post_store.go | 112 +++++--- server/channels/store/store.go | 1 + .../storetest/mocks/ScheduledPostStore.go | 18 ++ .../store/storetest/scheduled_post_store.go | 243 ++++++++++++++++ .../channels/store/timerlayer/timerlayer.go | 16 ++ server/i18n/en.json | 20 ++ server/public/model/feature_flags.go | 5 + server/public/model/feature_flags_test.go | 11 + server/public/model/scheduled_post.go | 56 +++- .../public/model/scheduled_post_recurrence.go | 40 +++ .../model/scheduled_post_recurrence_test.go | 82 ++++++ server/public/model/scheduled_post_test.go | 129 +++++++++ .../channels/src/actions/websocket_actions.ts | 4 +- .../advanced_text_editor.tsx | 3 +- .../scheduled_post_indicator.tsx | 22 +- .../scheduled_post_custom_time_modal.scss | 24 ++ .../scheduled_post_custom_time_modal.test.tsx | 92 ++++++ .../scheduled_post_custom_time_modal.tsx | 78 ++++- .../send_button/send_button.tsx | 4 +- .../send_button/send_post_options/index.tsx | 12 +- .../draft_actions/draft_actions.test.tsx | 1 + .../drafts/draft_actions/draft_actions.tsx | 9 +- .../scheduled_post_actions.tsx | 20 +- .../src/components/drafts/draft_row.tsx | 22 +- .../components/drafts/panel/panel_header.tsx | 14 + webapp/channels/src/i18n/en.json | 3 + .../src/actions/scheduled_posts.ts | 5 +- .../reducers/entities/scheduled_posts.test.ts | 88 ++++++ .../src/reducers/entities/scheduled_posts.ts | 85 +++--- .../entities/scheduled_posts.test.ts | 147 ++++++++++ .../src/selectors/entities/scheduled_posts.ts | 49 +++- webapp/channels/src/types/store/draft.ts | 4 + webapp/platform/types/src/config.ts | 1 + webapp/platform/types/src/schedule_post.ts | 10 + 55 files changed, 2157 insertions(+), 192 deletions(-) create mode 100644 server/channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.down.sql create mode 100644 server/channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.up.sql create mode 100644 server/channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.down.sql create mode 100644 server/channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.up.sql create mode 100644 server/public/model/scheduled_post_recurrence.go create mode 100644 server/public/model/scheduled_post_recurrence_test.go create mode 100644 webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.scss create mode 100644 webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.test.tsx create mode 100644 webapp/channels/src/packages/mattermost-redux/src/reducers/entities/scheduled_posts.test.ts create mode 100644 webapp/channels/src/packages/mattermost-redux/src/selectors/entities/scheduled_posts.test.ts diff --git a/api/v4/source/definitions.yaml b/api/v4/source/definitions.yaml index b8c1ed2a0ea1..b5d512632e09 100644 --- a/api/v4/source/definitions.yaml +++ b/api/v4/source/definitions.yaml @@ -4732,6 +4732,14 @@ components: error_code: type: string description: Explains the error behind why a scheduled post could not have been sent + repeat_type: + type: string + description: > + Empty for a one-time schedule, or `weekly` for a recurring weekly schedule in `repeat_timezone`. + repeat_timezone: + type: string + description: > + IANA timezone name (for example `America/New_York`) used to interpret weekly recurrence; required when `repeat_type` is `weekly`. metadata: $ref: "#/components/schemas/PostMetadata" AccessControlFieldsAutocompleteResponse: diff --git a/api/v4/source/scheduled_post.yaml b/api/v4/source/scheduled_post.yaml index 14f2b4cd3c00..519dc93aa6a5 100644 --- a/api/v4/source/scheduled_post.yaml +++ b/api/v4/source/scheduled_post.yaml @@ -42,6 +42,12 @@ props: description: A general JSON property bag to attach to the post type: object + repeat_type: + type: string + description: Set to `weekly` for a recurring weekly schedule, or omit for a one-time schedule + repeat_timezone: + type: string + description: IANA timezone for weekly recurrence; required when `repeat_type` is `weekly` responses: "200": description: Created scheduled post @@ -150,6 +156,12 @@ message: type: string description: The message contents, can be formatted with Markdown + repeat_type: + type: string + description: Set to `weekly` for a recurring weekly schedule, or empty for a one-time schedule + repeat_timezone: + type: string + description: IANA timezone for weekly recurrence; required when `repeat_type` is `weekly` responses: "200": description: Updated scheduled post diff --git a/e2e-tests/.ci/server.generate.sh b/e2e-tests/.ci/server.generate.sh index 377da33d1d5c..6dc5624e64c5 100755 --- a/e2e-tests/.ci/server.generate.sh +++ b/e2e-tests/.ci/server.generate.sh @@ -73,6 +73,7 @@ services: MM_FEATUREFLAGS_PROPERTYFIELDRANK: "true" MM_FEATUREFLAGS_ATTRIBUTEVALUEMASKING: "true" MM_FEATUREFLAGS_WYSIWYGEDITOR: "true" + MM_FEATUREFLAGS_RECURRINGSCHEDULEDPOSTS: "true" MM_LOGSETTINGS_ENABLEDIAGNOSTICS: "false" MM_LOGSETTINGS_CONSOLELEVEL: "DEBUG" network_mode: host diff --git a/e2e-tests/playwright/lib/src/containers/env_baseline.ts b/e2e-tests/playwright/lib/src/containers/env_baseline.ts index e288d8f85631..1419149a980e 100644 --- a/e2e-tests/playwright/lib/src/containers/env_baseline.ts +++ b/e2e-tests/playwright/lib/src/containers/env_baseline.ts @@ -22,6 +22,7 @@ export const SERVER_ENV_BASELINE: Record = { MM_FEATUREFLAGS_MOVETHREADSENABLED: 'true', MM_FEATUREFLAGS_PERMISSIONPOLICIES: 'true', MM_FEATUREFLAGS_PROPERTYFIELDRANK: 'true', + MM_FEATUREFLAGS_RECURRINGSCHEDULEDPOSTS: 'true', MM_FEATUREFLAGS_TEAMMEMBERSHIPACCESSCONTROL: 'true', MM_FEATUREFLAGS_WYSIWYGEDITOR: 'true', }; diff --git a/e2e-tests/playwright/lib/src/ui/components/channels/schedule_message_modal.ts b/e2e-tests/playwright/lib/src/ui/components/channels/schedule_message_modal.ts index 61668ff2159e..54e381e96b54 100644 --- a/e2e-tests/playwright/lib/src/ui/components/channels/schedule_message_modal.ts +++ b/e2e-tests/playwright/lib/src/ui/components/channels/schedule_message_modal.ts @@ -9,6 +9,7 @@ export default class ScheduleMessageModal { readonly dateButton: Locator; readonly timeButton: Locator; readonly timeOptionDropdown: Locator; + readonly repeatWeeklyCheckbox: Locator; readonly closeButton: Locator; readonly scheduleButton: Locator; readonly cancelButton: Locator; @@ -18,6 +19,7 @@ export default class ScheduleMessageModal { this.dateButton = container.getByRole('button', {name: /Date/}); this.timeButton = container.getByTestId('time_button'); this.timeOptionDropdown = container.getByLabel('Choose a time'); + this.repeatWeeklyCheckbox = container.getByRole('checkbox', {name: 'Repeat weekly'}); this.closeButton = container.getByRole('button', {name: 'Close'}); this.scheduleButton = container.getByRole('button', {name: 'Schedule'}); this.cancelButton = container.getByRole('button', {name: 'Cancel'}); @@ -98,9 +100,21 @@ export default class ScheduleMessageModal { return text; } - async scheduleMessage(dayFromToday: number = 0, timeOptionIndex: number = 0) { + async setRepeatWeekly(enabled: boolean) { + const isChecked = await this.repeatWeeklyCheckbox.isChecked(); + + if (isChecked !== enabled) { + await this.repeatWeeklyCheckbox.click(); + } + } + + async scheduleMessage(dayFromToday: number = 0, timeOptionIndex: number = 0, repeatWeekly?: boolean) { await this.toBeVisible(); + if (typeof repeatWeekly === 'boolean') { + await this.setRepeatWeekly(repeatWeekly); + } + const selectedDate = await this.selectDate(dayFromToday); const fromDateButtonText = (await this.dateButton.textContent()) ?? ''; diff --git a/e2e-tests/playwright/lib/src/ui/components/channels/scheduled_post.ts b/e2e-tests/playwright/lib/src/ui/components/channels/scheduled_post.ts index 4301f0be67c6..3f0137c711b8 100644 --- a/e2e-tests/playwright/lib/src/ui/components/channels/scheduled_post.ts +++ b/e2e-tests/playwright/lib/src/ui/components/channels/scheduled_post.ts @@ -9,6 +9,7 @@ export default class ScheduledPost { readonly panelHeader; readonly panelBody; + readonly repeatsWeeklyTag; readonly postBody; readonly postHeader; @@ -29,6 +30,7 @@ export default class ScheduledPost { this.panelHeader = container.getByTestId('draft-panel-header'); this.panelBody = container.getByTestId('draft-panel-body'); + this.repeatsWeeklyTag = container.getByText('Repeats weekly', {exact: true}); this.postBody = container.getByTestId('draft-post-body'); this.postHeader = container.getByTestId('draft-post-header'); diff --git a/e2e-tests/playwright/lib/src/ui/pages/channels.ts b/e2e-tests/playwright/lib/src/ui/pages/channels.ts index fb26c24e949c..96c0feb4148f 100644 --- a/e2e-tests/playwright/lib/src/ui/pages/channels.ts +++ b/e2e-tests/playwright/lib/src/ui/pages/channels.ts @@ -443,7 +443,12 @@ export default class ChannelsPage { return popover; } - async scheduleMessage(message: string, dayFromToday: number = 0, timeOptionIndex: number = 0) { + async scheduleMessage( + message: string, + dayFromToday: number = 0, + timeOptionIndex: number = 0, + repeatWeekly?: boolean, + ) { await this.centerView.postCreate.writeMessage(message); await expect(this.centerView.postCreate.scheduleMessageButton).toBeVisible(); @@ -452,10 +457,15 @@ export default class ChannelsPage { await this.scheduleMessageMenu.toBeVisible(); await this.scheduleMessageMenu.selectCustomTime(); - return this.scheduleMessageModal.scheduleMessage(dayFromToday, timeOptionIndex); + return this.scheduleMessageModal.scheduleMessage(dayFromToday, timeOptionIndex, repeatWeekly); } - async scheduleMessageFromThread(message: string, dayFromToday: number = 0, timeOptionIndex: number = 0) { + async scheduleMessageFromThread( + message: string, + dayFromToday: number = 0, + timeOptionIndex: number = 0, + repeatWeekly?: boolean, + ) { await this.sidebarRight.postCreate.writeMessage(message); await expect(this.sidebarRight.postCreate.scheduleMessageButton).toBeVisible(); @@ -464,7 +474,7 @@ export default class ChannelsPage { await this.scheduleMessageMenu.toBeVisible(); await this.scheduleMessageMenu.selectCustomTime(); - return this.scheduleMessageModal.scheduleMessage(dayFromToday, timeOptionIndex); + return this.scheduleMessageModal.scheduleMessage(dayFromToday, timeOptionIndex, repeatWeekly); } async getFlaggedPostViewDetailButton(flaggedPostId: string) { diff --git a/e2e-tests/playwright/lib/src/ui/pages/scheduled_posts.ts b/e2e-tests/playwright/lib/src/ui/pages/scheduled_posts.ts index 6024ddb0a8df..7a034d5fe6a1 100644 --- a/e2e-tests/playwright/lib/src/ui/pages/scheduled_posts.ts +++ b/e2e-tests/playwright/lib/src/ui/pages/scheduled_posts.ts @@ -67,12 +67,18 @@ export default class ScheduledPostsPage { return new components.ScheduledPost(nthPost); } - async rescheduleMessage(post: ScheduledPost, dayFromToday: number = 0, timeOptionIndex: number = 0) { + async openRescheduleMessageModal(post: ScheduledPost) { await post.hover(); await expect(post.rescheduleButton).toBeVisible(); await post.rescheduleButton.click(); + await this.scheduleMessageModal.toBeVisible(); + + return this.scheduleMessageModal; + } - return this.scheduleMessageModal.scheduleMessage(dayFromToday, timeOptionIndex); + async rescheduleMessage(post: ScheduledPost, dayFromToday: number = 0, timeOptionIndex: number = 0) { + const scheduleMessageModal = await this.openRescheduleMessageModal(post); + return scheduleMessageModal.scheduleMessage(dayFromToday, timeOptionIndex); } async goto(teamName: string) { diff --git a/e2e-tests/playwright/specs/functional/channels/scheduled_messages/scheduled_messages.spec.ts b/e2e-tests/playwright/specs/functional/channels/scheduled_messages/scheduled_messages.spec.ts index 16c7dc90c9c3..d817d20f4282 100644 --- a/e2e-tests/playwright/specs/functional/channels/scheduled_messages/scheduled_messages.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/scheduled_messages/scheduled_messages.spec.ts @@ -68,6 +68,52 @@ test.fixme( }, ); +/** + * @objective Verify that a weekly recurring scheduled message does not pin the channel indicator above the composer. + * + * @precondition + * A test server with valid license and the RecurringScheduledPosts feature flag enabled + */ +test( + 'creates weekly recurring scheduled message from channel without showing the channel indicator', + {tag: '@scheduled_messages'}, + async ({pw}) => { + await pw.skipIfFeatureFlagNotSet('RecurringScheduledPosts', true); + + const draftMessage = `Weekly Scheduled Draft ${pw.random.id()}`; + + // # Initialize test user, login and navigate to a channel + const {user, team} = await pw.initSetup(); + const {channelsPage, scheduledPostsPage} = await pw.testBrowser.login(user); + await channelsPage.goto(); + await channelsPage.toBeVisible(); + + // # Create a weekly recurring scheduled message for tomorrow + const {selectedDate, selectedTime} = await channelsPage.scheduleMessage(draftMessage, 1, 0, true); + + // * Verify scheduled post badge appears with count of 1 + await verifyScheduledPostBadgeOnLeftSidebar(channelsPage, 1); + + // * Verify the channel indicator stays hidden since every scheduled post is recurring + await channelsPage.centerView.scheduledPostIndicator.toBeNotVisible(); + + // # Navigate to scheduled posts page + await scheduledPostsPage.goto(team.name); + + // * Verify scheduled post appears with recurring weekly details + const scheduledPost = await verifyScheduledPost(scheduledPostsPage, { + draftMessage, + selectedDate, + selectedTime, + badgeCountOnTab: 1, + repeatWeekly: true, + }); + + // * Verify one-time send now control is not available for recurring scheduled posts + await expect(scheduledPost.sendNowButton).toHaveCount(0); + }, +); + /** * @objective Verify the ability to create a scheduled message in a thread. * @@ -182,6 +228,70 @@ test( }, ); +/** + * @objective Verify rescheduling a weekly recurring scheduled message keeps its weekly recurrence. + * + * @precondition + * A test server with valid license and the RecurringScheduledPosts feature flag enabled + */ +test( + 'reschedules weekly recurring scheduled message from scheduled posts page and keeps weekly recurrence', + {tag: '@scheduled_messages'}, + async ({pw}) => { + await pw.skipIfFeatureFlagNotSet('RecurringScheduledPosts', true); + + const draftMessage = `Weekly Scheduled Draft ${pw.random.id()}`; + + // # Initialize test user, login and navigate to a channel + const {user, team} = await pw.initSetup(); + const {channelsPage, scheduledPostsPage} = await pw.testBrowser.login(user); + await channelsPage.goto(); + await channelsPage.toBeVisible(); + + // # Create a weekly recurring scheduled message for tomorrow + const {selectedDate, selectedTime} = await channelsPage.scheduleMessage(draftMessage, 1, 0, true); + + // # Navigate to scheduled posts page + await scheduledPostsPage.goto(team.name); + + // * Verify scheduled post appears with recurring weekly details + let scheduledPost = await verifyScheduledPost(scheduledPostsPage, { + draftMessage, + selectedDate, + selectedTime, + badgeCountOnTab: 1, + repeatWeekly: true, + }); + + // # Open the reschedule modal for the recurring scheduled message + const scheduleMessageModal = await scheduledPostsPage.openRescheduleMessageModal(scheduledPost); + + // * Verify the weekly recurrence option remains selected in the reschedule modal + await expect(scheduleMessageModal.repeatWeeklyCheckbox).toBeChecked(); + + // # Reschedule the recurring message to a different future date + const {selectedDate: newSelectedDate, selectedTime: newSelectedTime} = + await scheduleMessageModal.scheduleMessage(2); + + // * Verify the rescheduled post still appears as a weekly recurring message + scheduledPost = await verifyScheduledPost(scheduledPostsPage, { + draftMessage, + selectedDate: newSelectedDate, + selectedTime: newSelectedTime, + badgeCountOnTab: 1, + repeatWeekly: true, + }); + await expect(scheduledPost.sendNowButton).toHaveCount(0); + + // # Return to channel page + await channelsPage.goto(); + + // * Verify the channel indicator stays hidden since every scheduled post is recurring + await verifyScheduledPostBadgeOnLeftSidebar(channelsPage, 1); + await channelsPage.centerView.scheduledPostIndicator.toBeNotVisible(); + }, +); + /** * @objective Verify the ability to delete a scheduled message. * @@ -577,11 +687,23 @@ async function verifyScheduledPost( selectedDate, selectedTime, badgeCountOnTab, - }: {draftMessage: string; selectedDate: string; selectedTime: string | null; badgeCountOnTab: number}, + repeatWeekly = false, + }: { + draftMessage: string; + selectedDate: string; + selectedTime: string | null; + badgeCountOnTab: number; + repeatWeekly?: boolean; + }, ) { // * Verify scheduled posts page is visible await scheduledPostsPage.toBeVisible(); + // Clear hover and focus (e.g. left behind by the reschedule modal); the panel hides its + // timestamp/tag info section while hovered or focused. + await scheduledPostsPage.page.mouse.move(0, 0); + await scheduledPostsPage.page.evaluate(() => (document.activeElement as HTMLElement | null)?.blur()); + // * Verify scheduled post badge on tab has correct count expect(await scheduledPostsPage.getBadgeCountOnTab()).toBe(badgeCountOnTab.toString()); @@ -610,5 +732,9 @@ async function verifyScheduledPost( ); } + if (repeatWeekly) { + await expect(scheduledPost.repeatsWeeklyTag).toBeVisible(); + } + return scheduledPost; } diff --git a/server/channels/api4/scheduled_post.go b/server/channels/api4/scheduled_post.go index 9a5693be2ad6..0b8e77846e13 100644 --- a/server/channels/api4/scheduled_post.go +++ b/server/channels/api4/scheduled_post.go @@ -5,6 +5,7 @@ package api4 import ( "encoding/json" + "io" "net/http" "github.com/gorilla/mux" @@ -177,12 +178,27 @@ func updateScheduledPost(c *Context, w http.ResponseWriter, r *http.Request) { return } - var scheduledPost model.ScheduledPost - if err := json.NewDecoder(r.Body).Decode(&scheduledPost); err != nil { + body, err := io.ReadAll(r.Body) + if err != nil { c.SetInvalidParamWithErr("schedule_post", err) return } + var scheduledPost model.ScheduledPost + if unmarshalErr := json.Unmarshal(body, &scheduledPost); unmarshalErr != nil { + c.SetInvalidParamWithErr("schedule_post", unmarshalErr) + return + } + + // Detect whether the payload included repeat_type at all. + var rawPayload struct { + RepeatType json.RawMessage `json:"repeat_type"` + } + if unmarshalErr := json.Unmarshal(body, &rawPayload); unmarshalErr != nil { + c.SetInvalidParamWithErr("schedule_post", unmarshalErr) + return + } + if scheduledPost.Id != scheduledPostId { c.SetInvalidURLParam("scheduled_post_id") return @@ -207,6 +223,14 @@ func updateScheduledPost(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Clients that predate recurring scheduled posts omit the repeat fields entirely, so an + // absent repeat_type preserves the existing recurrence rather than ending the series. + // Sending an explicit, empty repeat_type remains the way to stop repeating. + if rawPayload.RepeatType == nil { + scheduledPost.RepeatType = existingScheduledPost.RepeatType + scheduledPost.RepeatTimezone = existingScheduledPost.RepeatTimezone + } + if len(scheduledPost.FileIds) > 0 { originalPost, err := existingScheduledPost.ToPost() if err != nil { diff --git a/server/channels/api4/scheduled_post_test.go b/server/channels/api4/scheduled_post_test.go index 5c183decfd4b..e88ac6659350 100644 --- a/server/channels/api4/scheduled_post_test.go +++ b/server/channels/api4/scheduled_post_test.go @@ -5,6 +5,9 @@ package api4 import ( "context" + "encoding/json" + "fmt" + "net/http" "testing" "github.com/mattermost/mattermost/server/public/model" @@ -13,7 +16,9 @@ import ( func TestUpdateScheduledPost(t *testing.T) { mainHelper.Parallel(t) - th := Setup(t).InitBasic(t) + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.RecurringScheduledPosts = true + }).InitBasic(t) th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) @@ -53,6 +58,119 @@ func TestUpdateScheduledPost(t *testing.T) { require.Equal(t, originalMessage, fetchedPost.Message) require.Equal(t, originalScheduledAt, fetchedPost.ScheduledAt) }) + + t.Run("should clear error state when rescheduling an existing scheduled post", func(t *testing.T) { + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "weekly recurring scheduled post", + }, + ScheduledAt: model.GetMillis() + 100000, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + createdScheduledPost, _, err := th.Client.CreateScheduledPost(context.Background(), scheduledPost) + require.NoError(t, err) + require.NotNil(t, createdScheduledPost) + + createdScheduledPost.ErrorCode = model.ScheduledPostErrorUnableToSend + createdScheduledPost.ProcessedAt = model.GetMillis() + require.NoError(t, th.App.Srv().Store().ScheduledPost().UpdatedScheduledPost(createdScheduledPost)) + + createdScheduledPost.ScheduledAt = model.GetMillis() + 300000 + createdScheduledPost.RepeatTimezone = "America/New_York" + + updatedScheduledPost, _, err := th.Client.UpdateScheduledPost(context.Background(), createdScheduledPost) + require.NoError(t, err) + require.NotNil(t, updatedScheduledPost) + require.Empty(t, updatedScheduledPost.ErrorCode) + require.Zero(t, updatedScheduledPost.ProcessedAt) + require.Equal(t, model.ScheduledPostRepeatTypeWeekly, updatedScheduledPost.RepeatType) + require.Equal(t, "America/New_York", updatedScheduledPost.RepeatTimezone) + + fetchedPost, err := th.App.Srv().Store().ScheduledPost().Get(createdScheduledPost.Id) + require.NoError(t, err) + require.Equal(t, model.ScheduledPostRepeatTypeWeekly, fetchedPost.RepeatType) + require.Equal(t, "America/New_York", fetchedPost.RepeatTimezone) + }) + + t.Run("should preserve recurrence when the update omits the repeat fields", func(t *testing.T) { + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "weekly recurring scheduled post", + }, + ScheduledAt: model.GetMillis() + 100000, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "America/New_York", + } + createdScheduledPost, _, err := th.Client.CreateScheduledPost(context.Background(), scheduledPost) + require.NoError(t, err) + require.NotNil(t, createdScheduledPost) + + // Clients that predate recurring scheduled posts leave the repeat fields out of their + // update payloads. Marshalling a model.ScheduledPost always emits them, so the payload + // has to be built by hand to reproduce what those clients send. + payload := fmt.Sprintf( + `{"id":"%s","create_at":%d,"user_id":"%s","channel_id":"%s","message":"rescheduled by an old client","scheduled_at":%d}`, + createdScheduledPost.Id, + createdScheduledPost.CreateAt, + th.BasicUser.Id, + th.BasicChannel.Id, + model.GetMillis()+300000, + ) + + httpResp, err := th.Client.DoAPIPut(context.Background(), "/posts/schedule/"+createdScheduledPost.Id, payload) + require.NoError(t, err) + require.Equal(t, http.StatusCreated, httpResp.StatusCode) + + var updatedScheduledPost model.ScheduledPost + require.NoError(t, json.NewDecoder(httpResp.Body).Decode(&updatedScheduledPost)) + require.NoError(t, httpResp.Body.Close()) + require.Equal(t, model.ScheduledPostRepeatTypeWeekly, updatedScheduledPost.RepeatType) + require.Equal(t, "America/New_York", updatedScheduledPost.RepeatTimezone) + + fetchedPost, err := th.App.Srv().Store().ScheduledPost().Get(createdScheduledPost.Id) + require.NoError(t, err) + require.Equal(t, "rescheduled by an old client", fetchedPost.Message) + require.Equal(t, model.ScheduledPostRepeatTypeWeekly, fetchedPost.RepeatType) + require.Equal(t, "America/New_York", fetchedPost.RepeatTimezone) + }) + + t.Run("should stop recurrence when the update sends empty repeat fields", func(t *testing.T) { + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "weekly recurring scheduled post", + }, + ScheduledAt: model.GetMillis() + 100000, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "America/New_York", + } + createdScheduledPost, _, err := th.Client.CreateScheduledPost(context.Background(), scheduledPost) + require.NoError(t, err) + require.NotNil(t, createdScheduledPost) + + createdScheduledPost.RepeatType = model.ScheduledPostRepeatTypeNone + createdScheduledPost.RepeatTimezone = "" + + updatedScheduledPost, _, err := th.Client.UpdateScheduledPost(context.Background(), createdScheduledPost) + require.NoError(t, err) + require.NotNil(t, updatedScheduledPost) + require.Empty(t, updatedScheduledPost.RepeatType) + require.Empty(t, updatedScheduledPost.RepeatTimezone) + + fetchedPost, err := th.App.Srv().Store().ScheduledPost().Get(createdScheduledPost.Id) + require.NoError(t, err) + require.Empty(t, fetchedPost.RepeatType) + require.Empty(t, fetchedPost.RepeatTimezone) + }) } func TestDeleteScheduledPost(t *testing.T) { @@ -95,7 +213,9 @@ func TestDeleteScheduledPost(t *testing.T) { func TestCreateScheduledPost(t *testing.T) { mainHelper.Parallel(t) - th := Setup(t).InitBasic(t) + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.RecurringScheduledPosts = true + }).InitBasic(t) th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) @@ -152,4 +272,149 @@ func TestCreateScheduledPost(t *testing.T) { require.Contains(t, httpErr.Error(), "You do not have the appropriate permissions.") require.Nil(t, createdScheduledPost) }) + + t.Run("weekly recurring persists repeat fields", func(t *testing.T) { + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "weekly message", + }, + ScheduledAt: model.GetMillis() + 100000, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "America/New_York", + } + created, _, err := client.CreateScheduledPost(context.Background(), scheduledPost) + require.NoError(t, err) + require.NotNil(t, created) + require.Equal(t, model.ScheduledPostRepeatTypeWeekly, created.RepeatType) + require.Equal(t, "America/New_York", created.RepeatTimezone) + }) + + t.Run("weekly recurring rejects file attachments", func(t *testing.T) { + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "weekly message with a file", + FileIds: model.StringArray{model.NewId()}, + }, + ScheduledAt: model.GetMillis() + 100000, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "America/New_York", + } + created, resp, err := client.CreateScheduledPost(context.Background(), scheduledPost) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + require.Nil(t, created) + }) +} + +func TestScheduledPostRecurringFeatureFlag(t *testing.T) { + mainHelper.Parallel(t) + // SetupConfig also makes feature flags writable, so the subtests below can toggle the flag. + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.RecurringScheduledPosts = false + }).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + + setFlag := func(t *testing.T, enabled bool) { + t.Helper() + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.FeatureFlags.RecurringScheduledPosts = enabled + }) + } + + newScheduledPost := func(repeatType string) *model.ScheduledPost { + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "recurring feature flag scheduled post", + }, + ScheduledAt: model.GetMillis() + 100000, + RepeatType: repeatType, + } + if repeatType == model.ScheduledPostRepeatTypeWeekly { + scheduledPost.RepeatTimezone = "UTC" + } + return scheduledPost + } + + t.Run("creating a recurring scheduled post is rejected when the flag is off", func(t *testing.T) { + setFlag(t, false) + + created, resp, err := th.Client.CreateScheduledPost(context.Background(), newScheduledPost(model.ScheduledPostRepeatTypeWeekly)) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + CheckErrorID(t, err, "app.scheduled_post.recurring_disabled.app_error") + require.Nil(t, created) + }) + + t.Run("creating a one-shot scheduled post is still allowed when the flag is off", func(t *testing.T) { + setFlag(t, false) + + created, _, err := th.Client.CreateScheduledPost(context.Background(), newScheduledPost(model.ScheduledPostRepeatTypeNone)) + require.NoError(t, err) + require.NotNil(t, created) + }) + + t.Run("creating a recurring scheduled post succeeds when the flag is on", func(t *testing.T) { + setFlag(t, true) + + created, _, err := th.Client.CreateScheduledPost(context.Background(), newScheduledPost(model.ScheduledPostRepeatTypeWeekly)) + require.NoError(t, err) + require.NotNil(t, created) + require.Equal(t, model.ScheduledPostRepeatTypeWeekly, created.RepeatType) + }) + + t.Run("converting a recurring scheduled post to one-shot is allowed when the flag is off", func(t *testing.T) { + setFlag(t, true) + created, _, err := th.Client.CreateScheduledPost(context.Background(), newScheduledPost(model.ScheduledPostRepeatTypeWeekly)) + require.NoError(t, err) + + setFlag(t, false) + created.RepeatType = model.ScheduledPostRepeatTypeNone + created.RepeatTimezone = "" + + updated, _, err := th.Client.UpdateScheduledPost(context.Background(), created) + require.NoError(t, err) + require.Equal(t, model.ScheduledPostRepeatTypeNone, updated.RepeatType) + }) + + t.Run("editing an existing recurring scheduled post keeps repeating when the flag is off", func(t *testing.T) { + setFlag(t, true) + created, _, err := th.Client.CreateScheduledPost(context.Background(), newScheduledPost(model.ScheduledPostRepeatTypeWeekly)) + require.NoError(t, err) + + setFlag(t, false) + created.Message = "updated message for an existing weekly series" + + updated, _, err := th.Client.UpdateScheduledPost(context.Background(), created) + require.NoError(t, err) + require.Equal(t, "updated message for an existing weekly series", updated.Message) + require.Equal(t, model.ScheduledPostRepeatTypeWeekly, updated.RepeatType) + }) + + t.Run("converting a one-shot scheduled post to recurring is rejected when the flag is off", func(t *testing.T) { + setFlag(t, false) + created, _, err := th.Client.CreateScheduledPost(context.Background(), newScheduledPost(model.ScheduledPostRepeatTypeNone)) + require.NoError(t, err) + + created.RepeatType = model.ScheduledPostRepeatTypeWeekly + created.RepeatTimezone = "UTC" + + _, resp, err := th.Client.UpdateScheduledPost(context.Background(), created) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + CheckErrorID(t, err, "app.scheduled_post.recurring_disabled.app_error") + + fetched, storeErr := th.App.Srv().Store().ScheduledPost().Get(created.Id) + require.NoError(t, storeErr) + require.Equal(t, model.ScheduledPostRepeatTypeNone, fetched.RepeatType) + }) } diff --git a/server/channels/app/scheduled_post.go b/server/channels/app/scheduled_post.go index 687dc15b0c74..706cf7c25831 100644 --- a/server/channels/app/scheduled_post.go +++ b/server/channels/app/scheduled_post.go @@ -12,6 +12,17 @@ import ( "github.com/mattermost/mattermost/server/public/shared/request" ) +// recurringScheduledPostsEnabled gates turning recurrence on. The api4 routes already enforce +// the ScheduledPosts setting and license for every scheduled post request, and the job keeps +// sending existing recurring series regardless of the flag. +func (a *App) recurringScheduledPostsEnabled() bool { + return a.Config().FeatureFlags.RecurringScheduledPosts +} + +func recurringScheduledPostsDisabledError(where string) *model.AppError { + return model.NewAppError(where, "app.scheduled_post.recurring_disabled.app_error", nil, "", http.StatusBadRequest) +} + func (a *App) SaveScheduledPost(rctx request.CTX, scheduledPost *model.ScheduledPost, connectionId string) (*model.ScheduledPost, *model.AppError) { maxMessageLength := a.Srv().Store().ScheduledPost().GetMaxMessageSize() scheduledPost.PreSave() @@ -19,6 +30,10 @@ func (a *App) SaveScheduledPost(rctx request.CTX, scheduledPost *model.Scheduled return nil, validationErr } + if scheduledPost.RepeatType != model.ScheduledPostRepeatTypeNone && !a.recurringScheduledPostsEnabled() { + return nil, recurringScheduledPostsDisabledError("App.SaveScheduledPost") + } + // validate the channel is not archived channel, appErr := a.GetChannel(rctx, scheduledPost.ChannelId) if appErr != nil { @@ -89,9 +104,20 @@ func (a *App) UpdateScheduledPost(rctx request.CTX, userId string, scheduledPost return nil, model.NewAppError("app.UpdateScheduledPost", "app.update_scheduled_post.existing_scheduled_post.not_exist", map[string]any{"user_id": userId, "scheduled_post_id": scheduledPost.Id}, "", http.StatusNotFound) } + // Only turning recurrence ON is blocked while the flag is off; editing or rescheduling an + // already-recurring post and turning recurrence off must stay possible so users can manage + // series created while the flag was on. + if scheduledPost.RepeatType != model.ScheduledPostRepeatTypeNone && + existingScheduledPost.RepeatType == model.ScheduledPostRepeatTypeNone && + !a.recurringScheduledPostsEnabled() { + return nil, recurringScheduledPostsDisabledError("App.UpdateScheduledPost") + } + // This step is not required for update but is useful as we want to return the // updated scheduled post. It's better to do this before calling update than after. scheduledPost.RestoreNonUpdatableFields(existingScheduledPost) + scheduledPost.ErrorCode = "" + scheduledPost.ProcessedAt = 0 var appErr *model.AppError scheduledPost, appErr = a.runGuardedScheduledPostWillBeCreated(rctx, scheduledPost, "UpdateScheduledPost", func(reason string) *model.AppError { diff --git a/server/channels/app/scheduled_post_job.go b/server/channels/app/scheduled_post_job.go index 8e64e1f2b281..7d1e93ad60e4 100644 --- a/server/channels/app/scheduled_post_job.go +++ b/server/channels/app/scheduled_post_job.go @@ -5,6 +5,7 @@ package app import ( "context" + stderrors "errors" "fmt" "net/http" "strings" @@ -25,6 +26,8 @@ const ( func (a *App) ProcessScheduledPosts(rctx request.CTX) { rctx = rctx.WithLogFields(mlog.String("component", "scheduled_post_job")) + // Intentionally not gated on FeatureFlags.RecurringScheduledPosts: existing recurring rows + // keep sending and advancing even after the flag is turned off. if !*a.Config().ServiceSettings.ScheduledPosts { return } @@ -92,32 +95,82 @@ func (a *App) ProcessScheduledPosts(rctx request.CTX) { // once all scheduled posts are processed, we need to update and close the old ones // as we don't process pending scheduled posts more than 24 hours old. - if err := a.Srv().Store().ScheduledPost().UpdateOldScheduledPosts(beforeTime); err != nil { + if err := a.Srv().Store().ScheduledPost().UpdateOldScheduledPosts(afterTime); err != nil { rctx.Logger().Error( "App.ProcessScheduledPosts: failed to update old scheduled posts", - mlog.Int("before_time", beforeTime), + mlog.Int("cutoff_time", afterTime), mlog.Err(err), ) } } +// scheduledPostDisposition is the explicit outcome of attempting to send one scheduled post, +// so the batch loop never has to infer it from error/error-code combinations. +type scheduledPostDisposition int + +const ( + // scheduledPostPosted: the message was created; one-shots are done and series advance. + scheduledPostPosted scheduledPostDisposition = iota + + // scheduledPostFailed: the attempt failed; the post keeps its error code so the user can + // see and fix it, and any series stops until rescheduled. + scheduledPostFailed + + // scheduledPostUnsendable: the destination channel no longer exists, so the post — and any + // series — can never send and is permanently deleted. + scheduledPostUnsendable +) + // processScheduledPostBatch processes one batch func (a *App) processScheduledPostBatch(rctx request.CTX, scheduledPosts []*model.ScheduledPost) error { var failedScheduledPosts []*model.ScheduledPost - var successfulScheduledPostIDs []string + var completedScheduledPosts []*model.ScheduledPost + var recurringScheduledPosts []*model.ScheduledPost + now := model.GetMillis() for i := range scheduledPosts { - scheduledPost, err := a.postScheduledPost(rctx, scheduledPosts[i]) + scheduledPost := scheduledPosts[i] + + switch disposition, err := a.postScheduledPost(rctx, scheduledPost); disposition { + case scheduledPostFailed: + rctx.Logger().Error("processScheduledPostBatch scheduled post processing failed", mlog.String("scheduled_post_id", scheduledPost.Id), mlog.Err(err)) + failedScheduledPosts = append(failedScheduledPosts, scheduledPost) + continue + case scheduledPostUnsendable: + completedScheduledPosts = append(completedScheduledPosts, scheduledPost) + continue + case scheduledPostPosted: + default: + // An unhandled disposition is a bug; never delete on a guess. Fail the post so the + // user is told and the row survives. + rctx.Logger().Error("processScheduledPostBatch unhandled scheduled post disposition", mlog.Int("disposition", int(disposition)), mlog.String("scheduled_post_id", scheduledPost.Id)) + scheduledPost.ErrorCode = model.ScheduledPostErrorUnknownError + failedScheduledPosts = append(failedScheduledPosts, scheduledPost) + continue + } + + if !scheduledPost.IsRecurring() { + completedScheduledPosts = append(completedScheduledPosts, scheduledPost) + continue + } + + nextScheduledAt, err := scheduledPost.ComputeNextScheduledAt(now) if err != nil { - rctx.Logger().Error("processScheduledPostBatch scheduled post processing failed", mlog.String("scheduled_post_id", scheduledPosts[i].Id), mlog.Err(err)) + // Fail the series loudly rather than leaving it due, which would repost it on every job run. + rctx.Logger().Error("processScheduledPostBatch failed to compute next occurrence of recurring scheduled post", mlog.String("scheduled_post_id", scheduledPost.Id), mlog.Err(err)) + scheduledPost.ErrorCode = model.ScheduledPostErrorUnableToSend failedScheduledPosts = append(failedScheduledPosts, scheduledPost) continue } - successfulScheduledPostIDs = append(successfulScheduledPostIDs, scheduledPost.Id) + // The store resets these columns too; mirroring it here keeps the WS payload consistent. + scheduledPost.ScheduledAt = nextScheduledAt + scheduledPost.ErrorCode = "" + scheduledPost.ProcessedAt = 0 + recurringScheduledPosts = append(recurringScheduledPosts, scheduledPost) } - if err := a.handleSuccessfulScheduledPosts(rctx, successfulScheduledPostIDs); err != nil { + if err := a.handleSuccessfulScheduledPosts(rctx, completedScheduledPosts, recurringScheduledPosts); err != nil { return errors.Wrap(err, "App.processScheduledPostBatch: failed to handle successfully posted scheduled posts") } @@ -125,8 +178,9 @@ func (a *App) processScheduledPostBatch(rctx request.CTX, scheduledPosts []*mode return nil } -// postScheduledPost processes an individual scheduled post -func (a *App) postScheduledPost(rctx request.CTX, scheduledPost *model.ScheduledPost) (*model.ScheduledPost, error) { +// postScheduledPost attempts to send an individual scheduled post and returns an explicit +// disposition. Failed dispositions set the post's ErrorCode for persistence and notification. +func (a *App) postScheduledPost(rctx request.CTX, scheduledPost *model.ScheduledPost) (scheduledPostDisposition, error) { // we'll process scheduled posts one by one. // If an error occurs, we'll log it and move onto the next scheduled post @@ -136,7 +190,7 @@ func (a *App) postScheduledPost(rctx request.CTX, scheduledPost *model.Scheduled rctx.Logger().Warn("channel for scheduled post not found, setting error code", mlog.String("scheduled_post_id", scheduledPost.Id), mlog.String("channel_id", scheduledPost.ChannelId), mlog.String("error_code", model.ScheduledPostErrorCodeChannelNotFound), mlog.Err(appErr)) scheduledPost.ErrorCode = model.ScheduledPostErrorCodeChannelNotFound - return scheduledPost, nil + return scheduledPostUnsendable, nil } rctx.Logger().Error( @@ -148,7 +202,7 @@ func (a *App) postScheduledPost(rctx request.CTX, scheduledPost *model.Scheduled ) scheduledPost.ErrorCode = model.ScheduledPostErrorUnknownError - return scheduledPost, appErr + return scheduledPostFailed, appErr } errorCode, err := a.canPostScheduledPost(rctx, scheduledPost, channel) @@ -162,7 +216,7 @@ func (a *App) postScheduledPost(rctx request.CTX, scheduledPost *model.Scheduled mlog.Err(err), ) - return scheduledPost, err + return scheduledPostFailed, err } if scheduledPost.ErrorCode != "" { @@ -174,7 +228,7 @@ func (a *App) postScheduledPost(rctx request.CTX, scheduledPost *model.Scheduled mlog.String("error_code", scheduledPost.ErrorCode), ) - return scheduledPost, fmt.Errorf("App.processScheduledPostBatch: skipping posting a scheduled post as `can post` check failed, error_code: %s", scheduledPost.ErrorCode) + return scheduledPostFailed, fmt.Errorf("App.processScheduledPostBatch: skipping posting a scheduled post as `can post` check failed, error_code: %s", scheduledPost.ErrorCode) } post, err := scheduledPost.ToPost() @@ -187,7 +241,7 @@ func (a *App) postScheduledPost(rctx request.CTX, scheduledPost *model.Scheduled ) scheduledPost.ErrorCode = model.ScheduledPostErrorUnknownError - return scheduledPost, err + return scheduledPostFailed, err } _, _, appErr = a.CreatePost(rctx.WithContext(context.WithValue(rctx.Context(), model.PostContextKeyIsScheduledPost, true)), post, channel, model.CreatePostFlags{ @@ -204,13 +258,10 @@ func (a *App) postScheduledPost(rctx request.CTX, scheduledPost *model.Scheduled ) scheduledPost.ErrorCode = model.ScheduledPostErrorUnknownError - return scheduledPost, appErr + return scheduledPostFailed, appErr } - // send the WS event to delete the just posted scheduledPost from list - a.PublishScheduledPostEvent(rctx, model.WebsocketScheduledPostDeleted, scheduledPost, "") - - return scheduledPost, nil + return scheduledPostPosted, nil } // canPostScheduledPost checks whether the scheduled post be created based on permissions and other checks. @@ -342,26 +393,55 @@ func (a *App) canPostScheduledPost(rctx request.CTX, scheduledPost *model.Schedu return "", nil } -func (a *App) handleSuccessfulScheduledPosts(rctx request.CTX, successfulScheduledPostIDs []string) error { - if len(successfulScheduledPostIDs) > 0 { - // Successfully posted scheduled posts can be safely permanently deleted as no data is lost. - // The data is moved into the posts table. - err := a.Srv().Store().ScheduledPost().PermanentlyDeleteScheduledPosts(successfulScheduledPostIDs) - if err != nil { +// handleSuccessfulScheduledPosts advances recurring scheduled posts to their next occurrence and +// permanently deletes completed ones. The two operations touch disjoint rows and run +// independently, so a store failure in one can't cause reposts in the other. +func (a *App) handleSuccessfulScheduledPosts(rctx request.CTX, completedScheduledPosts, recurringScheduledPosts []*model.ScheduledPost) error { + var errs []error + + if len(recurringScheduledPosts) > 0 { + if err := a.Srv().Store().ScheduledPost().UpdateRecurringScheduledPosts(recurringScheduledPosts); err != nil { rctx.Logger().Error( - "App.handleSuccessfulScheduledPosts: failed to delete successfully posted scheduled posts", - mlog.Int("successfully_posted_count", len(successfulScheduledPostIDs)), + "App.handleSuccessfulScheduledPosts: failed to advance recurring scheduled posts", + mlog.Int("recurring_scheduled_post_count", len(recurringScheduledPosts)), mlog.Err(err), ) - return errors.Wrap(err, "App.handleSuccessfulScheduledPosts: failed to delete successfully posted scheduled posts") + errs = append(errs, errors.Wrap(err, "App.handleSuccessfulScheduledPosts: failed to advance recurring scheduled posts")) + } else { + for _, sp := range recurringScheduledPosts { + a.PublishScheduledPostEvent(rctx, model.WebsocketScheduledPostUpdated, sp, "") + } } } - return nil + if len(completedScheduledPosts) > 0 { + // Completed scheduled posts can be safely permanently deleted: posted ones have their + // data moved into the posts table, and ones whose channel is gone can never send. + toDelete := make([]string, len(completedScheduledPosts)) + for i, sp := range completedScheduledPosts { + toDelete[i] = sp.Id + } + + if err := a.Srv().Store().ScheduledPost().PermanentlyDeleteScheduledPosts(toDelete); err != nil { + rctx.Logger().Error( + "App.handleSuccessfulScheduledPosts: failed to delete completed scheduled posts", + mlog.Int("completed_count", len(toDelete)), + mlog.Err(err), + ) + errs = append(errs, errors.Wrap(err, "App.handleSuccessfulScheduledPosts: failed to delete completed scheduled posts")) + } else { + for _, sp := range completedScheduledPosts { + a.PublishScheduledPostEvent(rctx, model.WebsocketScheduledPostDeleted, sp, "") + } + } + } + + return stderrors.Join(errs...) } func (a *App) handleFailedScheduledPosts(rctx request.CTX, failedScheduledPosts []*model.ScheduledPost) { for _, failedScheduledPost := range failedScheduledPosts { + failedScheduledPost.ProcessedAt = model.GetMillis() err := a.Srv().Store().ScheduledPost().UpdatedScheduledPost(failedScheduledPost) if err != nil { // we intentionally don't stop on error as its possible to continue updating other scheduled posts diff --git a/server/channels/app/scheduled_post_job_test.go b/server/channels/app/scheduled_post_job_test.go index d422dd751517..abae0ce3b1b0 100644 --- a/server/channels/app/scheduled_post_job_test.go +++ b/server/channels/app/scheduled_post_job_test.go @@ -11,6 +11,7 @@ import ( "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/i18n" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestProcessScheduledPosts(t *testing.T) { @@ -54,6 +55,228 @@ func TestProcessScheduledPosts(t *testing.T) { assert.Len(t, scheduledPosts, 0) }) + t.Run("advances weekly recurring scheduled post instead of deleting", func(t *testing.T) { + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + + scheduledAt := model.GetMillis() - 1000 + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "weekly recurring scheduled post", + }, + ScheduledAt: scheduledAt, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + created, err := th.Server.Store().ScheduledPost().CreateScheduledPost(scheduledPost) + assert.NoError(t, err) + require.NotNil(t, created) + + th.App.ProcessScheduledPosts(th.Context) + + updated, err := th.Server.Store().ScheduledPost().Get(created.Id) + assert.NoError(t, err) + require.NotNil(t, updated) + assert.Equal(t, model.ScheduledPostRepeatTypeWeekly, updated.RepeatType) + assert.Equal(t, "UTC", updated.RepeatTimezone) + assert.Empty(t, updated.ErrorCode) + assert.Zero(t, updated.ProcessedAt) + + const weekMs = int64(7 * 24 * 60 * 60 * 1000) + assert.InDelta(t, scheduledAt+weekMs, updated.ScheduledAt, float64(60*1000)) + }) + + t.Run("advances multiple weekly recurring scheduled posts", func(t *testing.T) { + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + + scheduledAt := model.GetMillis() - 1000 + firstScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "first weekly recurring scheduled post", + }, + ScheduledAt: scheduledAt, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + firstCreated, err := th.Server.Store().ScheduledPost().CreateScheduledPost(firstScheduledPost) + require.NoError(t, err) + require.NotNil(t, firstCreated) + + secondScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "second weekly recurring scheduled post", + }, + ScheduledAt: scheduledAt, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + secondCreated, err := th.Server.Store().ScheduledPost().CreateScheduledPost(secondScheduledPost) + require.NoError(t, err) + require.NotNil(t, secondCreated) + + th.App.ProcessScheduledPosts(th.Context) + + firstUpdated, err := th.Server.Store().ScheduledPost().Get(firstCreated.Id) + require.NoError(t, err) + require.NotNil(t, firstUpdated) + assert.Equal(t, model.ScheduledPostRepeatTypeWeekly, firstUpdated.RepeatType) + assert.Empty(t, firstUpdated.ErrorCode) + assert.Zero(t, firstUpdated.ProcessedAt) + assert.Greater(t, firstUpdated.ScheduledAt, scheduledAt) + + secondUpdated, err := th.Server.Store().ScheduledPost().Get(secondCreated.Id) + require.NoError(t, err) + require.NotNil(t, secondUpdated) + assert.Equal(t, model.ScheduledPostRepeatTypeWeekly, secondUpdated.RepeatType) + assert.Empty(t, secondUpdated.ErrorCode) + assert.Zero(t, secondUpdated.ProcessedAt) + assert.Greater(t, secondUpdated.ScheduledAt, scheduledAt) + }) + + t.Run("advances overdue weekly recurring scheduled post older than one day", func(t *testing.T) { + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + + scheduledAt := model.GetMillis() - (48 * 60 * 60 * 1000) + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "overdue weekly recurring scheduled post", + }, + ScheduledAt: scheduledAt, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + created, err := th.Server.Store().ScheduledPost().CreateScheduledPost(scheduledPost) + assert.NoError(t, err) + require.NotNil(t, created) + + th.App.ProcessScheduledPosts(th.Context) + + updated, err := th.Server.Store().ScheduledPost().Get(created.Id) + assert.NoError(t, err) + require.NotNil(t, updated) + assert.Equal(t, model.ScheduledPostRepeatTypeWeekly, updated.RepeatType) + assert.Equal(t, "UTC", updated.RepeatTimezone) + assert.Empty(t, updated.ErrorCode) + assert.Zero(t, updated.ProcessedAt) + assert.Greater(t, updated.ScheduledAt, model.GetMillis()) + }) + + t.Run("permanently deletes recurring and one-shot posts when the channel no longer exists", func(t *testing.T) { + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + + scheduledAt := model.GetMillis() - 1000 + deletedChannelId := model.NewId() + + recurringScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: deletedChannelId, + Message: "recurring scheduled post for a channel that no longer exists", + }, + ScheduledAt: scheduledAt, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + recurringCreated, err := th.Server.Store().ScheduledPost().CreateScheduledPost(recurringScheduledPost) + require.NoError(t, err) + + oneShotScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: th.BasicUser.Id, + ChannelId: deletedChannelId, + Message: "one-shot scheduled post for a channel that no longer exists", + }, + ScheduledAt: scheduledAt, + } + oneShotCreated, err := th.Server.Store().ScheduledPost().CreateScheduledPost(oneShotScheduledPost) + require.NoError(t, err) + + th.App.ProcessScheduledPosts(th.Context) + + // Both rows must be permanently deleted: the series ends rather than advancing, + // erroring, or being silently reposted on later runs. + _, err = th.Server.Store().ScheduledPost().Get(recurringCreated.Id) + require.Error(t, err) + _, err = th.Server.Store().ScheduledPost().Get(oneShotCreated.Id) + require.Error(t, err) + }) + + t.Run("marks overdue one-shot posts even when overdue weekly posts move pagination backward", func(t *testing.T) { + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + + now := model.GetMillis() + weeklyScheduledAt := now - (48 * 60 * 60 * 1000) + oneShotScheduledAt := now - (36 * 60 * 60 * 1000) + + weeklyScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: now, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "overdue weekly recurring scheduled post", + }, + ScheduledAt: weeklyScheduledAt, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + weeklyCreated, err := th.Server.Store().ScheduledPost().CreateScheduledPost(weeklyScheduledPost) + assert.NoError(t, err) + require.NotNil(t, weeklyCreated) + + oneShotScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: now, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "overdue one-shot scheduled post", + }, + ScheduledAt: oneShotScheduledAt, + } + oneShotCreated, err := th.Server.Store().ScheduledPost().CreateScheduledPost(oneShotScheduledPost) + assert.NoError(t, err) + require.NotNil(t, oneShotCreated) + + th.App.ProcessScheduledPosts(th.Context) + + weeklyUpdated, err := th.Server.Store().ScheduledPost().Get(weeklyCreated.Id) + assert.NoError(t, err) + require.NotNil(t, weeklyUpdated) + assert.Equal(t, model.ScheduledPostRepeatTypeWeekly, weeklyUpdated.RepeatType) + assert.Equal(t, "UTC", weeklyUpdated.RepeatTimezone) + assert.Empty(t, weeklyUpdated.ErrorCode) + assert.Zero(t, weeklyUpdated.ProcessedAt) + assert.Greater(t, weeklyUpdated.ScheduledAt, model.GetMillis()) + + oneShotUpdated, err := th.Server.Store().ScheduledPost().Get(oneShotCreated.Id) + assert.NoError(t, err) + require.NotNil(t, oneShotUpdated) + assert.Equal(t, model.ScheduledPostErrorUnableToSend, oneShotUpdated.ErrorCode) + assert.Greater(t, oneShotUpdated.ProcessedAt, int64(0)) + }) + t.Run("sets error code for archived channel", func(t *testing.T) { th := Setup(t).InitBasic(t) diff --git a/server/channels/app/scheduled_post_test.go b/server/channels/app/scheduled_post_test.go index 8665d8108864..89680621caf6 100644 --- a/server/channels/app/scheduled_post_test.go +++ b/server/channels/app/scheduled_post_test.go @@ -625,7 +625,9 @@ func TestUpdateScheduledPost(t *testing.T) { require.NotEqual(t, newChannelId, updatedScheduledPost.ChannelId) require.NotEqual(t, newCreateAt, updatedScheduledPost.CreateAt) require.Equal(t, 2, len(updatedScheduledPost.FileIds)) - require.Equal(t, model.ScheduledPostErrorUnknownError, createdScheduledPost.ErrorCode) + require.Empty(t, createdScheduledPost.ErrorCode) + require.Empty(t, updatedScheduledPost.ErrorCode) + require.Zero(t, updatedScheduledPost.ProcessedAt) }) t.Run("should be able to update scheduled posts for channels user does not belong to", func(t *testing.T) { diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 6465c5c72743..3ec0c0d59aee 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -417,3 +417,7 @@ channels/db/migrations/postgres/000210_add_recap_skip_fields.down.sql channels/db/migrations/postgres/000210_add_recap_skip_fields.up.sql channels/db/migrations/postgres/000211_add_recaps_scheduled_recap_id_index.down.sql channels/db/migrations/postgres/000211_add_recaps_scheduled_recap_id_index.up.sql +channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.down.sql +channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.up.sql +channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.down.sql +channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.up.sql diff --git a/server/channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.down.sql b/server/channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.down.sql new file mode 100644 index 000000000000..e399ea337cc0 --- /dev/null +++ b/server/channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE scheduledposts DROP COLUMN IF EXISTS repeattimezone; +ALTER TABLE scheduledposts DROP COLUMN IF EXISTS repeattype; diff --git a/server/channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.up.sql b/server/channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.up.sql new file mode 100644 index 000000000000..baefcc7b27b2 --- /dev/null +++ b/server/channels/db/migrations/postgres/000212_add_scheduled_post_recurrence.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE scheduledposts ADD COLUMN IF NOT EXISTS repeattype VARCHAR(64) NOT NULL DEFAULT ''; +ALTER TABLE scheduledposts ADD COLUMN IF NOT EXISTS repeattimezone VARCHAR(128) NOT NULL DEFAULT ''; diff --git a/server/channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.down.sql b/server/channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.down.sql new file mode 100644 index 000000000000..dbe08201ceeb --- /dev/null +++ b/server/channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.down.sql @@ -0,0 +1,2 @@ +-- morph:nontransactional +DROP INDEX CONCURRENTLY IF EXISTS idx_scheduledposts_pending_scheduled_at_id; diff --git a/server/channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.up.sql b/server/channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.up.sql new file mode 100644 index 000000000000..28be817a7e30 --- /dev/null +++ b/server/channels/db/migrations/postgres/000213_add_scheduled_post_pending_index.up.sql @@ -0,0 +1,4 @@ +-- morph:nontransactional +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_scheduledposts_pending_scheduled_at_id + ON scheduledposts (scheduledat DESC, id) + WHERE errorcode = ''; diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 020f6ad8ff45..e641373e4be5 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -12820,6 +12820,27 @@ func (s *RetryLayerScheduledPostStore) UpdateOldScheduledPosts(beforeTime int64) } +func (s *RetryLayerScheduledPostStore) UpdateRecurringScheduledPosts(scheduledPosts []*model.ScheduledPost) error { + + tries := 0 + for { + err := s.ScheduledPostStore.UpdateRecurringScheduledPosts(scheduledPosts) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerScheduledPostStore) UpdatedScheduledPost(scheduledPost *model.ScheduledPost) error { tries := 0 diff --git a/server/channels/store/sqlstore/scheduled_post_store.go b/server/channels/store/sqlstore/scheduled_post_store.go index 3970e6e9fbec..871705b83be7 100644 --- a/server/channels/store/sqlstore/scheduled_post_store.go +++ b/server/channels/store/sqlstore/scheduled_post_store.go @@ -33,6 +33,7 @@ func normalizePrefix(prefix string) string { return prefix } +// Type is nullable, so it's kept out of baseColumns and coalesced on read. func baseColumns(prefix string) []string { return []string{ prefix + "Id", @@ -48,19 +49,19 @@ func baseColumns(prefix string) []string { prefix + "ScheduledAt", prefix + "ProcessedAt", prefix + "ErrorCode", + prefix + "RepeatType", + prefix + "RepeatTimezone", } } func (s *SqlScheduledPostStore) columnsForWrite(prefix string) []string { prefix = normalizePrefix(prefix) - columns := baseColumns(prefix) - return append(columns, prefix+"Type") + return append(baseColumns(prefix), prefix+"Type") } func (s *SqlScheduledPostStore) columnsForRead(prefix string) []string { prefix = normalizePrefix(prefix) - columns := baseColumns(prefix) - return append(columns, "COALESCE("+prefix+"Type, '') AS Type") + return append(baseColumns(prefix), "COALESCE("+prefix+"Type, '') AS Type") } func (s *SqlScheduledPostStore) scheduledPostToSlice(scheduledPost *model.ScheduledPost) []any { @@ -78,6 +79,8 @@ func (s *SqlScheduledPostStore) scheduledPostToSlice(scheduledPost *model.Schedu scheduledPost.ScheduledAt, scheduledPost.ProcessedAt, scheduledPost.ErrorCode, + scheduledPost.RepeatType, + scheduledPost.RepeatTimezone, scheduledPost.Type, } } @@ -152,33 +155,29 @@ func (s *SqlScheduledPostStore) GetMaxMessageSize() int { } func (s *SqlScheduledPostStore) GetPendingScheduledPosts(beforeTime, afterTime int64, lastScheduledPostId string, perPage uint64) ([]*model.ScheduledPost, error) { + // The ScheduledAt <= beforeTime bound stays outside the keyset tie-break so Postgres can + // use it as the boundary of idx_scheduledposts_pending_scheduled_at_id; the equivalent + // pure OR form would force scanning the index from the top on every page. + pendingCursor := sq.And{sq.LtOrEq{"ScheduledAt": beforeTime}} + if lastScheduledPostId != "" { + pendingCursor = append(pendingCursor, sq.Or{ + sq.Lt{"ScheduledAt": beforeTime}, + sq.Gt{"Id": lastScheduledPostId}, + }) + } + query := s.getQueryBuilder(). Select(s.columnsForRead("")...). From("ScheduledPosts"). Where(sq.Eq{"ErrorCode": ""}). + Where(pendingCursor). + Where(sq.Or{ + sq.Eq{"RepeatType": model.ScheduledPostRepeatTypeWeekly}, + sq.GtOrEq{"ScheduledAt": afterTime}, + }). OrderBy("ScheduledAt DESC", "Id"). Limit(perPage) - if lastScheduledPostId == "" { - query = query.Where(sq.And{ - sq.LtOrEq{"ScheduledAt": beforeTime}, - sq.GtOrEq{"ScheduledAt": afterTime}, - }) - } - if lastScheduledPostId != "" { - query = query. - Where(sq.Or{ - sq.And{ - sq.LtOrEq{"ScheduledAt": beforeTime}, - sq.GtOrEq{"ScheduledAt": afterTime}, - }, - sq.And{ - sq.Eq{"ScheduledAt": beforeTime}, - sq.Gt{"Id": lastScheduledPostId}, - }, - }) - } - // We read from the master here instead of a replica on purpose. The scheduled post job // deletes processed posts and then fetches the next page of pending posts. Reading from a // replica can return stale data, causing already-processed posts to reappear on later pages. @@ -226,6 +225,8 @@ func (s *SqlScheduledPostStore) PermanentlyDeleteScheduledPosts(scheduledPostIDs return nil } +// UpdatedScheduledPost persists the scheduled post as given; ProcessedAt and ErrorCode are +// caller-owned and stored verbatim. func (s *SqlScheduledPostStore) UpdatedScheduledPost(scheduledPost *model.ScheduledPost) error { scheduledPost.PreUpdate() @@ -249,18 +250,60 @@ func (s *SqlScheduledPostStore) UpdatedScheduledPost(scheduledPost *model.Schedu return nil } +// UpdateRecurringScheduledPosts advances recurring scheduled posts to their next occurrence in a +// single query, persisting each post's ScheduledAt and resetting ErrorCode/ProcessedAt so the posts +// are pending again. It also stamps UpdateAt on the given posts. +func (s *SqlScheduledPostStore) UpdateRecurringScheduledPosts(scheduledPosts []*model.ScheduledPost) error { + if len(scheduledPosts) == 0 { + return nil + } + + updateAt := model.GetMillis() + scheduledAtCase := sq.Case("Id") + ids := make([]string, len(scheduledPosts)) + + for i, scheduledPost := range scheduledPosts { + scheduledPost.UpdateAt = updateAt + ids[i] = scheduledPost.Id + // The ::bigint cast is required for Postgres to infer the parameter type inside CASE...THEN. + scheduledAtCase = scheduledAtCase.When(sq.Expr("?", scheduledPost.Id), sq.Expr("?::bigint", scheduledPost.ScheduledAt)) + } + + builder := s.getQueryBuilder(). + Update("ScheduledPosts"). + Set("ScheduledAt", scheduledAtCase). + Set("ErrorCode", ""). + Set("ProcessedAt", 0). + Set("UpdateAt", updateAt). + Where(sq.Eq{"Id": ids}) + + query, args, err := builder.ToSql() + if err != nil { + mlog.Error("SqlScheduledPostStore.UpdateRecurringScheduledPosts failed to generate SQL from updating scheduled posts", mlog.Err(err)) + return errors.Wrap(err, "SqlScheduledPostStore.UpdateRecurringScheduledPosts failed to generate SQL from updating scheduled posts") + } + + if _, err := s.GetMaster().Exec(query, args...); err != nil { + mlog.Error("SqlScheduledPostStore.UpdateRecurringScheduledPosts failed to update scheduled posts", mlog.Int("scheduled_post_count", len(scheduledPosts)), mlog.Err(err)) + return errors.Wrap(err, "SqlScheduledPostStore.UpdateRecurringScheduledPosts failed to update scheduled posts") + } + + return nil +} + func (s *SqlScheduledPostStore) toUpdateMap(scheduledPost *model.ScheduledPost) map[string]any { - now := model.GetMillis() return map[string]any{ - "UpdateAt": now, - "Message": scheduledPost.Message, - "Props": model.StringInterfaceToJSON(scheduledPost.GetProps()), - "FileIds": model.ArrayToJSON(scheduledPost.FileIds), - "Priority": model.StringInterfaceToJSON(scheduledPost.Priority), - "ScheduledAt": scheduledPost.ScheduledAt, - "ProcessedAt": now, - "ErrorCode": scheduledPost.ErrorCode, - "Type": scheduledPost.Type, + "UpdateAt": model.GetMillis(), + "Message": scheduledPost.Message, + "Props": model.StringInterfaceToJSON(scheduledPost.GetProps()), + "FileIds": model.ArrayToJSON(scheduledPost.FileIds), + "Priority": model.StringInterfaceToJSON(scheduledPost.Priority), + "ScheduledAt": scheduledPost.ScheduledAt, + "ProcessedAt": scheduledPost.ProcessedAt, + "ErrorCode": scheduledPost.ErrorCode, + "Type": scheduledPost.Type, + "RepeatType": scheduledPost.RepeatType, + "RepeatTimezone": scheduledPost.RepeatTimezone, } } @@ -290,6 +333,7 @@ func (s *SqlScheduledPostStore) UpdateOldScheduledPosts(beforeTime int64) error Set("ProcessedAt", model.GetMillis()). Where(sq.And{ sq.Eq{"ErrorCode": ""}, + sq.NotEq{"RepeatType": model.ScheduledPostRepeatTypeWeekly}, sq.Lt{"ScheduledAt": beforeTime}, }) diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 039127c98f93..4d4c8846fa2c 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1174,6 +1174,7 @@ type ScheduledPostStore interface { GetPendingScheduledPosts(beforeTime, afterTime int64, lastScheduledPostId string, perPage uint64) ([]*model.ScheduledPost, error) PermanentlyDeleteScheduledPosts(scheduledPostIDs []string) error UpdatedScheduledPost(scheduledPost *model.ScheduledPost) error + UpdateRecurringScheduledPosts(scheduledPosts []*model.ScheduledPost) error Get(scheduledPostId string) (*model.ScheduledPost, error) UpdateOldScheduledPosts(beforeTime int64) error PermanentDeleteByUser(userId string) error diff --git a/server/channels/store/storetest/mocks/ScheduledPostStore.go b/server/channels/store/storetest/mocks/ScheduledPostStore.go index c45d64d85f46..9851f297ebef 100644 --- a/server/channels/store/storetest/mocks/ScheduledPostStore.go +++ b/server/channels/store/storetest/mocks/ScheduledPostStore.go @@ -206,6 +206,24 @@ func (_m *ScheduledPostStore) UpdateOldScheduledPosts(beforeTime int64) error { return r0 } +// UpdateRecurringScheduledPosts provides a mock function with given fields: scheduledPosts +func (_m *ScheduledPostStore) UpdateRecurringScheduledPosts(scheduledPosts []*model.ScheduledPost) error { + ret := _m.Called(scheduledPosts) + + if len(ret) == 0 { + panic("no return value specified for UpdateRecurringScheduledPosts") + } + + var r0 error + if rf, ok := ret.Get(0).(func([]*model.ScheduledPost) error); ok { + r0 = rf(scheduledPosts) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // UpdatedScheduledPost provides a mock function with given fields: scheduledPost func (_m *ScheduledPostStore) UpdatedScheduledPost(scheduledPost *model.ScheduledPost) error { ret := _m.Called(scheduledPost) diff --git a/server/channels/store/storetest/scheduled_post_store.go b/server/channels/store/storetest/scheduled_post_store.go index c8f6691e59eb..3be523be2c4f 100644 --- a/server/channels/store/storetest/scheduled_post_store.go +++ b/server/channels/store/storetest/scheduled_post_store.go @@ -4,6 +4,7 @@ package storetest import ( + "fmt" "testing" "time" @@ -19,6 +20,7 @@ func TestScheduledPostStore(t *testing.T, rctx request.CTX, ss store.Store, s Sq t.Run("GetPendingScheduledPosts", func(t *testing.T) { testGetScheduledPosts(t, rctx, ss, s) }) t.Run("PermanentlyDeleteScheduledPosts", func(t *testing.T) { testPermanentlyDeleteScheduledPosts(t, rctx, ss, s) }) t.Run("UpdatedScheduledPost", func(t *testing.T) { testUpdatedScheduledPost(t, rctx, ss, s) }) + t.Run("UpdateRecurringScheduledPosts", func(t *testing.T) { testUpdateRecurringScheduledPosts(t, rctx, ss, s) }) t.Run("UpdateOldScheduledPosts", func(t *testing.T) { testUpdateOldScheduledPosts(t, rctx, ss, s) }) t.Run("PermanentDeleteByUser", func(t *testing.T) { testPermanentDeleteScheduledPostsByUser(t, rctx, ss, s) }) } @@ -85,6 +87,35 @@ func testCreateScheduledPost(t *testing.T, rctx request.CTX, ss store.Store, s S _ = ss.ScheduledPost().PermanentlyDeleteScheduledPosts([]string{scheduledPost.Id}) }() }) + + t.Run("weekly recurrence fields persist", func(t *testing.T) { + userId := model.NewId() + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: userId, + ChannelId: createdChannel.Id, + Message: "this is a weekly scheduled post", + }, + ScheduledAt: model.GetMillis() + 100000, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "America/New_York", + } + + createdScheduledPost, err := ss.ScheduledPost().CreateScheduledPost(scheduledPost) + assert.NoError(t, err) + assert.NotEmpty(t, createdScheduledPost.Id) + + defer func() { + _ = ss.ScheduledPost().PermanentlyDeleteScheduledPosts([]string{createdScheduledPost.Id}) + }() + + scheduledPostsFromDatabase, err := ss.ScheduledPost().GetScheduledPostsForUser(userId, "team_id_1") + assert.NoError(t, err) + require.Len(t, scheduledPostsFromDatabase, 1) + assert.Equal(t, model.ScheduledPostRepeatTypeWeekly, scheduledPostsFromDatabase[0].RepeatType) + assert.Equal(t, "America/New_York", scheduledPostsFromDatabase[0].RepeatTimezone) + }) } func testGetScheduledPosts(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { @@ -170,6 +201,87 @@ func testGetScheduledPosts(t *testing.T, rctx request.CTX, ss store.Store, s Sql assert.NoError(t, err) assert.Equal(t, 0, len(scheduledPosts)) }) + + t.Run("should paginate with the keyset cursor without skipping or repeating posts", func(t *testing.T) { + batchTime := model.GetMillisForTime(time.Date(2100, time.June, 1, 9, 0, 0, 0, time.UTC)) + earlierTime := batchTime - (60 * 60 * 1000) + scheduledAts := []int64{batchTime, batchTime, batchTime, earlierTime, earlierTime} + + var createdIDs []string + for i, scheduledAt := range scheduledAts { + scheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: model.NewId(), + ChannelId: model.NewId(), + Message: fmt.Sprintf("pagination scheduled post %d", i), + }, + ScheduledAt: scheduledAt, + } + + createdScheduledPost, err := ss.ScheduledPost().CreateScheduledPost(scheduledPost) + require.NoError(t, err) + createdIDs = append(createdIDs, createdScheduledPost.Id) + } + + defer func() { + _ = ss.ScheduledPost().PermanentlyDeleteScheduledPosts(createdIDs) + }() + + // page through the same way ProcessScheduledPosts does: the last item of each + // page provides the next page's beforeTime and lastScheduledPostId. + beforeTime := batchTime + (24 * 60 * 60 * 1000) + afterTime := earlierTime - (24 * 60 * 60 * 1000) + lastScheduledPostId := "" + perPage := uint64(2) + + var seenIDs []string + for { + page, err := ss.ScheduledPost().GetPendingScheduledPosts(beforeTime, afterTime, lastScheduledPostId, perPage) + require.NoError(t, err) + if len(page) == 0 { + break + } + + for _, scheduledPost := range page { + seenIDs = append(seenIDs, scheduledPost.Id) + } + + lastScheduledPostId = page[len(page)-1].Id + beforeTime = page[len(page)-1].ScheduledAt + } + + assert.ElementsMatch(t, createdIDs, seenIDs) + }) + + t.Run("should include overdue recurring scheduled posts older than the one-shot window", func(t *testing.T) { + recurringScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: model.NewId(), + ChannelId: model.NewId(), + Message: "this is a recurring scheduled post", + }, + ScheduledAt: model.GetMillisForTime(time.Date(2100, time.January, 1, 1, 0, 0, 0, time.UTC)), + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + + createdRecurringScheduledPost, err := ss.ScheduledPost().CreateScheduledPost(recurringScheduledPost) + assert.NoError(t, err) + assert.NotEmpty(t, createdRecurringScheduledPost.Id) + + defer func() { + _ = ss.ScheduledPost().PermanentlyDeleteScheduledPosts([]string{createdRecurringScheduledPost.Id}) + }() + + beforeTime := model.GetMillisForTime(time.Date(2100, time.March, 1, 1, 0, 0, 0, time.UTC)) + afterTime := model.GetMillisForTime(time.Date(2100, time.February, 1, 1, 0, 0, 0, time.UTC)) + scheduledPosts, err := ss.ScheduledPost().GetPendingScheduledPosts(beforeTime, afterTime, "", 10) + assert.NoError(t, err) + require.Len(t, scheduledPosts, 1) + assert.Equal(t, createdRecurringScheduledPost.Id, scheduledPosts[0].Id) + }) } func testPermanentlyDeleteScheduledPosts(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { @@ -293,6 +405,7 @@ func testUpdatedScheduledPost(t *testing.T, rctx request.CTX, ss store.Store, s updateSchedulePost := &model.ScheduledPost{ Id: createdScheduledPost.Id, ScheduledAt: newScheduledAt, + ProcessedAt: 0, ErrorCode: "test_error_code", Draft: model.Draft{ CreateAt: model.GetMillis(), @@ -320,6 +433,7 @@ func testUpdatedScheduledPost(t *testing.T, rctx request.CTX, ss store.Store, s // fields that should have changed assert.Equal(t, newScheduledAt, userScheduledPosts[0].ScheduledAt) assert.Equal(t, "test_error_code", userScheduledPosts[0].ErrorCode) + assert.Zero(t, userScheduledPosts[0].ProcessedAt) assert.Equal(t, "updated message", userScheduledPosts[0].Message) assert.Equal(t, 2, len(userScheduledPosts[0].FileIds)) assert.Equal(t, "urgent", userScheduledPosts[0].Priority["priority"]) @@ -351,6 +465,7 @@ func testUpdatedScheduledPost(t *testing.T, rctx request.CTX, ss store.Store, s // now we'll update the scheduled post now := model.GetMillis() scheduledPost.ErrorCode = model.ScheduledPostErrorUnknownError + scheduledPost.ProcessedAt = now err = ss.ScheduledPost().UpdatedScheduledPost(scheduledPost) assert.NoError(t, err) @@ -362,6 +477,92 @@ func testUpdatedScheduledPost(t *testing.T, rctx request.CTX, ss store.Store, s }) } +func testUpdateRecurringScheduledPosts(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { + channel := &model.Channel{ + TeamId: model.NewId(), + Type: model.ChannelTypeOpen, + Name: "recurring_channel", + DisplayName: "Recurring Channel", + } + + createdChannel, err := ss.Channel().Save(rctx, channel, 1000) + require.NoError(t, err) + + defer func() { + _ = ss.Channel().PermanentDelete(rctx, createdChannel.Id) + }() + + t.Run("should update recurring scheduled posts in bulk", func(t *testing.T) { + userId := model.NewId() + firstScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: userId, + ChannelId: createdChannel.Id, + Message: "first recurring scheduled post", + }, + ScheduledAt: model.GetMillis() + 100000, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + firstCreated, err := ss.ScheduledPost().CreateScheduledPost(firstScheduledPost) + require.NoError(t, err) + require.NotEmpty(t, firstCreated.Id) + + secondScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: userId, + ChannelId: createdChannel.Id, + Message: "second recurring scheduled post", + }, + ScheduledAt: model.GetMillis() + 200000, + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + secondCreated, err := ss.ScheduledPost().CreateScheduledPost(secondScheduledPost) + require.NoError(t, err) + require.NotEmpty(t, secondCreated.Id) + + defer func() { + _ = ss.ScheduledPost().PermanentlyDeleteScheduledPosts([]string{firstCreated.Id, secondCreated.Id}) + }() + + firstNextAt := firstCreated.ScheduledAt + int64(7*24*time.Hour/time.Millisecond) + secondNextAt := secondCreated.ScheduledAt + int64(14*24*time.Hour/time.Millisecond) + firstCreated.ScheduledAt = firstNextAt + firstCreated.ErrorCode = "" + firstCreated.ProcessedAt = 0 + secondCreated.ScheduledAt = secondNextAt + secondCreated.ErrorCode = "" + secondCreated.ProcessedAt = 0 + + err = ss.ScheduledPost().UpdateRecurringScheduledPosts([]*model.ScheduledPost{firstCreated, secondCreated}) + require.NoError(t, err) + + updatedFirst, err := ss.ScheduledPost().Get(firstCreated.Id) + require.NoError(t, err) + require.NotNil(t, updatedFirst) + assert.Equal(t, firstNextAt, updatedFirst.ScheduledAt) + assert.Empty(t, updatedFirst.ErrorCode) + assert.Zero(t, updatedFirst.ProcessedAt) + assert.GreaterOrEqual(t, updatedFirst.UpdateAt, firstCreated.UpdateAt) + + updatedSecond, err := ss.ScheduledPost().Get(secondCreated.Id) + require.NoError(t, err) + require.NotNil(t, updatedSecond) + assert.Equal(t, secondNextAt, updatedSecond.ScheduledAt) + assert.Empty(t, updatedSecond.ErrorCode) + assert.Zero(t, updatedSecond.ProcessedAt) + assert.GreaterOrEqual(t, updatedSecond.UpdateAt, secondCreated.UpdateAt) + }) + + t.Run("should not fail for empty input", func(t *testing.T) { + err := ss.ScheduledPost().UpdateRecurringScheduledPosts(nil) + require.NoError(t, err) + }) +} + func testUpdateOldScheduledPosts(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { setupScheduledPosts := func(baseTime int64, userId, teamId string) func() { channel := &model.Channel{ @@ -469,6 +670,48 @@ func testUpdateOldScheduledPosts(t *testing.T, rctx request.CTX, ss store.Store, assert.Equal(t, "", scheduledPosts[2].ErrorCode) assert.Equal(t, "", scheduledPosts[3].ErrorCode) }) + + t.Run("should not update overdue recurring scheduled posts", func(t *testing.T) { + channel := &model.Channel{ + TeamId: model.NewId(), + Type: model.ChannelTypeOpen, + Name: "recurring_channel", + DisplayName: "Recurring Channel", + } + + createdChannel, err := ss.Channel().Save(rctx, channel, 1000) + assert.NoError(t, err) + + recurringScheduledPost := &model.ScheduledPost{ + Draft: model.Draft{ + CreateAt: model.GetMillis(), + UserId: model.NewId(), + ChannelId: createdChannel.Id, + Message: "this is an overdue recurring scheduled post", + }, + ScheduledAt: model.GetMillis() - (7 * 24 * 60 * 60 * 1000), + RepeatType: model.ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "UTC", + } + + createdRecurringScheduledPost, err := ss.ScheduledPost().CreateScheduledPost(recurringScheduledPost) + assert.NoError(t, err) + assert.NotEmpty(t, createdRecurringScheduledPost.Id) + + defer func() { + _ = ss.ScheduledPost().PermanentlyDeleteScheduledPosts([]string{createdRecurringScheduledPost.Id}) + _ = ss.Channel().PermanentDelete(rctx, createdChannel.Id) + }() + + err = ss.ScheduledPost().UpdateOldScheduledPosts(model.GetMillis()) + assert.NoError(t, err) + + storedRecurringScheduledPost, err := ss.ScheduledPost().Get(createdRecurringScheduledPost.Id) + assert.NoError(t, err) + require.NotNil(t, storedRecurringScheduledPost) + assert.Empty(t, storedRecurringScheduledPost.ErrorCode) + assert.Zero(t, storedRecurringScheduledPost.ProcessedAt) + }) } func testPermanentDeleteScheduledPostsByUser(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 9ca3fa06dfde..06315cd20c57 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -10163,6 +10163,22 @@ func (s *TimerLayerScheduledPostStore) UpdateOldScheduledPosts(beforeTime int64) return err } +func (s *TimerLayerScheduledPostStore) UpdateRecurringScheduledPosts(scheduledPosts []*model.ScheduledPost) error { + start := time.Now() + + err := s.ScheduledPostStore.UpdateRecurringScheduledPosts(scheduledPosts) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("ScheduledPostStore.UpdateRecurringScheduledPosts", success, elapsed) + } + return err +} + func (s *TimerLayerScheduledPostStore) UpdatedScheduledPost(scheduledPost *model.ScheduledPost) error { start := time.Now() diff --git a/server/i18n/en.json b/server/i18n/en.json index 9398c354129a..5dd7798a3629 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -9118,6 +9118,10 @@ "id": "app.scheduled_post.private_channel", "translation": "Private channel" }, + { + "id": "app.scheduled_post.recurring_disabled.app_error", + "translation": "Recurring scheduled posts are disabled on this server." + }, { "id": "app.scheduled_post.save.rejected_by_plugin", "translation": "Scheduled post rejected by plugin: {{.Reason}}" @@ -13234,6 +13238,22 @@ "id": "model.scheduled_post.is_valid.processed_at.app_error", "translation": "Invalid processed at time." }, + { + "id": "model.scheduled_post.is_valid.repeat_files.app_error", + "translation": "Recurring scheduled posts cannot have file attachments." + }, + { + "id": "model.scheduled_post.is_valid.repeat_timezone.app_error", + "translation": "Repeat weekly scheduled posts require a valid timezone." + }, + { + "id": "model.scheduled_post.is_valid.repeat_timezone_invalid.app_error", + "translation": "Invalid timezone for recurring scheduled post." + }, + { + "id": "model.scheduled_post.is_valid.repeat_type.app_error", + "translation": "Invalid repeat type for scheduled post." + }, { "id": "model.scheduled_post.is_valid.scheduled_at.app_error", "translation": "Invalid scheduled at time." diff --git a/server/public/model/feature_flags.go b/server/public/model/feature_flags.go index 1bc6322c0bdd..7e5113184aef 100644 --- a/server/public/model/feature_flags.go +++ b/server/public/model/feature_flags.go @@ -151,6 +151,9 @@ type FeatureFlags struct { // Enable verifying plugin signatures against the MFI public key, in addition to the // existing hard-coded Mattermost public key and any admin-configured public keys. EnableMFIPluginSignaturePublicKey bool + + // FEATURE_FLAG_REMOVAL: RecurringScheduledPosts - Remove this when the feature is GA. + RecurringScheduledPosts bool } func (f *FeatureFlags) SetDefaults() { @@ -215,6 +218,8 @@ func (f *FeatureFlags) SetDefaults() { f.EnableConcurrentReact = false f.EnableMFIPluginSignaturePublicKey = true + + f.RecurringScheduledPosts = false } // IsChannelPermissionPoliciesEnabled reports whether channel-scope diff --git a/server/public/model/feature_flags_test.go b/server/public/model/feature_flags_test.go index 87ff1408c1ba..4c159340aff8 100644 --- a/server/public/model/feature_flags_test.go +++ b/server/public/model/feature_flags_test.go @@ -64,6 +64,17 @@ func TestFeatureFlagsSetDefaults_AttributeValueMasking(t *testing.T) { require.Equal(t, "true", flags.ToMap()["AttributeValueMasking"]) } +func TestFeatureFlagsSetDefaults_RecurringScheduledPosts(t *testing.T) { + var flags FeatureFlags + flags.SetDefaults() + + require.False(t, flags.RecurringScheduledPosts, "RecurringScheduledPosts should default to false") + require.Equal(t, "false", flags.ToMap()["RecurringScheduledPosts"]) + + flags.RecurringScheduledPosts = true + require.Equal(t, "true", flags.ToMap()["RecurringScheduledPosts"]) +} + func TestFeatureFlagsSetDefaults_PostAttributes(t *testing.T) { var flags FeatureFlags flags.SetDefaults() diff --git a/server/public/model/scheduled_post.go b/server/public/model/scheduled_post.go index 199eec3fc1ec..adca84098980 100644 --- a/server/public/model/scheduled_post.go +++ b/server/public/model/scheduled_post.go @@ -6,6 +6,7 @@ package model import ( "fmt" "net/http" + "time" ) const ( @@ -29,10 +30,12 @@ const scheduledPostMaxTimeGap = -5000 type ScheduledPost struct { Draft - Id string `json:"id"` - ScheduledAt int64 `json:"scheduled_at"` - ProcessedAt int64 `json:"processed_at"` - ErrorCode string `json:"error_code"` + Id string `json:"id"` + ScheduledAt int64 `json:"scheduled_at"` + ProcessedAt int64 `json:"processed_at"` + ErrorCode string `json:"error_code"` + RepeatType string `json:"repeat_type"` + RepeatTimezone string `json:"repeat_timezone"` } func (s *ScheduledPost) IsValid(maxMessageSize int) *AppError { @@ -65,6 +68,31 @@ func (s *ScheduledPost) BaseIsValid() *AppError { return NewAppError("ScheduledPost.IsValid", "model.scheduled_post.is_valid.processed_at.app_error", nil, "id="+s.Id, http.StatusBadRequest) } + switch s.RepeatType { + case ScheduledPostRepeatTypeNone, ScheduledPostRepeatTypeWeekly: + default: + return NewAppError("ScheduledPost.IsValid", "model.scheduled_post.is_valid.repeat_type.app_error", nil, "id="+s.Id+", repeat_type="+s.RepeatType, http.StatusBadRequest) + } + + if s.RepeatType == ScheduledPostRepeatTypeWeekly { + // Files are bound to the first post they're attached to, so later occurrences + // would silently send without them. + if len(s.FileIds) > 0 { + return NewAppError("ScheduledPost.IsValid", "model.scheduled_post.is_valid.repeat_files.app_error", nil, "id="+s.Id, http.StatusBadRequest) + } + if s.RepeatTimezone == "" { + return NewAppError("ScheduledPost.IsValid", "model.scheduled_post.is_valid.repeat_timezone.app_error", nil, "id="+s.Id, http.StatusBadRequest) + } + // "Local" loads successfully but depends on each server's host timezone; a persisted + // recurring schedule needs a fixed zone (UTC or an IANA name). + if s.RepeatTimezone == "Local" { + return NewAppError("ScheduledPost.IsValid", "model.scheduled_post.is_valid.repeat_timezone_invalid.app_error", nil, "id="+s.Id+", repeat_timezone="+s.RepeatTimezone, http.StatusBadRequest) + } + if _, err := time.LoadLocation(s.RepeatTimezone); err != nil { + return NewAppError("ScheduledPost.IsValid", "model.scheduled_post.is_valid.repeat_timezone_invalid.app_error", nil, "id="+s.Id+", repeat_timezone="+s.RepeatTimezone+", "+err.Error(), http.StatusBadRequest) + } + } + return nil } @@ -137,15 +165,17 @@ func (s *ScheduledPost) Auditable() map[string]any { } return map[string]any{ - "id": s.Id, - "create_at": s.CreateAt, - "update_at": s.UpdateAt, - "user_id": s.UserId, - "channel_id": s.ChannelId, - "root_id": s.RootId, - "props": s.GetProps(), - "file_ids": s.FileIds, - "metadata": metaData, + "id": s.Id, + "create_at": s.CreateAt, + "update_at": s.UpdateAt, + "user_id": s.UserId, + "channel_id": s.ChannelId, + "root_id": s.RootId, + "props": s.GetProps(), + "file_ids": s.FileIds, + "metadata": metaData, + "repeat_type": s.RepeatType, + "repeat_timezone": s.RepeatTimezone, } } diff --git a/server/public/model/scheduled_post_recurrence.go b/server/public/model/scheduled_post_recurrence.go new file mode 100644 index 000000000000..89c130b7d7fd --- /dev/null +++ b/server/public/model/scheduled_post_recurrence.go @@ -0,0 +1,40 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +import ( + "fmt" + "time" +) + +const ( + ScheduledPostRepeatTypeNone = "" + ScheduledPostRepeatTypeWeekly = "weekly" +) + +func (s *ScheduledPost) IsRecurring() bool { + return s.RepeatType == ScheduledPostRepeatTypeWeekly +} + +// ComputeNextScheduledAt returns the next occurrence strictly after nowMillis for +// the post's repeat type, preserving local wall-clock time in RepeatTimezone. +func (s *ScheduledPost) ComputeNextScheduledAt(nowMillis int64) (int64, error) { + switch s.RepeatType { + case ScheduledPostRepeatTypeWeekly: + loc, err := time.LoadLocation(s.RepeatTimezone) + if err != nil { + return 0, fmt.Errorf("failed to load repeat timezone %q: %w", s.RepeatTimezone, err) + } + + // AddDate on a time in loc adds 7 local days, keeping the wall-clock time across DST changes. + now := time.UnixMilli(nowMillis) + next := time.UnixMilli(s.ScheduledAt).In(loc).AddDate(0, 0, 7) + for !next.After(now) { + next = next.AddDate(0, 0, 7) + } + return next.UnixMilli(), nil + default: + return 0, fmt.Errorf("unsupported scheduled post repeat type %q", s.RepeatType) + } +} diff --git a/server/public/model/scheduled_post_recurrence_test.go b/server/public/model/scheduled_post_recurrence_test.go new file mode 100644 index 000000000000..27e27cd80df5 --- /dev/null +++ b/server/public/model/scheduled_post_recurrence_test.go @@ -0,0 +1,82 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestComputeNextScheduledAt(t *testing.T) { + tz := "America/New_York" + loc, err := time.LoadLocation(tz) + require.NoError(t, err) + + // Thursday March 26, 2026 9:00 AM local + base := time.Date(2026, time.March, 26, 9, 0, 0, 0, loc) + + t.Run("advances one week preserving wall-clock time", func(t *testing.T) { + scheduledPost := &ScheduledPost{ + ScheduledAt: base.UnixMilli(), + RepeatType: ScheduledPostRepeatTypeWeekly, + RepeatTimezone: tz, + } + now := base.Add(1 * time.Minute) // just after send + + next, err := scheduledPost.ComputeNextScheduledAt(now.UnixMilli()) + require.NoError(t, err) + nextTime := time.UnixMilli(next).In(loc) + require.Equal(t, time.Thursday, nextTime.Weekday()) + require.Equal(t, 9, nextTime.Hour()) + require.Equal(t, 0, nextTime.Minute()) + // Next Thursday April 2 + require.Equal(t, time.April, nextTime.Month()) + require.Equal(t, 2, nextTime.Day()) + }) + + t.Run("skips past occurrences when the post is overdue by more than a week", func(t *testing.T) { + scheduledPost := &ScheduledPost{ + ScheduledAt: base.UnixMilli(), + RepeatType: ScheduledPostRepeatTypeWeekly, + RepeatTimezone: tz, + } + now := base.AddDate(0, 0, 16) // more than two weeks later + + next, err := scheduledPost.ComputeNextScheduledAt(now.UnixMilli()) + require.NoError(t, err) + nextTime := time.UnixMilli(next).In(loc) + require.True(t, nextTime.After(now)) + require.Equal(t, time.Thursday, nextTime.Weekday()) + require.Equal(t, 9, nextTime.Hour()) + }) + + t.Run("returns an error for an invalid timezone", func(t *testing.T) { + scheduledPost := &ScheduledPost{ + ScheduledAt: base.UnixMilli(), + RepeatType: ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "Not/AZone", + } + + _, err := scheduledPost.ComputeNextScheduledAt(base.UnixMilli()) + require.Error(t, err) + }) + + t.Run("returns an error for an unsupported repeat type", func(t *testing.T) { + scheduledPost := &ScheduledPost{ + ScheduledAt: base.UnixMilli(), + RepeatType: "daily", + RepeatTimezone: tz, + } + + _, err := scheduledPost.ComputeNextScheduledAt(base.UnixMilli()) + require.Error(t, err) + }) +} + +func TestScheduledPostIsRecurring(t *testing.T) { + require.True(t, (&ScheduledPost{RepeatType: ScheduledPostRepeatTypeWeekly}).IsRecurring()) + require.False(t, (&ScheduledPost{}).IsRecurring()) +} diff --git a/server/public/model/scheduled_post_test.go b/server/public/model/scheduled_post_test.go index d9c444f1c209..d8ce25ab1b51 100644 --- a/server/public/model/scheduled_post_test.go +++ b/server/public/model/scheduled_post_test.go @@ -79,6 +79,135 @@ func TestScheduledPostBaseIsValid(t *testing.T) { assert.Equal(t, "model.scheduled_post.is_valid.processed_at.app_error", err.Id) }) + t.Run("invalid repeat type", func(t *testing.T) { + s := ScheduledPost{ + Draft: Draft{ + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + UserId: NewId(), + ChannelId: NewId(), + Message: "test", + }, + Id: NewId(), + ScheduledAt: GetMillis() + 100000, + RepeatType: "daily", + } + err := s.BaseIsValid() + require.NotNil(t, err) + assert.Equal(t, "model.scheduled_post.is_valid.repeat_type.app_error", err.Id) + }) + + t.Run("weekly repeat requires a timezone", func(t *testing.T) { + s := ScheduledPost{ + Draft: Draft{ + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + UserId: NewId(), + ChannelId: NewId(), + Message: "test", + }, + Id: NewId(), + ScheduledAt: GetMillis() + 100000, + RepeatType: ScheduledPostRepeatTypeWeekly, + } + err := s.BaseIsValid() + require.NotNil(t, err) + assert.Equal(t, "model.scheduled_post.is_valid.repeat_timezone.app_error", err.Id) + }) + + t.Run("weekly repeat requires a valid timezone", func(t *testing.T) { + s := ScheduledPost{ + Draft: Draft{ + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + UserId: NewId(), + ChannelId: NewId(), + Message: "test", + }, + Id: NewId(), + ScheduledAt: GetMillis() + 100000, + RepeatType: ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "Not/AZone", + } + err := s.BaseIsValid() + require.NotNil(t, err) + assert.Equal(t, "model.scheduled_post.is_valid.repeat_timezone_invalid.app_error", err.Id) + }) + + t.Run("weekly repeat rejects the host-dependent Local timezone", func(t *testing.T) { + s := ScheduledPost{ + Draft: Draft{ + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + UserId: NewId(), + ChannelId: NewId(), + Message: "test", + }, + Id: NewId(), + ScheduledAt: GetMillis() + 100000, + RepeatType: ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "Local", + } + err := s.BaseIsValid() + require.NotNil(t, err) + assert.Equal(t, "model.scheduled_post.is_valid.repeat_timezone_invalid.app_error", err.Id) + }) + + t.Run("weekly repeat rejects file attachments", func(t *testing.T) { + s := ScheduledPost{ + Draft: Draft{ + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + UserId: NewId(), + ChannelId: NewId(), + Message: "test", + FileIds: StringArray{NewId()}, + }, + Id: NewId(), + ScheduledAt: GetMillis() + 100000, + RepeatType: ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "Europe/Berlin", + } + err := s.BaseIsValid() + require.NotNil(t, err) + assert.Equal(t, "model.scheduled_post.is_valid.repeat_files.app_error", err.Id) + }) + + t.Run("one-shot allows file attachments", func(t *testing.T) { + s := ScheduledPost{ + Draft: Draft{ + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + UserId: NewId(), + ChannelId: NewId(), + Message: "test", + FileIds: StringArray{NewId()}, + }, + Id: NewId(), + ScheduledAt: GetMillis() + 100000, + } + err := s.BaseIsValid() + require.Nil(t, err) + }) + + t.Run("valid weekly repeat", func(t *testing.T) { + s := ScheduledPost{ + Draft: Draft{ + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + UserId: NewId(), + ChannelId: NewId(), + Message: "test", + }, + Id: NewId(), + ScheduledAt: GetMillis() + 100000, + RepeatType: ScheduledPostRepeatTypeWeekly, + RepeatTimezone: "Europe/Berlin", + } + err := s.BaseIsValid() + require.Nil(t, err) + }) + t.Run("valid with message", func(t *testing.T) { s := ScheduledPost{ Draft: Draft{ diff --git a/webapp/channels/src/actions/websocket_actions.ts b/webapp/channels/src/actions/websocket_actions.ts index 5e49c7012ad5..850139cbf23f 100644 --- a/webapp/channels/src/actions/websocket_actions.ts +++ b/webapp/channels/src/actions/websocket_actions.ts @@ -122,7 +122,7 @@ import {getGroup} from 'mattermost-redux/selectors/entities/groups'; import {getPost, getMostRecentPostIdInChannel, getTeamIdFromPost} from 'mattermost-redux/selectors/entities/posts'; import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; import {haveISystemPermission, haveITeamPermission} from 'mattermost-redux/selectors/entities/roles'; -import {isScheduledPostsEnabled} from 'mattermost-redux/selectors/entities/scheduled_posts'; +import {getScheduledPostTeamId, isScheduledPostsEnabled} from 'mattermost-redux/selectors/entities/scheduled_posts'; import { getTeamIdByChannelId, getMyTeams, @@ -2260,11 +2260,13 @@ function handleCreateScheduledPostEvent(msg: WebSocketMessages.ScheduledPost): T function handleUpdateScheduledPostEvent(msg: WebSocketMessages.ScheduledPost): ThunkActionFunc { return async (doDispatch) => { const scheduledPost = JSON.parse(msg.data.scheduledPost) as ScheduledPost; + const teamId = getScheduledPostTeamId(getState(), scheduledPost); doDispatch({ type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, data: { scheduledPost, + teamId, }, }); }; diff --git a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx index a7e5e1879ff5..1545e287f261 100644 --- a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx +++ b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx @@ -67,7 +67,7 @@ import * as Utils from 'utils/utils'; import type {GlobalState} from 'types/store'; import type {PostDraft} from 'types/store/draft'; -import {isPostDraftEmpty} from 'types/store/draft'; +import {draftHasAttachments, isPostDraftEmpty} from 'types/store/draft'; import AIActionsMenu from './ai_actions_menu'; import DoNotDisturbWarning from './do_not_disturb_warning'; @@ -715,6 +715,7 @@ const AdvancedTextEditor = ({ disabled={disableSendButton} handleSubmit={handleSubmitPostAndScheduledMessage} channelId={channelId} + allowRecurring={!draftHasAttachments(draft)} /> ); diff --git a/webapp/channels/src/components/advanced_text_editor/scheduled_post_indicator/scheduled_post_indicator.tsx b/webapp/channels/src/components/advanced_text_editor/scheduled_post_indicator/scheduled_post_indicator.tsx index 652b6b5060d7..f8a09bcc3e23 100644 --- a/webapp/channels/src/components/advanced_text_editor/scheduled_post_indicator/scheduled_post_indicator.tsx +++ b/webapp/channels/src/components/advanced_text_editor/scheduled_post_indicator/scheduled_post_indicator.tsx @@ -41,7 +41,7 @@ export default function ScheduledPostIndicator({location, channelId, postId, rem const currentTeamName = useSelector((state: GlobalState) => getCurrentTeam(state)?.name); const scheduledPostLinkURL = `/${currentTeamName}/scheduled_posts?target_id=${id}`; - if (!scheduledPostData?.count) { + if (!scheduledPostData) { return null; } @@ -58,20 +58,20 @@ export default function ScheduledPostIndicator({location, channelId, postId, rem // display scheduled post's details of there is only one scheduled post if (scheduledPostData.count === 1 && scheduledPostData.scheduledPost) { + const scheduledPost = scheduledPostData.scheduledPost; + const dateTime = ( + + ); scheduledPostText = ( - ), - }} + values={{dateTime}} /> ); } diff --git a/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.scss b/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.scss new file mode 100644 index 000000000000..4f04c3a5b613 --- /dev/null +++ b/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.scss @@ -0,0 +1,24 @@ +.scheduled_post_custom_time_modal { + .ScheduledPostCustomTimeModal__repeat { + // Shrink-wrap so WithTooltip anchors to the checkbox/label, not the modal width. + display: flex; + width: fit-content; + align-items: flex-start; + margin-bottom: 12px; + gap: 8px; + + input[type='checkbox'] { + margin-top: 4px; + } + + label { + cursor: pointer; + } + + input[type='checkbox']:disabled, + input[type='checkbox']:disabled + label { + cursor: default; + opacity: 0.5; + } + } +} diff --git a/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.test.tsx b/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.test.tsx new file mode 100644 index 000000000000..7f4b38ed5949 --- /dev/null +++ b/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.test.tsx @@ -0,0 +1,92 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import {renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils'; + +import ScheduledPostCustomTimeModal from './scheduled_post_custom_time_modal'; + +jest.mock('mattermost-redux/actions/preferences', () => ({ + savePreferences: jest.fn(() => ({type: 'MOCK_SAVE_PREFERENCES'})), +})); + +describe('ScheduledPostCustomTimeModal', () => { + const onConfirm = jest.fn().mockResolvedValue({}); + + beforeEach(() => { + onConfirm.mockClear(); + }); + + function renderModal({recurringEnabled = true, initialRepeatWeekly = false, allowRecurring = true} = {}) { + return renderWithContext( + , + { + entities: { + general: { + config: { + ScheduledPosts: 'true', + FeatureFlagRecurringScheduledPosts: String(recurringEnabled), + }, + license: {IsLicensed: 'true'}, + }, + users: { + currentUserId: 'current_user_id', + profiles: {current_user_id: {id: 'current_user_id', roles: ''}}, + }, + }, + }, + ); + } + + it('should render the repeat weekly checkbox when recurring scheduled posts are enabled', () => { + renderModal(); + + expect(screen.getByLabelText('Repeat weekly')).toBeInTheDocument(); + }); + + it('should not render the repeat weekly checkbox when recurring scheduled posts are disabled', () => { + renderModal({recurringEnabled: false}); + + expect(screen.queryByLabelText('Repeat weekly')).not.toBeInTheDocument(); + }); + + it('should disable the repeat weekly checkbox when the message has attachments', () => { + renderModal({allowRecurring: false}); + + expect(screen.getByLabelText('Repeat weekly')).toBeDisabled(); + }); + + it('should preserve existing recurrence when recurring scheduled posts are disabled', async () => { + renderModal({recurringEnabled: false, initialRepeatWeekly: true}); + + await userEvent.click(screen.getByText('Schedule')); + + await waitFor(() => expect(onConfirm).toHaveBeenCalled()); + expect(onConfirm).toHaveBeenCalledWith(expect.objectContaining({repeat_type: 'weekly'})); + }); + + it('should send empty repeat fields when recurring scheduled posts are disabled and the post does not repeat', async () => { + renderModal({recurringEnabled: false}); + + await userEvent.click(screen.getByText('Schedule')); + + await waitFor(() => expect(onConfirm).toHaveBeenCalled()); + expect(onConfirm).toHaveBeenCalledWith(expect.objectContaining({repeat_type: '', repeat_timezone: ''})); + }); + + it('should send repeat fields when recurring scheduled posts are enabled and the post repeats', async () => { + renderModal({initialRepeatWeekly: true}); + + await userEvent.click(screen.getByText('Schedule')); + + await waitFor(() => expect(onConfirm).toHaveBeenCalled()); + expect(onConfirm).toHaveBeenCalledWith(expect.objectContaining({repeat_type: 'weekly'})); + }); +}); diff --git a/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.tsx b/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.tsx index 06c68a226087..9c26549436a1 100644 --- a/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.tsx +++ b/webapp/channels/src/components/advanced_text_editor/send_button/scheduled_post_custom_time_modal/scheduled_post_custom_time_modal.tsx @@ -7,8 +7,12 @@ import React, {useCallback, useMemo, useState} from 'react'; import {FormattedMessage, useIntl} from 'react-intl'; import {useDispatch, useSelector} from 'react-redux'; +import {WithTooltip} from '@mattermost/shared/components/tooltip'; +import type {SchedulingInfo} from '@mattermost/types/schedule_post'; + import {savePreferences} from 'mattermost-redux/actions/preferences'; import {testingEnabled} from 'mattermost-redux/selectors/entities/general'; +import {isRecurringScheduledPostsEnabled} from 'mattermost-redux/selectors/entities/scheduled_posts'; import {generateCurrentTimezoneLabel, getCurrentTimezone} from 'mattermost-redux/selectors/entities/timezone'; import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; @@ -19,19 +23,39 @@ import DateTimePickerModal from 'components/date_time_picker_modal/date_time_pic import {scheduledPosts} from 'utils/constants'; +import './scheduled_post_custom_time_modal.scss'; + const SCHEDULED_POST_CUSTOM_TIME_INTERVAL = 15; // minutes type Props = { channelId: string; onExited: () => void; - onConfirm: (timestamp: number) => Promise<{error?: string}>; + onConfirm: (schedulingInfo: SchedulingInfo) => Promise<{error?: string}>; initialTime?: Moment; + initialRepeatWeekly?: boolean; + + // Recurring posts can't carry file attachments, since files bind to the first post they're sent with. + allowRecurring?: boolean; }; -export default function ScheduledPostCustomTimeModal({channelId, onExited, onConfirm, initialTime}: Props) { +export default function ScheduledPostCustomTimeModal({ + channelId, + onExited, + onConfirm, + initialTime, + initialRepeatWeekly = false, + allowRecurring = true, +}: Props) { const {formatMessage} = useIntl(); const [errorMessage, setErrorMessage] = useState(); const userTimezone = useSelector(getCurrentTimezone); + const recurringEnabled = useSelector(isRecurringScheduledPostsEnabled); + const [repeatWeeklyChecked, setRepeatWeeklyChecked] = useState(initialRepeatWeekly); + const offerRecurring = recurringEnabled && allowRecurring; + + // While the checkbox can't be offered, keep whatever recurrence the post already has instead + // of silently clearing it; the job keeps sending existing series while the feature is off. + const repeatWeekly = offerRecurring ? repeatWeeklyChecked : initialRepeatWeekly; const now = moment().tz(userTimezone); const currentUserId = useSelector(getCurrentUserId); const dispatch = useDispatch(); @@ -47,7 +71,12 @@ export default function ScheduledPostCustomTimeModal({channelId, onExited, onCon const handleOnConfirm = useCallback(async (dateTime: Moment) => { const selectedTime = dateTime.valueOf(); - const response = await onConfirm(selectedTime); + const schedulingInfo: SchedulingInfo = { + scheduled_at: selectedTime, + repeat_type: repeatWeekly ? 'weekly' : '', + repeat_timezone: repeatWeekly ? userTimezone : '', + }; + const response = await onConfirm(schedulingInfo); dispatch( savePreferences( @@ -66,16 +95,47 @@ export default function ScheduledPostCustomTimeModal({channelId, onExited, onCon } else { onExited(); } - }, [onConfirm, onExited]); + }, [onConfirm, onExited, repeatWeekly, userTimezone, dispatch, currentUserId]); const bodySuffix = useMemo(() => { + const repeatRow = ( +
+ setRepeatWeeklyChecked(e.target.checked)} + /> + +
+ ); + return ( - + <> + {recurringEnabled && allowRecurring && repeatRow} + {recurringEnabled && !allowRecurring && ( + + {repeatRow} + + )} + + ); - }, [channelId, selectedDateTime]); + }, [channelId, selectedDateTime, recurringEnabled, allowRecurring, repeatWeekly, formatMessage]); const label = formatMessage({id: 'schedule_post.custom_time_modal.title', defaultMessage: 'Schedule message'}); diff --git a/webapp/channels/src/components/advanced_text_editor/send_button/send_button.tsx b/webapp/channels/src/components/advanced_text_editor/send_button/send_button.tsx index 70317cd14bc4..4108c66e628d 100644 --- a/webapp/channels/src/components/advanced_text_editor/send_button/send_button.tsx +++ b/webapp/channels/src/components/advanced_text_editor/send_button/send_button.tsx @@ -24,9 +24,10 @@ type SendButtonProps = { handleSubmit: (schedulingInfo?: SchedulingInfo) => void; disabled: boolean; channelId: string; + allowRecurring: boolean; }; -const SendButton = ({disabled, handleSubmit, channelId}: SendButtonProps) => { +const SendButton = ({disabled, handleSubmit, channelId, allowRecurring}: SendButtonProps) => { const {formatMessage} = useIntl(); const isScheduledPostEnabled = useSelector(isScheduledPostsEnabled); @@ -87,6 +88,7 @@ const SendButton = ({disabled, handleSubmit, channelId}: SendButtonProps) => { disabled={disabled} onSelect={handleSubmit} channelId={channelId} + allowRecurring={allowRecurring} /> } diff --git a/webapp/channels/src/components/advanced_text_editor/send_button/send_post_options/index.tsx b/webapp/channels/src/components/advanced_text_editor/send_button/send_post_options/index.tsx index 29cbf6075910..62804b3e6f5d 100644 --- a/webapp/channels/src/components/advanced_text_editor/send_button/send_post_options/index.tsx +++ b/webapp/channels/src/components/advanced_text_editor/send_button/send_post_options/index.tsx @@ -24,9 +24,10 @@ type Props = { channelId: string; disabled?: boolean; onSelect: (schedulingInfo: SchedulingInfo) => void; + allowRecurring: boolean; }; -export function SendPostOptions({disabled, onSelect, channelId}: Props) { +export function SendPostOptions({disabled, onSelect, channelId, allowRecurring}: Props) { const {formatMessage} = useIntl(); const dispatch = useDispatch(); @@ -41,11 +42,7 @@ export function SendPostOptions({disabled, onSelect, channelId}: Props) { onSelect(schedulingInfo); }, [onSelect]); - const handleSelectCustomTime = useCallback((scheduledAt: number) => { - const schedulingInfo: SchedulingInfo = { - scheduled_at: scheduledAt, - }; - + const handleSelectCustomTime = useCallback((schedulingInfo: SchedulingInfo) => { onSelect(schedulingInfo); return Promise.resolve({}); }, [onSelect]); @@ -57,9 +54,10 @@ export function SendPostOptions({disabled, onSelect, channelId}: Props) { dialogProps: { channelId, onConfirm: handleSelectCustomTime, + allowRecurring, }, })); - }, [channelId, dispatch, handleSelectCustomTime]); + }, [allowRecurring, channelId, dispatch, handleSelectCustomTime]); return ( { canEdit: true, onSchedule: jest.fn(), channelId: '', + allowRecurring: true, }; it('should match snapshot', () => { diff --git a/webapp/channels/src/components/drafts/draft_actions/draft_actions.tsx b/webapp/channels/src/components/drafts/draft_actions/draft_actions.tsx index 5d8b8e9a61be..deaa58f7a181 100644 --- a/webapp/channels/src/components/drafts/draft_actions/draft_actions.tsx +++ b/webapp/channels/src/components/drafts/draft_actions/draft_actions.tsx @@ -5,6 +5,8 @@ import React, {memo, useCallback} from 'react'; import {FormattedMessage} from 'react-intl'; import {useDispatch} from 'react-redux'; +import type {SchedulingInfo} from '@mattermost/types/schedule_post'; + import {openModal} from 'actions/views/modals'; import ScheduledPostCustomTimeModal @@ -30,8 +32,9 @@ type Props = { onSend: () => void; canEdit: boolean; canSend: boolean; - onSchedule: (timestamp: number) => Promise<{error?: string}>; + onSchedule: (schedulingInfo: SchedulingInfo) => Promise<{error?: string}>; channelId: string; + allowRecurring: boolean; }; function DraftActions({ @@ -43,6 +46,7 @@ function DraftActions({ canSend, onSchedule, channelId, + allowRecurring, }: Props) { const dispatch = useDispatch(); @@ -75,9 +79,10 @@ function DraftActions({ dialogProps: { channelId, onConfirm: onSchedule, + allowRecurring, }, })); - }, [channelId, dispatch, onSchedule]); + }, [allowRecurring, channelId, dispatch, onSchedule]); return ( <> diff --git a/webapp/channels/src/components/drafts/draft_actions/schedule_post_actions/scheduled_post_actions.tsx b/webapp/channels/src/components/drafts/draft_actions/schedule_post_actions/scheduled_post_actions.tsx index 55db0d32d16d..edee5add1dbb 100644 --- a/webapp/channels/src/components/drafts/draft_actions/schedule_post_actions/scheduled_post_actions.tsx +++ b/webapp/channels/src/components/drafts/draft_actions/schedule_post_actions/scheduled_post_actions.tsx @@ -7,7 +7,8 @@ import {FormattedMessage} from 'react-intl'; import {useDispatch, useSelector} from 'react-redux'; import type {Channel} from '@mattermost/types/channels'; -import type {ScheduledPost} from '@mattermost/types/schedule_post'; +import {isRecurringScheduledPost} from '@mattermost/types/schedule_post'; +import type {ScheduledPost, SchedulingInfo} from '@mattermost/types/schedule_post'; import {fetchMissingChannels} from 'mattermost-redux/actions/channels'; import {isDeactivatedDirectChannel} from 'mattermost-redux/selectors/entities/channels'; @@ -67,7 +68,7 @@ const copyTextTooltipText = ( type Props = { scheduledPost: ScheduledPost; channel?: Channel; - onReschedule: (timestamp: number) => Promise<{error?: string}>; + onReschedule: (schedulingInfo: SchedulingInfo) => Promise<{error?: string}>; onDelete: (scheduledPostId: string) => Promise<{error?: string}>; onSend: (scheduledPostId: string) => void; onEdit: () => void; @@ -79,6 +80,8 @@ function ScheduledPostActions({scheduledPost, channel, onReschedule, onDelete, o const userTimezone = useSelector(getCurrentTimezone); const myChannelsMemberships = useSelector((state: GlobalState) => getMyChannelMemberships(state)); const isAdmin = useSelector((state: GlobalState) => isCurrentUserSystemAdmin(state)); + const isWeeklyRecurringScheduledPost = isRecurringScheduledPost(scheduledPost); + const hasFiles = Boolean(scheduledPost.file_ids?.length || scheduledPost.metadata?.files?.length); useEffect(() => { // this ensures the DM is loaded in redux store and is available @@ -101,9 +104,11 @@ function ScheduledPostActions({scheduledPost, channel, onReschedule, onDelete, o channelId: scheduledPost.channel_id, onConfirm: onReschedule, initialTime, + initialRepeatWeekly: isWeeklyRecurringScheduledPost, + allowRecurring: !hasFiles, }, })); - }, [dispatch, onReschedule, scheduledPost.channel_id, scheduledPost.scheduled_at, userTimezone]); + }, [dispatch, hasFiles, isWeeklyRecurringScheduledPost, onReschedule, scheduledPost.channel_id, scheduledPost.scheduled_at, userTimezone]); const handleDelete = useCallback(() => { dispatch(openModal({ @@ -136,8 +141,11 @@ function ScheduledPostActions({scheduledPost, channel, onReschedule, onDelete, o const showEditOption = !scheduledPost.error_code && userChannelMember && !isChannelArchived; const isDeactivatedDM = useSelector((state: GlobalState) => isDeactivatedDirectChannel(state, scheduledPost.channel_id)); - const showSendNowOption = (!scheduledPost.error_code || scheduledPost.error_code === 'unknown' || scheduledPost.error_code === 'unable_to_send') && channel && !isChannelArchived && !isDeactivatedDM && userChannelMember; - const showRescheduleOption = (!scheduledPost.error_code || scheduledPost.error_code === 'unknown' || scheduledPost.error_code === 'unable_to_send') && userChannelMember && !isChannelArchived; + const canSendNow = (!scheduledPost.error_code || scheduledPost.error_code === 'unknown' || scheduledPost.error_code === 'unable_to_send') && channel && !isChannelArchived && !isDeactivatedDM && userChannelMember; + const showRescheduleOption = (!scheduledPost.error_code || scheduledPost.error_code === 'unknown' || scheduledPost.error_code === 'unable_to_send') && userChannelMember && !isChannelArchived && !isDeactivatedDM; + + // Recurring scheduled posts can't be sent now: sending would either end the series or fork it. + const showSendNowOption = !isWeeklyRecurringScheduledPost && (isAdmin || canSendNow); return (
@@ -181,7 +189,7 @@ function ScheduledPostActions({scheduledPost, channel, onReschedule, onDelete, o } { - (isAdmin || showSendNowOption) && + showSendNowOption && => { + const onScheduleDraft = useCallback(async (schedulingInfo: SchedulingInfo): Promise<{error?: string}> => { isBeingScheduled.current = true; - await handleOnSend(item as PostDraft, {scheduled_at: scheduledAt}); + await handleOnSend(item as PostDraft, schedulingInfo); return Promise.resolve({}); }, [item, handleOnSend]); const draftActions = useMemo(() => { - if (!channel) { + if (!channel || isScheduledPost) { return null; } return ( @@ -236,6 +238,7 @@ function DraftRow({ canEdit={canEdit} canSend={canSend} onSchedule={onScheduleDraft} + allowRecurring={!draftHasAttachments(item)} /> ); }, [ @@ -245,6 +248,8 @@ function DraftRow({ goToMessage, handleOnDelete, handleOnSend, + isScheduledPost, + item, user.id, onScheduleDraft, ]); @@ -253,12 +258,14 @@ function DraftRow({ setIsEditing(false); }, []); - const handleSchedulePostOnReschedule = useCallback(async (updatedScheduledAtTime: number) => { + const handleSchedulePostOnReschedule = useCallback(async (schedulingInfo: SchedulingInfo) => { handleCancelEdit(); const updatedScheduledPost: ScheduledPost = { ...(item as ScheduledPost), - scheduled_at: updatedScheduledAtTime, + scheduled_at: schedulingInfo.scheduled_at, + repeat_type: schedulingInfo.repeat_type, + repeat_timezone: schedulingInfo.repeat_timezone, }; const result = await dispatch(updateScheduledPost(updatedScheduledPost, connectionId)); @@ -393,6 +400,7 @@ function DraftRow({ timestamp={timestamp} remote={isRemote || false} error={postError || serverError?.message} + repeatsWeekly={isWeeklyRecurringScheduledPost} /> {isEditing && ( new Date(timestamp), [timestamp]); @@ -104,6 +106,18 @@ function PanelHeader({ }
+ {kind === 'scheduledPost' && repeatsWeekly && !error && ( + + )} + /> + )} {kind === 'draft' && !error && ( { + return async (dispatch: DispatchFunc, getState: GetStateFunc) => { try { const updatedScheduledPost = await Client4.updateScheduledPost(scheduledPost, connectionId); + const teamId = getScheduledPostTeamId(getState(), updatedScheduledPost.data); dispatch({ type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, data: { scheduledPost: updatedScheduledPost.data, + teamId, }, }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/scheduled_posts.test.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/scheduled_posts.test.ts new file mode 100644 index 000000000000..30fcaa3f4000 --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/scheduled_posts.test.ts @@ -0,0 +1,88 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {ScheduledPost} from '@mattermost/types/schedule_post'; + +import {ScheduledPostTypes} from 'mattermost-redux/action_types'; + +import reducer from './scheduled_posts'; + +describe('scheduled_posts reducer', () => { + const initialState = reducer(undefined, {type: ''} as any); + + function makeScheduledPost(overrides: Partial): ScheduledPost { + return { + id: 'post1', + channel_id: 'channel1', + scheduled_at: 100, + ...overrides, + } as ScheduledPost; + } + + describe('SCHEDULED_POST_UPDATED', () => { + it('should add an errored post to its team error list', () => { + const scheduledPost = makeScheduledPost({error_code: 'unable_to_send'}); + + const state = reducer(initialState, { + type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, + data: {scheduledPost, teamId: 'team1'}, + }); + + expect(state.errorsByTeamId.team1).toEqual(['post1']); + }); + + it('should fall back to directChannels when there is no team', () => { + const scheduledPost = makeScheduledPost({error_code: 'unable_to_send'}); + + const state = reducer(initialState, { + type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, + data: {scheduledPost, teamId: undefined}, + }); + + expect(state.errorsByTeamId.directChannels).toEqual(['post1']); + }); + + it('should remove a no-longer-errored post from every team error list', () => { + const erroredPost = makeScheduledPost({error_code: 'unable_to_send'}); + let state = reducer(initialState, { + type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, + data: {scheduledPost: erroredPost, teamId: 'team1'}, + }); + + const recoveredPost = makeScheduledPost({error_code: undefined}); + state = reducer(state, { + type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, + data: {scheduledPost: recoveredPost, teamId: 'team1'}, + }); + + expect(state.errorsByTeamId.team1).toEqual([]); + }); + + it('should not duplicate a post already in its team error list, keeping the state reference', () => { + const scheduledPost = makeScheduledPost({error_code: 'unable_to_send'}); + const state = reducer(initialState, { + type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, + data: {scheduledPost, teamId: 'team1'}, + }); + + const newState = reducer(state, { + type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, + data: {scheduledPost, teamId: 'team1'}, + }); + + expect(newState.errorsByTeamId.team1).toEqual(['post1']); + expect(newState.errorsByTeamId).toBe(state.errorsByTeamId); + }); + + it('should keep the error state reference when a post without errors is updated', () => { + const scheduledPost = makeScheduledPost({error_code: undefined}); + + const state = reducer(initialState, { + type: ScheduledPostTypes.SCHEDULED_POST_UPDATED, + data: {scheduledPost, teamId: 'team1'}, + }); + + expect(state.errorsByTeamId).toBe(initialState.errorsByTeamId); + }); + }); +}); diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/scheduled_posts.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/scheduled_posts.ts index 5aa0e7b651fb..12aa74539618 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/scheduled_posts.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/scheduled_posts.ts @@ -8,6 +8,8 @@ import type {ScheduledPost, ScheduledPostsState} from '@mattermost/types/schedul import type {MMReduxAction} from 'mattermost-redux/action_types'; import {ScheduledPostTypes, UserTypes} from 'mattermost-redux/action_types'; +const emptyList: string[] = []; + function byId(state: ScheduledPostsState['byId'] = {}, action: MMReduxAction) { switch (action.type) { case ScheduledPostTypes.SCHEDULED_POSTS_RECEIVED: { @@ -24,13 +26,7 @@ function byId(state: ScheduledPostsState['byId'] = {}, action: MMReduxAction) { return newState; } - case ScheduledPostTypes.SINGLE_SCHEDULED_POST_RECEIVED: { - const scheduledPost = action.data.scheduledPost; - return { - ...state, - [scheduledPost.id]: scheduledPost, - }; - } + case ScheduledPostTypes.SINGLE_SCHEDULED_POST_RECEIVED: case ScheduledPostTypes.SCHEDULED_POST_UPDATED: { const scheduledPost = action.data.scheduledPost; return { @@ -68,21 +64,12 @@ function byTeamId(state: ScheduledPostsState['byTeamId'] = {}, action: MMReduxAc case ScheduledPostTypes.SINGLE_SCHEDULED_POST_RECEIVED: { const scheduledPost = action.data.scheduledPost as ScheduledPost; const teamId = action.data.teamId || 'directChannels'; + const existingScheduledPostIds = state[teamId] || emptyList; - const newState = {...state}; - - const existingIndex = newState[teamId].findIndex((existingScheduledPostId) => existingScheduledPostId === scheduledPost.id); - if (existingIndex >= 0) { - newState[teamId].splice(existingIndex, 1); - } - - if (newState[teamId]) { - newState[teamId] = [...newState[teamId], scheduledPost.id]; - } else { - newState[teamId] = [scheduledPost.id]; - } - - return newState; + return { + ...state, + [teamId]: [...existingScheduledPostIds.filter((existingScheduledPostId) => existingScheduledPostId !== scheduledPost.id), scheduledPost.id], + }; } case ScheduledPostTypes.SCHEDULED_POST_DELETED: { const scheduledPost = action.data.scheduledPost as ScheduledPost; @@ -126,25 +113,32 @@ function errorsByTeamId(state: ScheduledPostsState['errorsByTeamId'] = {}, actio return newState; } - case ScheduledPostTypes.SINGLE_SCHEDULED_POST_RECEIVED: { - let changed = false; - + case ScheduledPostTypes.SINGLE_SCHEDULED_POST_RECEIVED: + case ScheduledPostTypes.SCHEDULED_POST_UPDATED: { + // A scheduled post's channel (and so its team) can't change, so both actions reduce to + // keeping the post in its team's error list exactly when it has an error code. const teamId = action.data.teamId || 'directChannels'; - const newState = {...state}; - if (!newState[teamId]) { - newState[teamId] = []; - } - + const existingScheduledPostIds = state[teamId] || emptyList; const scheduledPost = action.data.scheduledPost as ScheduledPost; if (scheduledPost.error_code) { - const alreadyExists = newState[teamId].find((scheduledPostId) => scheduledPostId === scheduledPost.id); - if (!alreadyExists) { - newState[teamId] = [...newState[teamId], scheduledPost.id]; - changed = true; + if (!existingScheduledPostIds.includes(scheduledPost.id)) { + return { + ...state, + [teamId]: [...existingScheduledPostIds, scheduledPost.id], + }; } + + return state; } - return changed ? newState : state; + if (!existingScheduledPostIds.includes(scheduledPost.id)) { + return state; + } + + return { + ...state, + [teamId]: existingScheduledPostIds.filter((scheduledPostId) => scheduledPostId !== scheduledPost.id), + }; } case ScheduledPostTypes.SCHEDULED_POST_DELETED: { let changed = false; @@ -201,23 +195,17 @@ function byChannelOrThreadId(state: ScheduledPostsState['byChannelOrThreadId'] = } case ScheduledPostTypes.SINGLE_SCHEDULED_POST_RECEIVED: { const scheduledPost = action.data.scheduledPost; - const newState = {...state}; const id = scheduledPost.root_id || scheduledPost.channel_id; + const existingScheduledPostIds = state[id] || emptyList; - if (!newState[id]) { - newState[id] = [scheduledPost.id]; - return newState; - } - - let changed = false; - const existingIndex = newState[id].findIndex((scheduledPostId) => scheduledPostId === scheduledPost.id); - - if (existingIndex) { - newState[id] = [...newState[id], scheduledPost.id]; - changed = true; + if (existingScheduledPostIds.includes(scheduledPost.id)) { + return state; } - return changed ? newState : state; + return { + ...state, + [id]: [...existingScheduledPostIds, scheduledPost.id], + }; } case ScheduledPostTypes.SCHEDULED_POST_DELETED: { const scheduledPost = action.data.scheduledPost; @@ -229,6 +217,9 @@ function byChannelOrThreadId(state: ScheduledPostsState['byChannelOrThreadId'] = const newState = {...state}; const index = newState[id].findIndex((scheduledPostId) => scheduledPostId === scheduledPost.id); + if (index < 0) { + return state; + } newState[id] = [...newState[id]]; newState[id].splice(index, 1); diff --git a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/scheduled_posts.test.ts b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/scheduled_posts.test.ts new file mode 100644 index 000000000000..268e5b400947 --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/scheduled_posts.test.ts @@ -0,0 +1,147 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {ScheduledPost} from '@mattermost/types/schedule_post'; +import type {GlobalState} from '@mattermost/types/store'; + +import {getScheduledPostTeamId, isRecurringScheduledPostsEnabled, showChannelOrThreadScheduledPostIndicator} from './scheduled_posts'; + +describe('getScheduledPostTeamId', () => { + const scheduledPost = {id: 'post1', channel_id: 'channel1'} as ScheduledPost; + + function makeState(channels: Record, byTeamId: Record): GlobalState { + return { + entities: { + channels: {channels}, + scheduledPosts: {byTeamId}, + }, + } as unknown as GlobalState; + } + + it('should return the team of the post channel when the channel is loaded', () => { + const state = makeState({channel1: {team_id: 'team1'}}, {}); + expect(getScheduledPostTeamId(state, scheduledPost)).toBe('team1'); + }); + + it('should return an empty team for loaded DM/GM channels', () => { + const state = makeState({channel1: {team_id: ''}}, {}); + expect(getScheduledPostTeamId(state, scheduledPost)).toBe(''); + }); + + it('should fall back to the bucket already holding the post when the channel is not loaded', () => { + const state = makeState({}, {team2: ['other', 'post1'], directChannels: []}); + expect(getScheduledPostTeamId(state, scheduledPost)).toBe('team2'); + }); + + it('should return undefined when the channel is not loaded and no bucket holds the post', () => { + const state = makeState({}, {team2: ['other']}); + expect(getScheduledPostTeamId(state, scheduledPost)).toBeUndefined(); + }); +}); + +describe('isRecurringScheduledPostsEnabled', () => { + function makeState(scheduledPosts: string, featureFlag: string, isLicensed: string): GlobalState { + return { + entities: { + general: { + config: { + ScheduledPosts: scheduledPosts, + FeatureFlagRecurringScheduledPosts: featureFlag, + }, + license: {IsLicensed: isLicensed}, + }, + }, + } as unknown as GlobalState; + } + + it('should be enabled only when scheduled posts, the license and the feature flag all allow it', () => { + expect(isRecurringScheduledPostsEnabled(makeState('true', 'true', 'true'))).toBe(true); + expect(isRecurringScheduledPostsEnabled(makeState('false', 'true', 'true'))).toBe(false); + expect(isRecurringScheduledPostsEnabled(makeState('true', 'false', 'true'))).toBe(false); + expect(isRecurringScheduledPostsEnabled(makeState('true', 'true', 'false'))).toBe(false); + }); + + it('should be disabled when the feature flag is missing from the config', () => { + const state = { + entities: { + general: { + config: {ScheduledPosts: 'true'}, + license: {IsLicensed: 'true'}, + }, + }, + } as unknown as GlobalState; + + expect(isRecurringScheduledPostsEnabled(state)).toBe(false); + }); +}); + +describe('showChannelOrThreadScheduledPostIndicator', () => { + function makeState(scheduledPosts: ScheduledPost[]): GlobalState { + return { + entities: { + scheduledPosts: { + byId: Object.fromEntries(scheduledPosts.map((post) => [post.id, post])), + byChannelOrThreadId: {channel1: scheduledPosts.map((post) => post.id)}, + }, + }, + } as unknown as GlobalState; + } + + function makePost(id: string, overrides: Partial = {}): ScheduledPost { + return {id, channel_id: 'channel1', ...overrides} as ScheduledPost; + } + + it('should return null when every scheduled post is recurring', () => { + const state = makeState([ + makePost('post1', {repeat_type: 'weekly'}), + makePost('post2', {repeat_type: 'weekly'}), + ]); + + expect(showChannelOrThreadScheduledPostIndicator(state, 'channel1')).toBeNull(); + }); + + it('should show the indicator when the only scheduled post is a one-shot', () => { + const scheduledPost = makePost('post1'); + const state = makeState([scheduledPost]); + + expect(showChannelOrThreadScheduledPostIndicator(state, 'channel1')).toEqual({ + count: 1, + scheduledPost, + }); + }); + + it('should count recurring posts alongside a non-recurring one', () => { + const state = makeState([ + makePost('post1', {repeat_type: 'weekly'}), + makePost('post2'), + ]); + + expect(showChannelOrThreadScheduledPostIndicator(state, 'channel1')).toEqual({ + count: 2, + }); + }); + + it('should ignore errored posts', () => { + const state = makeState([ + makePost('post1', {error_code: 'unknown'}), + makePost('post2', {repeat_type: 'weekly'}), + ]); + + expect(showChannelOrThreadScheduledPostIndicator(state, 'channel1')).toBeNull(); + }); + + it('should ignore ids without a loaded post', () => { + const scheduledPost = makePost('post1'); + const state = makeState([scheduledPost]); + (state.entities.scheduledPosts.byChannelOrThreadId.channel1 as string[]).push('dangling'); + + expect(showChannelOrThreadScheduledPostIndicator(state, 'channel1')).toEqual({ + count: 1, + scheduledPost, + }); + }); + + it('should return null for an empty channel', () => { + expect(showChannelOrThreadScheduledPostIndicator(makeState([]), 'channel2')).toBeNull(); + }); +}); diff --git a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/scheduled_posts.ts b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/scheduled_posts.ts index 760c2d4bb552..203b1f4c07e3 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/scheduled_posts.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/scheduled_posts.ts @@ -3,10 +3,12 @@ import type {ClientLicense, ClientConfig} from '@mattermost/types/config'; import type {ScheduledPost, ScheduledPostsState} from '@mattermost/types/schedule_post'; +import {isRecurringScheduledPost} from '@mattermost/types/schedule_post'; import type {GlobalState} from '@mattermost/types/store'; import {createSelector} from 'mattermost-redux/selectors/create_selector'; -import {getConfig, getLicense} from 'mattermost-redux/selectors/entities/general'; +import {getConfig, getFeatureFlagValue, getLicense} from 'mattermost-redux/selectors/entities/general'; +import {getTeamIdByChannelId} from 'mattermost-redux/selectors/entities/teams'; const emptyList: string[] = []; @@ -55,24 +57,42 @@ export function getScheduledPostsByTeamCount(state: GlobalState, teamId: string, return count; } +// getScheduledPostTeamId resolves the team bucket a scheduled post belongs to: its channel's team +// when the channel is loaded, otherwise whichever bucket already holds the post. A falsy result +// means the directChannels bucket. +export function getScheduledPostTeamId(state: GlobalState, scheduledPost: ScheduledPost): string | undefined { + const teamId = getTeamIdByChannelId(state, scheduledPost.channel_id); + if (teamId !== undefined) { + return teamId; + } + + const byTeamId = state.entities.scheduledPosts.byTeamId; + return Object.keys(byTeamId).find((currentTeamId) => byTeamId[currentTeamId].includes(scheduledPost.id)); +} + export function hasScheduledPostError(state: GlobalState, teamId: string) { return state.entities.scheduledPosts.errorsByTeamId[teamId]?.length > 0 || state.entities.scheduledPosts.errorsByTeamId.directChannels?.length > 0; } -export function showChannelOrThreadScheduledPostIndicator(state: GlobalState, channelOrThreadId: string): ChannelScheduledPostIndicatorData { +// Returns the indicator data for a channel or thread, or null when the indicator must not show. +// A recurring series always has a next occurrence, so recurring posts never keep the indicator +// pinned above the composer on their own: at least one non-recurring scheduled post is required. +export function showChannelOrThreadScheduledPostIndicator(state: GlobalState, channelOrThreadId: string): ChannelScheduledPostIndicatorData | null { const allChannelScheduledPosts = state.entities.scheduledPosts.byChannelOrThreadId[channelOrThreadId] || emptyList; - const eligibleScheduledPosts = allChannelScheduledPosts.filter((scheduledPostId: string) => { - const scheduledPost = state.entities.scheduledPosts.byId[scheduledPostId]; - return !scheduledPost?.error_code; - }); + const eligibleScheduledPosts = allChannelScheduledPosts. + map((scheduledPostId: string) => state.entities.scheduledPosts.byId[scheduledPostId]). + filter((scheduledPost): scheduledPost is ScheduledPost => scheduledPost !== undefined && !scheduledPost.error_code); - const data = { + if (!eligibleScheduledPosts.some((scheduledPost) => !isRecurringScheduledPost(scheduledPost))) { + return null; + } + + const data: ChannelScheduledPostIndicatorData = { count: eligibleScheduledPosts.length, - } as ChannelScheduledPostIndicatorData; + }; if (data.count === 1) { - const scheduledPostId = eligibleScheduledPosts[0]; - data.scheduledPost = state.entities.scheduledPosts.byId[scheduledPostId]; + data.scheduledPost = eligibleScheduledPosts[0]; } return data; @@ -86,3 +106,12 @@ export const isScheduledPostsEnabled: (a: GlobalState) => boolean = createSelect return config.ScheduledPosts === 'true' && license.IsLicensed === 'true'; }, ); + +export const isRecurringScheduledPostsEnabled: (a: GlobalState) => boolean = createSelector( + 'isRecurringScheduledPostsEnabled', + isScheduledPostsEnabled, + (state: GlobalState) => getFeatureFlagValue(state, 'RecurringScheduledPosts'), + (scheduledPostsEnabled: boolean, featureFlagValue: string | undefined): boolean => { + return scheduledPostsEnabled && featureFlagValue === 'true'; + }, +); diff --git a/webapp/channels/src/types/store/draft.ts b/webapp/channels/src/types/store/draft.ts index f03be6c221eb..f8530b16614a 100644 --- a/webapp/channels/src/types/store/draft.ts +++ b/webapp/channels/src/types/store/draft.ts @@ -54,6 +54,10 @@ export function isPostDraftEmpty(draft: PostDraft): boolean { return !hasMessage && !hasAttachment && !hasUploadingFiles && !hasPriority && !hasBurnOnRead; } +export function draftHasAttachments(draft: Pick): boolean { + return draft.fileInfos?.length > 0 || draft.uploadsInProgress?.length > 0; +} + export function scheduledPostToPostDraft(scheduledPost: ScheduledPost): PostDraft { return { message: scheduledPost.message, diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index aba07cc07553..59b3c1f0db55 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -140,6 +140,7 @@ export type ClientConfig = { FeatureFlagSessionAttributes: string; FeatureFlagPostAttributes: string; FeatureFlagDiscoverableChannels: string; + FeatureFlagRecurringScheduledPosts: string; ForgotPasswordLink: string; GiphySdkKey: string; diff --git a/webapp/platform/types/src/schedule_post.ts b/webapp/platform/types/src/schedule_post.ts index 1296c14e8b3d..1541a9da61c9 100644 --- a/webapp/platform/types/src/schedule_post.ts +++ b/webapp/platform/types/src/schedule_post.ts @@ -6,16 +6,24 @@ import type {Post} from './posts'; export type ScheduledPostErrorCode = 'unknown' | 'channel_archived' | 'channel_not_found' | 'user_missing' | 'user_deleted' | 'no_channel_permission' | 'no_channel_member' | 'thread_deleted' | 'unable_to_send' | 'invalid_post'; +export type ScheduledPostRepeatType = '' | 'weekly'; + export type SchedulingInfo = { scheduled_at: number; processed_at?: number; error_code?: ScheduledPostErrorCode; + repeat_type?: ScheduledPostRepeatType; + repeat_timezone?: string; }; export type ScheduledPost = Omit & SchedulingInfo & { id: string; }; +export function isRecurringScheduledPost(schedulingInfo: Pick): boolean { + return schedulingInfo.repeat_type === 'weekly'; +} + export type ScheduledPostsState = { byId: { [scheduledPostId: string]: ScheduledPost | undefined; @@ -45,6 +53,8 @@ export function scheduledPostFromPost(post: Post, schedulingInfo: SchedulingInfo metadata: post.metadata, priority: post.metadata.priority, type: post.type, + repeat_type: schedulingInfo.repeat_type, + repeat_timezone: schedulingInfo.repeat_timezone, }; }