Add weekly recurring scheduled posts - #10025
Open
nickmisasi wants to merge 6 commits into
Open
Conversation
Mirrors server support for weekly recurring scheduled posts (mattermost#37746): - ScheduledPost schema v21 with repeat_type/repeat_timezone columns + migration - ScheduledPostModel fields, toApi() now round-trips repeat fields and the previously-dropped type field (PUT is a full overwrite server-side) - Transformer copies repeat fields and overwrites error_code instead of merging, so a recurring post that recovers clears its error state - updateScheduledPost takes UpdateSchedulingInfo so reschedule can change recurrence; weekly requires a timezone at compile time Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Gated on server >= 11.10 (no feature flag; older servers silently ignore the repeat fields). The scheduled-post options sheet and the reschedule screen share getScheduledPostRecurrence(), which resolves the recurrence timezone (manual > automatic > UTC) matching the webapp. Rescheduling a weekly post pre-checks the toggle; unchecking converts it to one-time. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Scheduled-post rows show an info tag next to the Send on... header when the post repeats weekly and is not in an error state, matching the webapp. The Send option is absent for recurring posts (sending now would end or fork the series), and the options sheet height is now derived from the same booleans that drive rendering. isRecurringScheduledPost() is the canonical recurrence predicate in @utils/scheduled_post. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
The server PR now gates recurrence behind the default-off FeatureFlagRecurringScheduledPosts flag (only the off-to-weekly transition is blocked when disabled) and rejects weekly posts with file attachments, since files are bound to the first post they are attached to. Replace the version gate with the flag check and hide the Repeat weekly toggle when the draft has attachments; when the toggle is not offered, the reschedule save omits the recurrence fields so an existing series is preserved. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Documentation Impact Analysis — updates neededDocumentation Impact AnalysisOverall Assessment: Documentation Updates Recommended Changes SummaryThis PR adds recurring weekly scheduled posts to the Mattermost mobile app, gated behind the Documentation Impact Details
Recommended Actions
ConfidenceHigh — The PR introduces clearly new user-facing behavior (toggle, badge, suppressed action, attachment constraint) with dedicated i18n strings and a named feature flag. The existing
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds weekly recurring scheduled posts. It updates scheduling and rescheduling flows, persists recurrence type and timezone data, applies a feature flag, and updates scheduled-post actions and header labels. ChangesRecurring scheduled posts
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DraftInput
participant ScheduledPostPicker
participant RescheduledDraft
participant UpdateScheduledPost
participant ScheduledPostClient
DraftInput->>ScheduledPostPicker: pass draft attachment state
ScheduledPostPicker->>ScheduledPostClient: create scheduled post with recurrence
RescheduledDraft->>UpdateScheduledPost: send scheduled_at and recurrence
UpdateScheduledPost->>ScheduledPostClient: submit merged scheduled-post payload
ScheduledPostClient-->>UpdateScheduledPost: return update result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
app/screens/draft_scheduled_post_options/index.test.tsx (1)
122-122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the required test-name format.
Rename this test to start with
should, for exampleit('should not render the send option for a recurring scheduled post', ...).As per coding guidelines, “Use
it('should...')test names.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/screens/draft_scheduled_post_options/index.test.tsx` at line 122, Rename the test case in the recurring scheduled post coverage to use the required “should” naming convention, changing its description to start with “should not render...” while preserving the test behavior and assertions.Source: Coding guidelines
app/components/draft_scheduled_post_header/draft_scheduled_post_header.tsx (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
defineMessages()for the new recurrence label.Replace
defineMessagewith adefineMessages()object and pass itsrepeatsWeeklydescriptor toTag. Runnpm run i18n-extractand confirm thatassets/base/i18n/en.jsoncontains the new message.As per coding guidelines, “Define new messages with
defineMessages()and runnpm run i18n-extractto updateen.json.”Proposed change
-import {defineMessage, useIntl} from 'react-intl'; +import {defineMessages, useIntl} from 'react-intl'; -const repeatsWeeklyLabel = defineMessage({ - id: 'scheduled_post.header.repeats_weekly', - defaultMessage: 'Repeats weekly', +const messages = defineMessages({ + repeatsWeekly: { + id: 'scheduled_post.header.repeats_weekly', + defaultMessage: 'Repeats weekly', + }, }); - message={repeatsWeeklyLabel} + message={messages.repeatsWeekly}Also applies to: 42-45, 176-177
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/draft_scheduled_post_header/draft_scheduled_post_header.tsx` at line 5, Replace the standalone defineMessage usage in DraftScheduledPostHeader with a defineMessages() descriptor object containing repeatsWeekly, pass that descriptor to Tag, and run npm run i18n-extract so assets/base/i18n/en.json includes the new message.Source: Coding guidelines
app/utils/scheduled_post/scheduled_post.test.ts (1)
347-369: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the automatic-timezone weekly path.
Line 349 passes an automatic timezone only when recurrence is disabled. That path does not call
getTimezone(). Add a weekly case that expectsAmerica/New_YorkwhenuseAutomaticTimezoneis true. A regression in automatic timezone selection can otherwise pass these tests.As per coding guidelines, test actual implementation behavior.
Proposed test
+ it('should use the automatic timezone when enabled', () => { + expect(getScheduledPostRecurrence(true, {useAutomaticTimezone: true, automaticTimezone: 'America/New_York', manualTimezone: 'Asia/Tokyo'})).toEqual({ + repeat_type: 'weekly', + repeat_timezone: 'America/New_York', + }); + }); +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/utils/scheduled_post/scheduled_post.test.ts` around lines 347 - 369, Extend the getScheduledPostRecurrence tests with an enabled-weekly case using useAutomaticTimezone: true and automaticTimezone: 'America/New_York', asserting repeat_type 'weekly' and repeat_timezone 'America/New_York'. Keep the existing disabled-recurrence and manual-timezone cases unchanged.Source: Coding guidelines
app/utils/scheduled_post/index.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
defineMessages()for the new label.Line 81 defines a new message with
defineMessage(). Define it withdefineMessages()and retain the exportedrepeatWeeklyLabelbinding. Runnpm run i18n-extractafter the change.As per coding guidelines, define new messages with
defineMessages()and runnpm run i18n-extract.Proposed change
-import {defineMessage, defineMessages, type IntlShape} from 'react-intl'; +import {defineMessages, type IntlShape} from 'react-intl'; -export const repeatWeeklyLabel = defineMessage({ - id: 'scheduled_post.repeat_weekly', - defaultMessage: 'Repeat weekly', +export const {repeatWeekly: repeatWeeklyLabel} = defineMessages({ + repeatWeekly: { + id: 'scheduled_post.repeat_weekly', + defaultMessage: 'Repeat weekly', + }, });Also applies to: 81-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/utils/scheduled_post/index.ts` at line 4, Update the new label definition near the exported repeatWeeklyLabel binding to use defineMessages() instead of defineMessage(), while retaining the repeatWeeklyLabel export and its existing message metadata. Run npm run i18n-extract after making the change.Source: Coding guidelines
app/database/operator/server_data_operator/transformers/post.ts (1)
199-199: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a nullish fallback for
raw.type.Replace
||with??for this fallback. This follows the TypeScript guideline and preserves the intended absent-value semantics.Proposed change
- scheduledPost.type = raw.type || ''; + scheduledPost.type = raw.type ?? '';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/database/operator/server_data_operator/transformers/post.ts` at line 199, Update the assignment to scheduledPost.type in the post transformer to use a nullish fallback with raw.type, replacing the current truthiness-based fallback while retaining the empty-string default for null or undefined values.Source: Coding guidelines
app/screens/scheduled_post_options/scheduled_post_picker.test.tsx (1)
185-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required timer helper.
Replace
jest.runAllTimers()withadvanceTimers(). Configure fake timers withdoNotFake: ['nextTick']for this timer test.As per coding guidelines, “Use
jest.useFakeTimers({doNotFake: ['nextTick']})andadvanceTimers()for timer tests.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/screens/scheduled_post_options/scheduled_post_picker.test.tsx` around lines 185 - 206, Update the timer setup for the scheduled post picker tests around scheduleAtMondayWithRepeatWeekly to use jest.useFakeTimers({doNotFake: ['nextTick']}) and replace jest.runAllTimers() with the required advanceTimers() helper.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/actions/remote/scheduled_post.test.ts`:
- Around line 277-301: Remove the isScheduledPostModel mock from the scheduled
post integration test and leave the real predicate active. Keep the
operator.handleScheduledPosts() setup and updateScheduledPost assertions
unchanged so the test validates the actual model-to-API path.
In `@app/components/post_draft/draft_input/draft_input.test.tsx`:
- Line 175: Rename the test case near “tells the scheduled post options when the
draft has attachments” to use the required `it('should...')` naming format while
preserving its existing behavior and assertions.
In `@app/screens/draft_scheduled_post_options/index.test.tsx`:
- Around line 57-67: Update the test setup around setupServerDatabase and the
scheduled-post inserts to initialize a fresh in-memory LokiJS database in
beforeEach with autosave disabled, recreate the recurring fixture for every
test, and destroy the server database in afterEach so records and observer state
cannot leak between tests.
In `@app/screens/scheduled_post_options/scheduled_post_picker.test.tsx`:
- Around line 183-201: Update the recurrence toggle queries in
app/screens/scheduled_post_options/scheduled_post_picker.test.tsx lines 183-201
and app/screens/reschedule_draft/reschedule_draft.test.tsx lines 436-449 to use
PickerOption’s emitted ID format,
post_priority_picker_item.repeat_weekly.toggled.<state>.button, instead of the
supplied scheduled_post_options.repeat_weekly ID. Preserve each test’s expected
toggle state.
In `@app/screens/scheduled_post_options/scheduled_post_picker.tsx`:
- Around line 85-86: Update the scheduling payload construction around
getScheduledPostRecurrence to pass offerRepeatWeekly && repeatWeekly, ensuring
recurrence remains disabled if the feature gate changes while the sheet is open.
Add a test covering selecting the weekly toggle, then disabling recurrence
before submission, and verify the payload does not request weekly recurrence.
---
Nitpick comments:
In `@app/components/draft_scheduled_post_header/draft_scheduled_post_header.tsx`:
- Line 5: Replace the standalone defineMessage usage in DraftScheduledPostHeader
with a defineMessages() descriptor object containing repeatsWeekly, pass that
descriptor to Tag, and run npm run i18n-extract so assets/base/i18n/en.json
includes the new message.
In `@app/database/operator/server_data_operator/transformers/post.ts`:
- Line 199: Update the assignment to scheduledPost.type in the post transformer
to use a nullish fallback with raw.type, replacing the current truthiness-based
fallback while retaining the empty-string default for null or undefined values.
In `@app/screens/draft_scheduled_post_options/index.test.tsx`:
- Line 122: Rename the test case in the recurring scheduled post coverage to use
the required “should” naming convention, changing its description to start with
“should not render...” while preserving the test behavior and assertions.
In `@app/screens/scheduled_post_options/scheduled_post_picker.test.tsx`:
- Around line 185-206: Update the timer setup for the scheduled post picker
tests around scheduleAtMondayWithRepeatWeekly to use
jest.useFakeTimers({doNotFake: ['nextTick']}) and replace jest.runAllTimers()
with the required advanceTimers() helper.
In `@app/utils/scheduled_post/index.ts`:
- Line 4: Update the new label definition near the exported repeatWeeklyLabel
binding to use defineMessages() instead of defineMessage(), while retaining the
repeatWeeklyLabel export and its existing message metadata. Run npm run
i18n-extract after making the change.
In `@app/utils/scheduled_post/scheduled_post.test.ts`:
- Around line 347-369: Extend the getScheduledPostRecurrence tests with an
enabled-weekly case using useAutomaticTimezone: true and automaticTimezone:
'America/New_York', asserting repeat_type 'weekly' and repeat_timezone
'America/New_York'. Keep the existing disabled-recurrence and manual-timezone
cases unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de6a8ea4-c9e8-430f-9747-c8a0a4867779
📒 Files selected for processing (39)
app/actions/remote/scheduled_post.test.tsapp/actions/remote/scheduled_post.tsapp/client/rest/scheduled_post.test.tsapp/components/draft_scheduled_post/draft_scheduled_post.test.tsxapp/components/draft_scheduled_post/draft_scheduled_post.tsxapp/components/draft_scheduled_post_header/draft_scheduled_post_header.test.tsxapp/components/draft_scheduled_post_header/draft_scheduled_post_header.tsxapp/components/post_draft/draft_input/draft_input.test.tsxapp/components/post_draft/draft_input/draft_input.tsxapp/database/migration/server/index.tsapp/database/models/server/scheduled_post.tsapp/database/operator/server_data_operator/transformers/post.test.tsapp/database/operator/server_data_operator/transformers/post.tsapp/database/schema/server/index.tsapp/database/schema/server/table_schemas/scheduled_post.tsapp/database/schema/server/test.tsapp/queries/servers/scheduled_post.test.tsapp/queries/servers/scheduled_post.tsapp/routes/(bottom_sheet)/scheduled_post_options.tsxapp/screens/draft_scheduled_post_options/draft_scheduled_post_options.tsxapp/screens/draft_scheduled_post_options/index.test.tsxapp/screens/reschedule_draft/index.tsxapp/screens/reschedule_draft/indext.test.tsxapp/screens/reschedule_draft/reschedule_draft.test.tsxapp/screens/reschedule_draft/reschedule_draft.tsxapp/screens/scheduled_post_options/index.test.tsxapp/screens/scheduled_post_options/index.tsapp/screens/scheduled_post_options/scheduled_post_picker.test.tsxapp/screens/scheduled_post_options/scheduled_post_picker.tsxapp/utils/post/index.test.tsapp/utils/post/index.tsapp/utils/scheduled_post/index.tsapp/utils/scheduled_post/scheduled_post.test.tsassets/base/i18n/en.jsondocs/database/server/server.mdtest/test_helper.tstypes/api/config.d.tstypes/api/scheduled_post.d.tstypes/database/models/servers/scheduled_post.ts
Coverage Comparison Report |
- Gate the picker's recurrence payload on the toggle actually being offered, closing a race where the feature flag flips off while the sheet is open; add a regression test - Use the real isScheduledPostModel predicate in the model round-trip test instead of mocking it - Cover the automatic-timezone branch of getScheduledPostRecurrence - Align new picker tests with the advanceTimers helper, use nullish coalescing for raw.type, and fix test names to the should... format Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
mattermost-build
added
the
E2E/Run
mattermost-build
removed
the
E2E/Run
…ng-scheduled-posts-d939 Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
mattermost-build
added
E2E/Run
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mirrors weekly recurring scheduled posts (mattermost/mattermost#37746) into the mobile app. Users can flip a "Repeat weekly" toggle when scheduling or rescheduling a message; the server re-sends the post every week at the same local wall-clock time and advances the series via the existing
scheduled_post_updatedwebsocket event.Data layer
ScheduledPostschema v21 addsrepeat_type/repeat_timezonecolumns (+ migration and schema doc bump).ScheduledPostModel.toApi()round-trips the repeat fields and the previously-droppedtypefield, so rescheduling can no longer silently strip fields from the PUT body.error_codeinstead of merging it, so a recurring post that fails once and later succeeds clears its error state (previously the error was sticky forever).updateScheduledPosttakes anUpdateSchedulingInfowhose type forbids sendingrepeat_type: 'weekly'without a timezone; omitting the recurrence preserves the existing series.UI
Gating
FeatureFlagRecurringScheduledPosts: 'true'(default-off flag; pre-feature servers never send it) and the draft has no file attachments (the server rejects weekly posts with files, since files are bound to the first post they're attached to). When the toggle can't be offered, rescheduling omits the recurrence fields so an existing series is preserved.Ticket Link
Mirrors the server/webapp feature in mattermost/mattermost#37746
Checklist
Device Information
This PR was tested on: iPhone 17 Pro simulator (iOS 26.5), Android emulator (emulator-5554), against a cloud test server running the server-side PR (v11.11.0) with
FeatureFlagRecurringScheduledPostson and off.Screenshots
Release Note
To show artifacts inline, enable in settings.