mirror of
https://github.com/mattermost/mattermost.git
synced 2026-05-24 03:05:38 -04: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>
|
||
|---|---|---|
| .. | ||
| testdata | ||
| assertions.go | ||
| cluster.go | ||
| doc.go | ||
| hashers_dev.go | ||
| hashers_production.go | ||
| helper.go | ||
| resources.go | ||
| resources_test.go | ||
| store.go | ||