mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-19 02:06:37 +08:00
deafd88fd5
* MM-68762: Add Postgres migrations for discoverable private channels Three online-safe migrations introduce the schema that supports the Discoverable Private Channels feature (PRs 2-5 of MM-68430 will land behind it): - 000175 adds Channels.Discoverable BOOLEAN NOT NULL DEFAULT FALSE. Metadata-only on Postgres >= 11; no table rewrite. - 000176 creates a partial index on (TeamId) WHERE Discoverable AND Type='P' AND DeleteAt=0 using CREATE INDEX CONCURRENTLY (-- morph:nontransactional) so the build never blocks writes on the populated Channels table. - 000177 creates the ChannelJoinRequests table with three indexes, the important one being the partial unique index on (ChannelId, UserId) WHERE Status = 'pending'. That keeps the full audit history intact while still enforcing at-most-one active pending request per (channel, user). Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add FeatureFlagDiscoverableChannels (default false) Gates the per-channel Discoverable toggle and the channel-join-request flow. Default-OFF so all PRs in the MM-68430 series can land on master without exposing partial UX. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add Discoverable + ChannelJoinRequest models - Channel gains a Discoverable bool, ChannelPatch a *bool, both serialized as 'discoverable'. Patch() applies it, Auditable() logs it, and IsValid() rejects Discoverable=true on any non-private channel so a misconfigured patch can never produce a public discoverable channel. - New ChannelJoinRequest type captures the per-row state of a non-member's request: pending -> approved | denied | withdrawn. Rows are append-only with reviewer and timestamps so the table is also the audit trail. IsValid() enforces: * recognized status, * Message and DenialReason rune limits, * DenialReason only on denied rows (no orphan reasons), * reviewer + reviewed_at present for any terminal review (approved / denied) but not for self-service withdrawal. - Two new WebSocket event constants -- channel_join_request_created and channel_join_request_updated -- that later PRs broadcast on the admin queue and the requester's My Pending Requests panel. Unit tests cover Patch(), the new IsValid() rule on Discoverable, the PreSave/PreUpdate timestamp behavior on ChannelJoinRequest, and every IsValid branch including the reviewer-required-on-review invariant. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add discoverable-channel permissions Two new channel-scoped permissions, each independently rebindable from the System Console: - manage_private_channel_discoverability gates the per-channel toggle so admins can restrict who can flip discoverability without also handing out manage_private_channel_properties. - manage_channel_join_requests gates the queue list / approve / deny / count endpoints (added in PR 2). Both are added to the channel_admin role bootstrap so new deployments get them by default, and a new permissions migration (add_discoverable_channel_permissions) grants them to channel_admin, team_admin and system_admin scheme roles on existing deployments. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add ChannelJoinRequestStore and wire Discoverable into channel store - channelSliceColumns / channelToSlice / updateChannelT now include the new Discoverable column so Save() and Update() round-trip the field. Existing select paths inherit the column automatically because every read goes through channelSliceColumns. - New ChannelJoinRequestStore interface and SQL implementation: Save / Get / GetPendingForChannelAndUser / GetForChannel / GetForUser / Update / CountPending. Save translates the idx_channeljoinrequests_pending_unique partial unique index violation into store.ErrConflict so the app layer (PR 2) can return 409 without re-parsing pq errors. - Storetest suite at storetest/channel_join_request_store.go is invoked from sqlstore via the existing StoreTest harness; covers insert / partial-unique conflict / re-insert after withdrawal / NotFound / status filtering / pagination with TotalCount / Update / CountPending. - Mocks and retrylayer / timerlayer are regenerated via make store-mocks and go generate ./channels/store -- no hand-written generator output. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add TS types for Discoverable channels + join requests webapp/platform/types: - Channel.discoverable?: boolean alongside existing policy_enforced / policy_is_active so the web client sees the same wire shape the server emits. - ChannelJoinRequest, ChannelJoinRequestStatus, ChannelJoinRequestList, GetChannelJoinRequestsOptions for the API contract surfaced in PR 2. webapp/platform/client: - WebSocketEvents enum gains ChannelJoinRequestCreated and ChannelJoinRequestUpdated so PR 3 can hang WS handlers off them without redeclaring constants. These are model-only updates with no UI consumer yet; PR 3 introduces the toggle, request flow, and admin queue surfaces. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Split ChannelJoinRequests indexes into concurrent migrations The mattermost-govet concurrentIndex lint check enforces CREATE INDEX CONCURRENTLY on every CREATE INDEX statement, even on an empty freshly-created table where it would be a no-op. The original 000177 file inlined three CREATE INDEX statements; that failed check-style. Mirror the convention used by 000166_create_views + 000167_create_views_channel_id_delete_at_index: keep the CREATE TABLE in its own (transactional) file, and move each index into a separate nontransactional file that runs CREATE INDEX CONCURRENTLY. Verified locally against Postgres 15 that all four new migrations apply in order and the storetest suite (partial unique constraint + paged list + count) still passes. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Wire new permission migration into test fixtures Two CI test surfaces missed when the channel_admin role and the permission-migration list gained the new manage_private_channel_discoverability and manage_channel_join_requests entries: - testlib/store.go: the shared mocked SystemStore used by SetupWithStoreMock / SetupEnterpriseWithStoreMock needs an explicit GetByName expectation for every migration key (because the mock panics on unexpected calls). Add the new MigrationKeyAddDiscoverableChannelPermissions key so TestCreateOrUpdateAccessControlPolicy, the elasticsearch aggregation_job_test, and every other mock-store test stop panicking on server bootstrap. - cmd/mmctl/commands/permissions_test.go: TestResetPermissionsCmd hard-codes the channel_admin default permission list and expects PatchRole to be called with exactly that slice. Extend the expected slice with the two new permission ids so the mmctl reset path stays in sync with the role bootstrap. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Register new idx_channels_discoverable_team in TestGetSchemaDefinition The schema-dump test asserts an exact index count and definition map for the channels table. Migration 000176 added idx_channels_discoverable_team — a partial btree on (teamid) gated by discoverable=true AND type='P' AND deleteat=0. Bump the expected count from 12 to 13 and add the index's CREATE INDEX definition as produced by pg_indexes (note: type is cast to channel_type, the existing domain). Verified locally against Postgres 15. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Fix golangci-lint findings in ChannelJoinRequest store Two golangci-lint findings on the freshly-added files: - sqlstore/channel_join_request_store.go:133 (modernize): collapse the 'if page < 0 { page = 0 }' clamp into max(opts.Page, 0). - storetest/channel_join_request_store.go:243 (govet shadow): the inner Save loop redeclared err with :=, shadowing the outer err captured from the first CountPending call. Switch to plain assignment so the same err is reused. Verified locally with golangci-lint v2.11.4 across public/..., channels/app/..., channels/store/..., channels/testlib/... and cmd/mmctl/commands/... — 0 issues. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Sync channel_admin bootstrap with TestDoAdvancedPermissionsMigration app_test.go pins the exact list of permissions the channel_admin role is expected to hold after DoAdvancedPermissionsMigration completes. The role bootstrap in role.go grew two entries (manage_private_channel_discoverability and manage_channel_join_requests), so the test's expected slice needs the same two entries appended in the same order, otherwise assert.Equal fails on slice ordering. This is the same class of fix as the mmctl/permissions_test.go change in a previous commit -- two parallel test fixtures encode the channel_admin defaults and have to be updated in lockstep with the bootstrap. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add English translations for new model error keys 12 keys were emitted by the new Discoverable + ChannelJoinRequest validation paths but had no en.json entry, which trips i18n-check on CI. Add the missing entries with one-line English copy that mirrors adjacent model errors (Invalid <field>., Create at must be a valid time., etc.). The new entries are: - model.channel.is_valid.discoverable.app_error - model.channel_join_request.is_valid.channel_id.app_error - model.channel_join_request.is_valid.create_at.app_error - model.channel_join_request.is_valid.denial_reason.app_error - model.channel_join_request.is_valid.denial_reason_status.app_error - model.channel_join_request.is_valid.id.app_error - model.channel_join_request.is_valid.message.app_error - model.channel_join_request.is_valid.reviewed_by.app_error - model.channel_join_request.is_valid.reviewer.app_error - model.channel_join_request.is_valid.status.app_error - model.channel_join_request.is_valid.update_at.app_error - model.channel_join_request.is_valid.user_id.app_error Generated through 'make i18n-extract'; verified clean with 'make i18n-check'. Per the workspace rule, only en.json was modified -- no other locale files. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Address CodeRabbit review: stable pagination + redact denial reason from audit log Two production-code findings from CodeRabbit on the freshly-added ChannelJoinRequest server code: - sqlstore/channel_join_request_store.go (GetForChannel / GetForUser): OrderBy("CreateAt DESC") alone is unstable when two rows share a millisecond (NewId is monotonic-ish but CreateAt is millisecond resolution), so offset paging could duplicate or skip rows between pages. Add Id DESC as a deterministic tie-breaker on both list queries. - model/channel_join_request.Auditable: the denial reason is admin-typed free text and could carry sensitive content. Mirror the existing has_message pattern by emitting has_denial_reason as a boolean presence flag instead of the raw value. Reviewer id, review timestamp, and status are still logged, so the audit trail keeps every piece needed for compliance review. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Tighten model tests per CodeRabbit review Two test-only findings from CodeRabbit: - TestChannelJoinRequestPreUpdateAdvancesUpdateAt previously asserted GreaterOrEqual(r.UpdateAt, originalCreate). Because validRequest initialises UpdateAt to GetMillis() (same call site as CreateAt), a no-op PreUpdate would still pass that check. Seed r.UpdateAt = 1 before calling PreUpdate() and assert Greater(r.UpdateAt, int64(1)) so any regression that drops the GetMillis assignment fails the test. - TestChannelIsValidDiscoverable did not cover ChannelTypeGroup. Add the case alongside ChannelTypeOpen and ChannelTypeDirect so the contract that 'only ChannelTypePrivate accepts Discoverable=true' is fully pinned across all four channel types. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Mock ChannelJoinRequest accessor in retrylayer test retrylayer_test.go's genStore() helper mocks every Store() accessor because retrylayer.New() wraps the entire surface. The new ChannelJoinRequest() method I added on Store was missing from the mock, so TestRetry/on_regular_error_should_not_retry panicked with 'Unexpected Method Call ChannelJoinRequest()' on Postgres shard 0. Add the mock alongside the other accessors. No production code change. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>
mmctl
mmctl is a remote CLI tool that lets you manage your Mattermost instance from the terminal.
The project's documentation can be accessed within the the Mattermost documentation.