From bfc2637d215af9291f42c2e59caa7f7f11c4fea3 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Thu, 20 Aug 2026 17:16:19 -0300 Subject: [PATCH 1/4] Precompute multibyte mention keywords once per post (#38038) --- .../channels/app/mention_parser_standard.go | 36 ++++-- .../app/mention_parser_standard_test.go | 122 ++++++++++++++++++ 2 files changed, 145 insertions(+), 13 deletions(-) diff --git a/server/channels/app/mention_parser_standard.go b/server/channels/app/mention_parser_standard.go index 6a96cf1c3acf..16d2fca4e93f 100644 --- a/server/channels/app/mention_parser_standard.go +++ b/server/channels/app/mention_parser_standard.go @@ -15,12 +15,25 @@ var _ MentionParser = &StandardMentionParser{} type StandardMentionParser struct { keywords MentionKeywords + // multibyteKeywords holds the subset of keywords containing a multibyte character. It's computed + // once here because isKeywordMultibyte needs it for every word of every post, and recomputing it + // there makes parsing cost the product of the number of keywords and the number of words. + multibyteKeywords []string + results *MentionResults } func makeStandardMentionParser(keywords MentionKeywords) *StandardMentionParser { + var multibyteKeywords []string + for keyword := range keywords { + if len(keyword) != utf8.RuneCountInString(keyword) { + multibyteKeywords = append(multibyteKeywords, keyword) + } + } + return &StandardMentionParser{ - keywords: keywords, + keywords: keywords, + multibyteKeywords: multibyteKeywords, results: &MentionResults{}, } @@ -86,7 +99,7 @@ func (p *StandardMentionParser) ProcessText(text string) { } } - if ids, match := isKeywordMultibyte(p.keywords, word); match { + if ids, match := p.isKeywordMultibyte(word); match { p.addMentions(ids, KeywordMention) } } @@ -139,22 +152,19 @@ func (p *StandardMentionParser) addMentions(ids []MentionableID, mentionType Men } // isKeywordMultibyte checks if a word containing a multibyte character contains a multibyte keyword -func isKeywordMultibyte(keywords MentionKeywords, word string) ([]MentionableID, bool) { +func (p *StandardMentionParser) isKeywordMultibyte(word string) ([]MentionableID, bool) { ids := []MentionableID{} match := false - var multibyteKeywords []string - for keyword := range keywords { - if len(keyword) != utf8.RuneCountInString(keyword) { - multibyteKeywords = append(multibyteKeywords, keyword) - } + + if len(p.multibyteKeywords) == 0 || len(word) == utf8.RuneCountInString(word) { + return ids, match } - if len(word) != utf8.RuneCountInString(word) { - for _, key := range multibyteKeywords { - if strings.Contains(word, key) { - ids, match = keywords[key] - } + for _, key := range p.multibyteKeywords { + if strings.Contains(word, key) { + ids, match = p.keywords[key] } } + return ids, match } diff --git a/server/channels/app/mention_parser_standard_test.go b/server/channels/app/mention_parser_standard_test.go index 5522ca683290..10e35df390e0 100644 --- a/server/channels/app/mention_parser_standard_test.go +++ b/server/channels/app/mention_parser_standard_test.go @@ -4,15 +4,60 @@ package app import ( + "fmt" + "maps" + "strings" "testing" "github.com/mattermost/mattermost/server/public/model" "github.com/stretchr/testify/assert" ) +// withFillerKeywords returns the given keywords along with count additional single-byte keywords +// that are not expected to match anything. +func withFillerKeywords(keywords map[string][]string, count int) map[string][]string { + result := make(map[string][]string, len(keywords)+count) + maps.Copy(result, keywords) + + filler := model.NewId() + for i := range count { + result[fmt.Sprintf("keyword%d", i)] = []string{filler} + } + + return result +} + +func TestMakeStandardMentionParser(t *testing.T) { + mainHelper.Parallel(t) + + t.Run("should precompute only the multibyte keywords", func(t *testing.T) { + id := model.NewId() + p := makeStandardMentionParser(mapsToMentionKeywords(map[string][]string{ + "apple": {id}, + "banana": {id}, + "番茄": {id}, + "世界": {id}, + "café": {id}, + }, nil)) + + assert.ElementsMatch(t, []string{"番茄", "世界", "café"}, p.multibyteKeywords) + }) + + t.Run("should precompute nothing when no keyword is multibyte", func(t *testing.T) { + id := model.NewId() + p := makeStandardMentionParser(mapsToMentionKeywords(map[string][]string{ + "apple": {id}, + "banana": {id}, + }, nil)) + + assert.Empty(t, p.multibyteKeywords) + }) +} + func TestIsKeywordMultibyte(t *testing.T) { mainHelper.Parallel(t) id1 := model.NewId() + id2 := model.NewId() for name, tc := range map[string]struct { Message string @@ -101,6 +146,38 @@ func TestIsKeywordMultibyte(t *testing.T) { Mentions: nil, }, }, + "MultibyteCharacterAlongsideManySingleByteKeywords": { + Message: "我爱吃番茄炒饭", + Keywords: withFillerKeywords(map[string][]string{"番茄": {id1}}, 500), + Expected: &MentionResults{ + Mentions: map[string]MentionType{ + id1: KeywordMention, + }, + }, + }, + "MultibyteCharacterWithOnlySingleByteKeywords": { + Message: "我爱吃番茄炒饭", + Keywords: map[string][]string{"tomato": {id1}}, + Expected: &MentionResults{ + Mentions: nil, + }, + }, + "SingleByteWordWithOnlyMultibyteKeywords": { + Message: "the quick brown fox", + Keywords: map[string][]string{"番茄": {id1}}, + Expected: &MentionResults{ + Mentions: nil, + }, + }, + "MultipleMultibyteKeywordsWhereOnlyOneMatches": { + Message: "我爱吃番茄炒饭", + Keywords: map[string][]string{"番茄": {id1}, "世界": {id2}}, + Expected: &MentionResults{ + Mentions: map[string]MentionType{ + id1: KeywordMention, + }, + }, + }, } { t.Run(name, func(t *testing.T) { post := &model.Post{ @@ -435,3 +512,48 @@ func TestProcessText(t *testing.T) { }) } } + +// makeBenchmarkKeywords returns count single-byte keywords, optionally alongside one multibyte +// keyword, all belonging to a single user. +func makeBenchmarkKeywords(count int, multibyteKeyword string) MentionKeywords { + keywords := make(MentionKeywords, count+1) + + id := mentionableUserID(model.NewId()) + for i := range count { + keywords[fmt.Sprintf("keyword%d", i)] = []MentionableID{id} + } + + if multibyteKeyword != "" { + keywords[multibyteKeyword] = []MentionableID{id} + } + + return keywords +} + +func BenchmarkGetExplicitMentions(b *testing.B) { + // Roughly 1800 words, matching the scale of a long message in a busy channel. + asciiMessage := strings.Repeat("the quick brown fox jumps over the lazy dog ", 200) + multibyteMessage := strings.Repeat("こんにちは、世界 the quick brown fox ", 200) + + for _, tc := range []struct { + name string + message string + multibyteKeyword string + }{ + {name: "ascii post, no multibyte keywords", message: asciiMessage}, + {name: "ascii post, one multibyte keyword", message: asciiMessage, multibyteKeyword: "世界"}, + {name: "multibyte post, one multibyte keyword", message: multibyteMessage, multibyteKeyword: "世界"}, + } { + for _, numKeywords := range []int{10, 1000, 10000, 55000} { + b.Run(fmt.Sprintf("%s/keywords=%d", tc.name, numKeywords), func(b *testing.B) { + keywords := makeBenchmarkKeywords(numKeywords, tc.multibyteKeyword) + post := &model.Post{Message: tc.message} + + b.ReportAllocs() + for b.Loop() { + getExplicitMentions(post, keywords, true) + } + }) + } + } +} From ddee8289bc70fb13b7a39db7e6144f0fd34968a2 Mon Sep 17 00:00:00 2001 From: Julien Tant <785518+JulienTant@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:40:35 -0700 Subject: [PATCH 2/4] [MM-69866] Add Applies-to resource picker (Users, Channels, Posts) to New attribute (#38002) * [MM-69866] Add Applies-to resource picker (Users, Channels, Posts) to New attribute Adds an "Applies to" Card to the Global Attributes "New attribute" page, letting a sysadmin pick which resources (Users, Channels, Posts) an attribute applies to before saving. Each selected resource becomes its own linked PropertyField, created serially after the template so a partial failure can be attributed to a specific resource and rolled back deterministically (linked fields deleted before the template, per the server's deletion-order protection). A `user`-scoped linked field shares its namespace with Custom Profile Attributes, so name conflicts and the shared 20-field cap are newly reachable failure modes -- both get distinct, actionable banners instead of a generic failure message. Client-only change; no server, migration, or Client4 changes required. Co-Authored-By: Claude Sonnet 5 * Split the Applies-to row into one component per resource type Replaces the single resourceType-parameterized AttributeAppliesToItem with three dedicated components (User/Channel/Post), each hardcoding its own icon, label, and testids instead of looking them up by a prop. All three share one prop signature (AttributeAppliesToItemProps, exported from attribute_applies_to_constants.tsx) so the parent's Record> lookup map fails to compile if any of the three drift from it. Co-Authored-By: Claude Sonnet 5 * Fix Prettier formatting in the global_attributes E2E files npm run check for e2e-tests/playwright runs lint && prettier && tsc, and CI's prettier --check step was failing on both files -- they'd only been checked against webapp's ESLint config locally, not this package's Prettier config. Co-Authored-By: Claude Sonnet 5 * Move the Applies-to row's Remove action into the expanded state The collapsed row no longer has any remove affordance -- the only way to remove a resource is to expand it first, then click "Remove resource" in the header (still a sibling of the toggle, not nested inside its clickable area). Styled per the design prototype: a plain text button, transparent by default, --error-text colored, with a rgba(--error-text-color-rgb, 0.08) hover tint -- the same pattern already used by OrphanedFieldDeleteButton (system_properties) rather than a new one-off style. Co-Authored-By: Claude Sonnet 5 * Add 12px right margin to the Applies-to row's Remove button Matches the design prototype's spacing between the button and the row's right edge. Co-Authored-By: Claude Sonnet 5 * Rename applies-to constants file to .ts The file has no JSX, so the .tsx extension was misleading. * Polish the Applies-to card empty state and expanded rows Match the design: updated copy and type, primary/tertiary add buttons, header-only open tint, and a 176px form row in the expanded body. * Drop bottom padding on the last Applies-to body row It stacked with the container padding and looked doubled. * Size Unique name label, value, and Edit to 12px The caption and Edit link were 14px against a 12px label; shrink the edit input to match. * Treat Unique name click-away as Done Opening Edit then clicking away left the input open and froze auto-derivation. Blur now uses the same commit path as Done, matching the channel URL field. * Lock Type to Text while an external source is linked LDAP and SAML only sync on text fields; disabling the type menu until the last chip is removed keeps that invariant in the UI. * Show linked sources on the Options line as Synced with chips Once a source is selected the chips replace the Text help copy; the divider stays until then so the unlinked state still separates Options from the add-source trigger. * Order flex container properties to satisfy stylelint. * Show CPA banners only for Users and copy leftover-template rollback. Channels and Posts name conflicts used User Attribute wording, and a leftover template after a linked-field rollback looked like a clean save. E2E cleanup now rediscovers template and linked fields instead of relying on the success-path list. * Use Compass Button for Applies-to row chrome. Override quaternary styling so the accordion header stays body-text chrome, and keep Remove as tertiary destructive. * Await hanging creates before unmount-save negative asserts. waitFor(() => Promise.resolve()) returns on the first check and can pass before finalizeSave, so the mount guard was not actually load-bearing. * Drop unused PropertyField and ResourceObjectType imports from the Playwright spec. --------- Co-authored-by: Claude Sonnet 5 Co-authored-by: Mattermost Build --- .../global_attributes.spec.ts | 378 ++++++++++++++++- .../global_attributes_helpers.ts | 93 ++++- .../attribute_applies_to.scss | 45 ++ .../attribute_applies_to.test.tsx | 97 +++++ .../attribute_applies_to.tsx | 174 ++++++++ ...attribute_applies_to_channel_item.test.tsx | 80 ++++ .../attribute_applies_to_channel_item.tsx | 100 +++++ .../attribute_applies_to_constants.ts | 43 ++ .../attribute_applies_to_item.scss | 95 +++++ .../attribute_applies_to_post_item.test.tsx | 80 ++++ .../attribute_applies_to_post_item.tsx | 100 +++++ .../attribute_applies_to_user_item.test.tsx | 80 ++++ .../attribute_applies_to_user_item.tsx | 100 +++++ .../attribute_details/attribute_details.scss | 20 +- .../attribute_details.test.tsx | 392 +++++++++++++++++- .../attribute_details/attribute_details.tsx | 329 +++++++++++++-- .../attribute_external_source.scss | 27 +- .../attribute_external_source.test.tsx | 7 +- .../attribute_external_source.tsx | 41 +- .../global_attributes/utils.test.ts | 79 +++- .../admin_console/global_attributes/utils.ts | 37 +- webapp/channels/src/i18n/en.json | 20 + 22 files changed, 2309 insertions(+), 108 deletions(-) create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.scss create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.test.tsx create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.tsx create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_channel_item.test.tsx create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_channel_item.tsx create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_constants.ts create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_item.scss create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_post_item.test.tsx create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_post_item.tsx create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_user_item.test.tsx create mode 100644 webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_user_item.tsx diff --git a/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes.spec.ts b/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes.spec.ts index ee857932a289..c3a7e959c35b 100644 --- a/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes.spec.ts +++ b/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes.spec.ts @@ -22,8 +22,10 @@ import { GLOBAL_ATTRIBUTES_ADMIN_PATH, createGlobalAttributeField, createLinkedDependentField, + deleteAppliesToAttributeAndLinkedFieldsIfExists, deleteGlobalAttributeFieldIfExists, deleteLinkedDependentField, + fetchLinkedFieldsForTemplate, requireGlobalAttributesEnabled, setGlobalAttributesFeatureFlag, } from './global_attributes_helpers'; @@ -517,11 +519,11 @@ test.describe('System Console - Global Attributes', {tag: '@system_console'}, () }); /** - * @objective Ensure "Done" and Enter both refuse to commit an invalid manual Unique name, - * so a reserved word can never be left sitting in the field, and that correcting the value - * unblocks the commit and lets the attribute save for real. + * @objective Ensure "Done", Enter, and blur all refuse to commit an invalid manual Unique + * name, so a reserved word can never be left sitting in the field, and that correcting the + * value unblocks the commit and lets the attribute save for real. */ - test('blocks Done and Enter while the manual Unique name is a reserved word, then commits once corrected', async ({ + test('blocks Done, Enter, and blur while the manual Unique name is a reserved word, then commits once corrected', async ({ pw, }) => { const {adminUser, adminClient} = await requireGlobalAttributesEnabled(pw); @@ -592,7 +594,16 @@ test.describe('System Console - Global Attributes', {tag: '@system_console'}, () 'reserved word', ); + // # Click away onto the Display name field (blur), same commit path as Done/Enter + await systemConsolePage.page.getByTestId('attributeDisplayNameInput').click(); + + // * Also rejected -- the editor stays open rather than committing on click-away + await expect(nameInput).toBeVisible(); + await expect(nameInput).toHaveValue('for'); + await expect(doneLink).toHaveText('Done'); + // # Correct the value by typing the rest of the identifier + await nameInput.click(); await nameInput.press('End'); await nameInput.pressSequentially(`_${timestamp}`); @@ -678,6 +689,62 @@ test.describe('System Console - Global Attributes', {tag: '@system_console'}, () await expect(systemConsolePage.page.getByTestId('saveSetting')).toBeEnabled(); }); + /** + * @objective Ensure clicking away from the Unique name input is the same as Done: an + * unchanged seed keeps auto-derivation live, and an actual edit pins the Name so further + * Display name changes no longer rewrite it. Mirrors the channel URL field in create/settings. + */ + test('treats clicking away from Unique name as Done: no-op keeps derivation, an edit pins', async ({pw}) => { + const {adminUser} = await requireGlobalAttributesEnabled(pw); + + const timestamp = Date.now(); + // Short prefix: see the 40-char Unique name cap noted in the reserved-word test above + const displayName = `Playwright Blur ${timestamp}`; + const autoDerivedName = `playwright_blur_${timestamp}`; + const extendedDisplayName = `${displayName} Two`; + const extendedAutoDerivedName = `${autoDerivedName}_two`; + const pinnedName = `custom_blur_${timestamp}`; + + // # Log in and open the create page + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await systemConsolePage.page.goto(GLOBAL_ATTRIBUTES_ADMIN_PATH); + await systemConsolePage.page.getByTestId('newAttributeButton').click(); + await expect(systemConsolePage.page).toHaveURL(/attribute_details/); + + const displayNameInput = systemConsolePage.page.getByTestId('attributeDisplayNameInput'); + await displayNameInput.fill(displayName); + await expect(systemConsolePage.page.getByTestId('attributeUniqueNameValue')).toHaveText(autoDerivedName); + + // # Open the editor and click away without changing the seeded value + await systemConsolePage.page.getByTestId('attributeNameEditLink').click(); + await expect(systemConsolePage.page.getByTestId('attributeNameInput')).toBeFocused(); + await displayNameInput.click(); + + // * Exited edit mode, still showing the auto-derived slug + await expect(systemConsolePage.page.getByTestId('attributeNameInput')).toHaveCount(0); + await expect(systemConsolePage.page.getByTestId('attributeNameEditLink')).toHaveText('Edit'); + await expect(systemConsolePage.page.getByTestId('attributeUniqueNameValue')).toHaveText(autoDerivedName); + + // * Auto-derivation is still live + await displayNameInput.fill(extendedDisplayName); + await expect(systemConsolePage.page.getByTestId('attributeUniqueNameValue')).toHaveText( + extendedAutoDerivedName, + ); + + // # Open the editor, type a different Name, and click away + await systemConsolePage.page.getByTestId('attributeNameEditLink').click(); + await systemConsolePage.page.getByTestId('attributeNameInput').fill(pinnedName); + await displayNameInput.click(); + + // * Committed -- the editor closed and the typed Name is shown + await expect(systemConsolePage.page.getByTestId('attributeNameInput')).toHaveCount(0); + await expect(systemConsolePage.page.getByTestId('attributeUniqueNameValue')).toHaveText(pinnedName); + + // * Manual override stays in effect -- further Display name edits do not overwrite it + await displayNameInput.fill(`${extendedDisplayName} Three`); + await expect(systemConsolePage.page.getByTestId('attributeUniqueNameValue')).toHaveText(pinnedName); + }); + /** * @objective Ensure a Select attribute can be created end-to-end with a real options * editor: type switch, add two options via Enter, Save is blocked until an option exists, @@ -849,9 +916,12 @@ test.describe('System Console - Global Attributes', {tag: '@system_console'}, () await systemConsolePage.page.getByRole('textbox').fill('employeeID'); await systemConsolePage.page.getByRole('button', {name: 'Save'}).click(); - // * A chip for AD/LDAP now appears, and Type shows Text + // * A chip for AD/LDAP now appears on the Options line, prefixed by Synced with, and Type shows Text await expect(systemConsolePage.page.getByTestId('attributeExternalSourceChip-ldap')).toBeVisible(); await expect(systemConsolePage.page.getByTestId('attributeTypeMenuButton')).toContainText('Text'); + await expect(systemConsolePage.page.getByTestId('attributeExternalSourceSynced')).toContainText( + 'Synced with', + ); await systemConsolePage.page.getByTestId('saveSetting').click(); @@ -982,10 +1052,9 @@ test.describe('System Console - Global Attributes', {tag: '@system_console'}, () /** * @objective Ensure the picker warns before converting a non-Text field to Text, and that - * manually switching Type away from Text afterward clears the link and announces it via - * the status region (not just a silently-removed chip). + * once a source is linked the Type control is locked to Text until the last chip is removed. */ - test('warns before converting a non-Text field, and clears + announces the link when Type is switched away from Text', async ({ + test('warns before converting a non-Text field, and locks Type to Text while a source is linked', async ({ pw, }) => { const {adminUser} = await requireGlobalAttributesEnabled(pw); @@ -1008,19 +1077,296 @@ test.describe('System Console - Global Attributes', {tag: '@system_console'}, () await systemConsolePage.page.getByPlaceholder('department').fill('employeeID'); await systemConsolePage.page.getByRole('button', {name: 'Save'}).click(); - // * Type switched to Text, and a chip appeared - await expect(systemConsolePage.page.getByTestId('attributeTypeMenuButton')).toContainText('Text'); + // * Type switched to Text, a chip appeared, and Type is locked + const typeButton = systemConsolePage.page.getByTestId('attributeTypeMenuButton'); + await expect(typeButton).toContainText('Text'); await expect(systemConsolePage.page.getByTestId('attributeExternalSourceChip-ldap')).toBeVisible(); + await expect(typeButton).toBeDisabled(); - // # Switch Type away from Text again - await systemConsolePage.page.getByTestId('attributeTypeMenuButton').click(); + // # Remove the chip + await systemConsolePage.page.getByTestId('attributeExternalSourceChip-ldap-remove').click(); + + // * Type is editable again + await expect(typeButton).toBeEnabled(); + await typeButton.click(); await systemConsolePage.page.getByRole('menuitemradio', {name: 'Select', exact: true}).click(); + await expect(typeButton).toContainText('Select'); + }); + }); - // * The link is cleared, and the removal is announced via the status region - await expect(systemConsolePage.page.getByTestId('attributeExternalSourceChip-ldap')).not.toBeVisible(); - await expect(systemConsolePage.page.getByTestId('attributeExternalSourceStatus')).toHaveText( - 'External source link removed', + test.describe('applies to', () => { + /** + * @objective Ensure a brand-new attribute's Applies-to card renders its empty state + * correctly, with no resources and both "Add resource" triggers available. + */ + test('shows the empty state with both Add-resource triggers, and no rows', async ({pw}) => { + const {adminUser} = await requireGlobalAttributesEnabled(pw); + + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await systemConsolePage.page.goto(GLOBAL_ATTRIBUTES_ADMIN_PATH); + await systemConsolePage.page.getByTestId('newAttributeButton').click(); + + // * Empty state renders with its heading/helper text + await expect(systemConsolePage.page.getByTestId('attributeAppliesToEmptyState')).toBeVisible(); + + // * No resource rows exist yet + for (const type of ['user', 'channel', 'post']) { + await expect(systemConsolePage.page.getByTestId(`attributeAppliesToRow-${type}`)).not.toBeVisible(); + } + + // * Both triggers are available + await expect(systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader')).toBeVisible(); + await expect(systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonInline')).toBeVisible(); + }); + + /** + * @objective Ensure the picker offers exactly the not-yet-selected types, adding one + * removes it from the picker and renders its row, and once all three are added both + * triggers disappear entirely. + */ + test('offers only unselected types, renders a row per addition, and hides both triggers once all three are added', async ({ + pw, + }) => { + const {adminUser} = await requireGlobalAttributesEnabled(pw); + + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await systemConsolePage.page.goto(GLOBAL_ATTRIBUTES_ADMIN_PATH); + await systemConsolePage.page.getByTestId('newAttributeButton').click(); + + // # Open the picker; all three types are offered, in order + await systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader').click(); + const menuItems = systemConsolePage.page.getByRole('menuitem'); + await expect(menuItems).toHaveText(['Users', 'Channels', 'Posts']); + + // # Pick Users + await systemConsolePage.page.getByRole('menuitem', {name: 'Users'}).click(); + + // * Users row renders, empty state is gone + await expect(systemConsolePage.page.getByTestId('attributeAppliesToRow-user')).toBeVisible(); + await expect(systemConsolePage.page.getByTestId('attributeAppliesToEmptyState')).not.toBeVisible(); + + // # Reopen the picker -- only Channels and Posts remain + await systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader').click(); + await expect(systemConsolePage.page.getByRole('menuitem')).toHaveText(['Channels', 'Posts']); + + // # Add Channels, then Posts + await systemConsolePage.page.getByRole('menuitem', {name: 'Channels'}).click(); + await systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader').click(); + await systemConsolePage.page.getByRole('menuitem', {name: 'Posts'}).click(); + + // * All three rows render, in insertion order + await expect(systemConsolePage.page.getByTestId('attributeAppliesToRow-user')).toBeVisible(); + await expect(systemConsolePage.page.getByTestId('attributeAppliesToRow-channel')).toBeVisible(); + await expect(systemConsolePage.page.getByTestId('attributeAppliesToRow-post')).toBeVisible(); + + // * Both triggers are gone now that all three types are selected + await expect( + systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader'), + ).not.toBeVisible(); + await expect( + systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonInline'), + ).not.toBeVisible(); + }); + + /** + * @objective Ensure removing a not-yet-saved resource is an immediate local change -- no + * confirmation modal, no network call -- and the removed type becomes available in the + * picker again. + */ + test('removes a pending resource locally with no confirm modal and no delete request', async ({pw}) => { + const {adminUser} = await requireGlobalAttributesEnabled(pw); + + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await systemConsolePage.page.goto(GLOBAL_ATTRIBUTES_ADMIN_PATH); + await systemConsolePage.page.getByTestId('newAttributeButton').click(); + + // Regression guard: no property-field delete request fires for a pre-save removal. + let deleteRequestFired = false; + await systemConsolePage.page.route( + '**/api/v4/properties/groups/access_control/*/fields/*', + async (route) => { + if (route.request().method() === 'DELETE') { + deleteRequestFired = true; + } + await route.continue(); + }, ); + + // # Add Channels + await systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader').click(); + await systemConsolePage.page.getByRole('menuitem', {name: 'Channels'}).click(); + await expect(systemConsolePage.page.getByTestId('attributeAppliesToRow-channel')).toBeVisible(); + + // # Remove is only reachable once the row is expanded + await expect(systemConsolePage.page.getByTestId('attributeAppliesToRow-channel-remove')).not.toBeVisible(); + await systemConsolePage.page.getByTestId('attributeAppliesToRow-channel-toggle').click(); + await systemConsolePage.page.getByTestId('attributeAppliesToRow-channel-remove').click(); + + // * The row disappears immediately, no modal/dialog ever rendered + await expect(systemConsolePage.page.getByTestId('attributeAppliesToRow-channel')).not.toBeVisible(); + await expect(systemConsolePage.page.getByRole('dialog')).not.toBeVisible(); + + // # Reopen the picker -- Channels is offered again + await systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader').click(); + await expect(systemConsolePage.page.getByRole('menuitem', {name: 'Channels'})).toBeVisible(); + + expect(deleteRequestFired).toBe(false); + }); + + /** + * @objective Ensure Save creates the template, then one linked field per selected + * resource, each correctly pointing back at the template -- verified end-to-end against + * the real server via the admin API, since the listing table's Applies-to column is a + * hardcoded placeholder (see Out of Scope). + */ + test('saves the template plus one linked field per selected resource', async ({pw}) => { + const {adminUser, adminClient} = await requireGlobalAttributesEnabled(pw); + + const timestamp = Date.now(); + const displayName = `Playwright Applies To ${timestamp}`; + const expectedName = `playwright_applies_to_${timestamp}`; + + try { + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await systemConsolePage.page.goto(GLOBAL_ATTRIBUTES_ADMIN_PATH); + await systemConsolePage.page.getByTestId('newAttributeButton').click(); + + // # Fill Display name, add Users and Channels + await systemConsolePage.page.getByTestId('attributeDisplayNameInput').fill(displayName); + await systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader').click(); + await systemConsolePage.page.getByRole('menuitem', {name: 'Users'}).click(); + await systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader').click(); + await systemConsolePage.page.getByRole('menuitem', {name: 'Channels'}).click(); + + // # Save + await systemConsolePage.page.getByTestId('saveSetting').click(); + + // * Redirected back to the Manage Attributes list + await expect(systemConsolePage.page).toHaveURL(new RegExp(`${GLOBAL_ATTRIBUTES_ADMIN_PATH}$`)); + + // * Exactly two linked fields exist, pointing back at the template + const templateFields = await adminClient.getPropertyFields( + 'access_control', + 'template', + 'system', + undefined, + {perPage: 200}, + ); + const templateField = templateFields.find((f) => f.name === expectedName && f.delete_at === 0); + expect(templateField).toBeDefined(); + + const linkedFields = await fetchLinkedFieldsForTemplate(adminClient, templateField!.id); + expect(linkedFields).toHaveLength(2); + + const userField = linkedFields.find((f) => f.object_type === 'user'); + const channelField = linkedFields.find((f) => f.object_type === 'channel'); + expect(userField).toBeDefined(); + expect(channelField).toBeDefined(); + for (const field of [userField!, channelField!]) { + expect(field.target_type).toBe('system'); + expect(field.linked_field_id).toBe(templateField!.id); + expect(field.attrs?.display_name).toBe(displayName); + } + } finally { + await deleteAppliesToAttributeAndLinkedFieldsIfExists(adminClient, expectedName); + } + }); + + /** + * @objective Ensure a mid-save failure rolls back everything created in that attempt and + * leaves Save retryable, rather than leaving an orphaned template or linked field behind. + */ + test('rolls back a partial save and lets the admin retry successfully', async ({pw}) => { + const {adminUser, adminClient} = await requireGlobalAttributesEnabled(pw); + + const timestamp = Date.now(); + const displayName = `Playwright Applies To Retry ${timestamp}`; + const expectedName = `playwright_applies_to_retry_${timestamp}`; + + try { + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await systemConsolePage.page.goto(GLOBAL_ATTRIBUTES_ADMIN_PATH); + await systemConsolePage.page.getByTestId('newAttributeButton').click(); + + // # Fill Display name, add Users, Channels, and Posts + await systemConsolePage.page.getByTestId('attributeDisplayNameInput').fill(displayName); + for (const label of ['Users', 'Channels', 'Posts']) { + await systemConsolePage.page.getByTestId('attributeAppliesToAddResourceButtonHeader').click(); + await systemConsolePage.page.getByRole('menuitem', {name: label}).click(); + } + + // # Force the "post" linked-field creation request to fail; let user/channel/template through + await systemConsolePage.page.route( + '**/api/v4/properties/groups/access_control/post/fields', + async (route) => { + if (route.request().method() === 'POST') { + await route.fulfill({ + status: 500, + contentType: 'application/json', + body: JSON.stringify({message: 'forced failure'}), + }); + } else { + await route.continue(); + } + }, + ); + + // # Click Save + await systemConsolePage.page.getByTestId('saveSetting').click(); + + // * The banner names the failed resource, and Save is re-clickable + await expect(systemConsolePage.page.getByTestId('attributeSaveError')).toContainText('Posts'); + await expect(systemConsolePage.page.getByTestId('saveSetting')).not.toBeDisabled(); + + // * Nothing survived the rollback -- no template, no user/channel linked fields + const templateFieldsAfterFailure = await adminClient.getPropertyFields( + 'access_control', + 'template', + 'system', + undefined, + {perPage: 200}, + ); + expect( + templateFieldsAfterFailure.find((f) => f.name === expectedName && f.delete_at === 0), + ).toBeUndefined(); + + const userFields = await adminClient.getPropertyFields('access_control', 'user', 'system', undefined, { + perPage: 200, + }); + const channelFields = await adminClient.getPropertyFields( + 'access_control', + 'channel', + 'system', + undefined, + {perPage: 200}, + ); + expect(userFields.find((f) => f.name === expectedName && f.delete_at === 0)).toBeUndefined(); + expect(channelFields.find((f) => f.name === expectedName && f.delete_at === 0)).toBeUndefined(); + + // # Remove the interception and retry + await systemConsolePage.page.unroute('**/api/v4/properties/groups/access_control/post/fields'); + await systemConsolePage.page.getByTestId('saveSetting').click(); + + // * This time it succeeds + await expect(systemConsolePage.page).toHaveURL(new RegExp(`${GLOBAL_ATTRIBUTES_ADMIN_PATH}$`)); + + const templateFieldsAfterRetry = await adminClient.getPropertyFields( + 'access_control', + 'template', + 'system', + undefined, + {perPage: 200}, + ); + const templateField = templateFieldsAfterRetry.find( + (f) => f.name === expectedName && f.delete_at === 0, + ); + expect(templateField).toBeDefined(); + + const linkedFields = await fetchLinkedFieldsForTemplate(adminClient, templateField!.id); + expect(linkedFields).toHaveLength(3); + } finally { + await deleteAppliesToAttributeAndLinkedFieldsIfExists(adminClient, expectedName); + } }); }); diff --git a/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes_helpers.ts b/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes_helpers.ts index 168580549a1a..7cd689fd384e 100644 --- a/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes_helpers.ts +++ b/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes_helpers.ts @@ -2,6 +2,7 @@ // See LICENSE.txt for license information. import type {Client4} from '@mattermost/client'; +import type {PropertyField} from '@mattermost/types/properties'; import {getAdminClient, licenseTier, test} from '@mattermost/playwright-lib'; import type {PlaywrightExtended} from '@mattermost/playwright-lib'; @@ -14,11 +15,13 @@ const PROPERTY_GROUP = 'access_control'; const OBJECT_TYPE = 'template'; const TARGET_TYPE = 'system'; -// Object type used to seed a field that *links to* a template field. It has to be -// anything but 'template': PropertyField.IsValid rejects a template field carrying a -// linked_field_id ("template fields cannot have a linked field"). 'user' matches the -// shape the store's own CountLinkedFields coverage uses. -const LINKED_OBJECT_TYPE = 'user'; +// The three resource object types an Applies-to linked field can use. Also the +// only object types (besides 'template') PropertyField.IsValid allows a linked +// field to carry -- a template field itself is rejected for having a +// linked_field_id ("template fields cannot have a linked field"). +// Canonical values: webapp/channels/.../attribute_details/attribute_applies_to_constants.ts +export type ResourceObjectType = 'user' | 'channel' | 'post'; +const ALL_RESOURCE_OBJECT_TYPES: ResourceObjectType[] = ['user', 'channel', 'post']; // Server clamps per_page to this max (see web.PerPageMaximum in server/channels/web/params.go). // Directory-mode search with no cursor sorts CreateAt ASC, so the default 60-item page only @@ -83,6 +86,28 @@ export async function deleteGlobalAttributeFieldIfExists(adminClient: Client4, n } } +/** + * Best-effort cleanup for an Applies-to save: delete linked fields first + * (looked up from the live template, not from the test's success-path locals) + * so the template delete is not 409'd by leftover dependents. + */ +export async function deleteAppliesToAttributeAndLinkedFieldsIfExists(adminClient: Client4, name: string) { + try { + const templates = await adminClient.getPropertyFields(PROPERTY_GROUP, OBJECT_TYPE, TARGET_TYPE, undefined, { + perPage: MAX_PROPERTY_FIELDS_PER_PAGE, + }); + for (const template of templates.filter((field) => field.name === name && field.delete_at === 0)) { + const linked = await fetchLinkedFieldsForTemplate(adminClient, template.id); + for (const field of linked) { + await deleteLinkedDependentField(adminClient, field.id, field.object_type as ResourceObjectType); + } + } + } catch { + // Listing may fail if the flag is off; still try the template delete below. + } + await deleteGlobalAttributeFieldIfExists(adminClient, name); +} + /** * Creates an access_control/template property field (the same group/object type/target * this ticket's table lists) for E2E seeding. Ensures a clean slate first so reruns don't @@ -104,35 +129,69 @@ export async function createGlobalAttributeField( } /** - * Creates a field that links to `sourceFieldId`, which makes the server refuse to delete - * that source field: deletePropertyField counts live linked dependents and returns 409 - * `has_linked_dependents` when any exist (server/channels/app/properties/property_field.go). - * This is the only way to exercise the listing's 409 branch against a real server response. + * Creates a field that links to `sourceFieldId` (e.g. the template). Two uses: + * seeding an already-saved Applies-to resource for a given `objectType` + * (defaulting to 'user', the one pre-existing call site's shape), and making + * the server refuse to delete the source field -- deletePropertyField counts + * live linked dependents and returns 409 `has_linked_dependents` when any + * exist (server/channels/app/properties/property_field.go), which this is + * the only way to exercise against a real server response. The create flow + * for a fresh Applies-to resource itself is exercised through the UI, not + * this helper. */ export async function createLinkedDependentField( adminClient: Client4, name: string, sourceFieldId: string, type: string, + objectType: ResourceObjectType = 'user', ) { - return adminClient.createPropertyField(PROPERTY_GROUP, LINKED_OBJECT_TYPE, { + return adminClient.createPropertyField(PROPERTY_GROUP, objectType, { name, type, target_type: TARGET_TYPE, target_id: '', linked_field_id: sourceFieldId, - } as unknown as Parameters[2]); + } as Parameters[2]); } /** - * Deletes a linked dependent field by id, ignoring failures (it may already be gone). - * Must run BEFORE deleting the field it points at — the source delete stays blocked - * with a 409 for as long as a live dependent exists. + * Deletes a linked property field, ignoring failures -- mirrors deleteGlobalAttributeFieldIfExists' + * best-effort cleanup style, since a test's own save/rollback assertions may have already deleted it. + * Must run BEFORE deleting the field it points at -- the source delete stays blocked with a 409 + * for as long as a live dependent exists. */ -export async function deleteLinkedDependentField(adminClient: Client4, fieldId: string) { +export async function deleteLinkedDependentField( + adminClient: Client4, + fieldId: string, + objectType: ResourceObjectType = 'user', +) { try { - await adminClient.deletePropertyField(PROPERTY_GROUP, LINKED_OBJECT_TYPE, fieldId); + await adminClient.deletePropertyField(PROPERTY_GROUP, objectType, fieldId); } catch { - // Already deleted, or routes unavailable; ignore. + // May already be gone (e.g. a prior rollback already deleted it); ignore. } } + +/** + * Finds every linked field (across all three resource object types) pointing at `templateFieldId`. + * Queries user/channel/post separately -- there is no single "all object types" listing endpoint -- + * and requests the max page size per call, since the `user` object type's result page is shared + * with every Custom Profile Attributes field on the server (see deleteGlobalAttributeFieldIfExists' + * own MAX_PROPERTY_FIELDS_PER_PAGE comment) and a freshly-created linked field is exactly the kind + * of newest-row a default ascending-CreateAt page can drop. + */ +export async function fetchLinkedFieldsForTemplate( + adminClient: Client4, + templateFieldId: string, +): Promise { + const results = await Promise.all( + ALL_RESOURCE_OBJECT_TYPES.map((objectType) => + adminClient.getPropertyFields(PROPERTY_GROUP, objectType, TARGET_TYPE, undefined, { + perPage: MAX_PROPERTY_FIELDS_PER_PAGE, + }), + ), + ); + + return results.flat().filter((field) => field.linked_field_id === templateFieldId && field.delete_at === 0); +} diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.scss b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.scss new file mode 100644 index 000000000000..737b21079216 --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.scss @@ -0,0 +1,45 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +.AttributeAppliesTo { + margin-top: 20px; +} + +.AttributeAppliesTo__trigger { + align-items: center; + gap: 6px; +} + +.AttributeAppliesTo__emptyState { + display: flex; + flex-direction: column; + align-items: center; + padding: 32px 0; + gap: 4px; + text-align: center; +} + +.AttributeAppliesTo__emptyStateHeading { + margin: 0; + font-family: Metropolis, sans-serif; + font-size: 20px; + font-weight: 600; + line-height: 28px; +} + +.AttributeAppliesTo__emptyStateHelperText { + margin: 0 0 12px; + color: rgba(var(--center-channel-color-rgb), 0.75); + font-size: 14px; + line-height: 20px; +} + +.AttributeAppliesTo__list { + display: flex; + flex-direction: column; + gap: 8px; +} + +.AttributeAppliesTo__list + .AttributeAppliesTo__trigger { + margin-top: 12px; +} diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.test.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.test.tsx new file mode 100644 index 000000000000..b420cbc01acc --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.test.tsx @@ -0,0 +1,97 @@ +// 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 AttributeAppliesTo from './attribute_applies_to'; +import type {ResourceObjectType} from './attribute_applies_to_constants'; + +describe('AttributeAppliesTo', () => { + const onAdd = jest.fn(); + const onRemove = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + const renderComponent = (props: Partial> = {}) => { + return renderWithContext( + , + ); + }; + + it('renders the empty state when appliesTo is empty', () => { + renderComponent(); + expect(screen.getByTestId('attributeAppliesToEmptyState')).toBeInTheDocument(); + expect(screen.queryByTestId(/attributeAppliesToRow-/)).not.toBeInTheDocument(); + }); + + it('shows both Add-resource triggers when the empty state is showing', () => { + renderComponent(); + const header = screen.getByTestId('attributeAppliesToAddResourceButtonHeader'); + const inline = screen.getByTestId('attributeAppliesToAddResourceButtonInline'); + expect(header).toBeVisible(); + expect(inline).toBeVisible(); + expect(header).toHaveAccessibleName('Add resource'); + expect(inline).toHaveAccessibleName('Add resource'); + expect(header).toHaveClass('btn-tertiary'); + expect(inline).toHaveClass('btn-primary'); + }); + + it('offers exactly the not-yet-selected types, in Users -> Channels -> Posts order', async () => { + renderComponent({appliesTo: ['channel']}); + + expect(screen.getByTestId('attributeAppliesToAddResourceButtonHeader')).toHaveClass('btn-tertiary'); + expect(screen.getByTestId('attributeAppliesToAddResourceButtonHeader')).toHaveAccessibleName('Add resource'); + expect(screen.getByTestId('attributeAppliesToAddResourceButtonInline')).toHaveClass('btn-tertiary'); + expect(screen.getByTestId('attributeAppliesToAddResourceButtonInline')).toHaveAccessibleName('Add another resource'); + + await userEvent.click(screen.getByTestId('attributeAppliesToAddResourceButtonHeader')); + const items = screen.getAllByRole('menuitem'); + expect(items.map((item) => item.textContent)).toEqual(['Users', 'Posts']); + }); + + it('calls onAdd with the correct type when a menu item is selected', async () => { + renderComponent(); + + await userEvent.click(screen.getByTestId('attributeAppliesToAddResourceButtonHeader')); + await userEvent.click(screen.getByRole('menuitem', {name: 'Channels'})); + + // A non-checkbox/radio Menu.Item defers onClick until after the + // menu's close transition completes (menu_item.tsx's addOnClosedListener). + await waitFor(() => expect(onAdd).toHaveBeenCalledWith('channel')); + }); + + it('renders one row per entry in appliesTo, in insertion order', () => { + renderComponent({appliesTo: ['post', 'user']}); + + const rows = screen.getAllByTestId(/^attributeAppliesToRow-(user|channel|post)$/); + expect(rows.map((row) => row.getAttribute('data-testid'))).toEqual(['attributeAppliesToRow-post', 'attributeAppliesToRow-user']); + }); + + it('hides both Add-resource triggers entirely once all three types are present', () => { + renderComponent({appliesTo: ['user', 'channel', 'post'] as ResourceObjectType[]}); + + expect(screen.queryByTestId('attributeAppliesToAddResourceButtonHeader')).not.toBeInTheDocument(); + expect(screen.queryByTestId('attributeAppliesToAddResourceButtonInline')).not.toBeInTheDocument(); + }); + + it('calls onRemove with each row\'s own type, once expanded', async () => { + renderComponent({appliesTo: ['user', 'channel']}); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-remove')); + expect(onRemove).toHaveBeenCalledWith('channel'); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-remove')); + expect(onRemove).toHaveBeenCalledWith('user'); + }); +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.tsx new file mode 100644 index 000000000000..02f66ba3c35c --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to.tsx @@ -0,0 +1,174 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import classNames from 'classnames'; +import type {ComponentType} from 'react'; +import React, {useMemo} from 'react'; +import {defineMessages, FormattedMessage, useIntl} from 'react-intl'; + +import {PlusIcon} from '@mattermost/compass-icons/components'; +import type {ButtonEmphasis} from '@mattermost/shared/components/button'; +import {buttonClassNames} from '@mattermost/shared/components/button'; + +import Card from 'components/card/card'; +import * as Menu from 'components/menu'; + +import AttributeAppliesToChannelItem from './attribute_applies_to_channel_item'; +import {ALL_RESOURCE_TYPES, ATTRIBUTE_APPLIES_TO_ADD_HEADER_TRIGGER_ID, RESOURCE_TYPE_ICONS, resourceTypeLabels} from './attribute_applies_to_constants'; +import type {AttributeAppliesToItemProps, ResourceObjectType} from './attribute_applies_to_constants'; +import AttributeAppliesToPostItem from './attribute_applies_to_post_item'; +import AttributeAppliesToUserItem from './attribute_applies_to_user_item'; + +import './attribute_applies_to.scss'; + +type Props = { + appliesTo: ResourceObjectType[]; + disabled?: boolean; + onAdd: (type: ResourceObjectType) => void; + onRemove: (type: ResourceObjectType) => void; +}; + +// Every entry here must implement AttributeAppliesToItemProps exactly -- +// TypeScript rejects the map itself if any of the three row components' +// props drift from that shared signature, rather than only failing wherever +// they happen to get used. +const RESOURCE_TYPE_ITEM_COMPONENTS: Record> = { + user: AttributeAppliesToUserItem, + channel: AttributeAppliesToChannelItem, + post: AttributeAppliesToPostItem, +}; + +// Owns only the Card chrome (header, "Add resource" triggers, empty state) +// and renders one per-type row component per entry in appliesTo (a dedicated +// component per resource type -- AttributeAppliesToUserItem/ChannelItem/ +// PostItem -- rather than one generic item parameterized by resourceType). +// Holds no selection state of its own -- "available" picker options are +// derived purely from props on every render. Makes no data-mutating dispatch +// calls, no Client4/API calls (see R6 -- the page owns all of that). +function AttributeAppliesTo({appliesTo, disabled = false, onAdd, onRemove}: Props): JSX.Element { + const {formatMessage} = useIntl(); + + const availableTypes = useMemo( + () => ALL_RESOURCE_TYPES.filter((type) => !appliesTo.includes(type)), + [appliesTo], + ); + + const renderAddResourceMenu = (triggerId: string, dataTestId: string, label: string, emphasis: ButtonEmphasis) => ( + + + {label} + + ), + dataTestId, + }} + menu={{ + id: `${triggerId}-menu`, + 'aria-label': label, + }} + > + {availableTypes.map((type) => { + const ItemIcon = RESOURCE_TYPE_ICONS[type]; + return ( + } + onClick={() => onAdd(type)} + labels={} + /> + ); + })} + + ); + + return ( +
+ + +
+
+ +
+ +
+ {availableTypes.length > 0 && renderAddResourceMenu( + ATTRIBUTE_APPLIES_TO_ADD_HEADER_TRIGGER_ID, + 'attributeAppliesToAddResourceButtonHeader', + formatMessage(messages.addResourceHeader), + 'tertiary', + )} +
+ + {appliesTo.length === 0 ? ( +
+
+ +
+

+ +

+ {availableTypes.length > 0 && renderAddResourceMenu( + 'attribute-applies-to-add-inline', + 'attributeAppliesToAddResourceButtonInline', + formatMessage(messages.addResourceHeader), + 'primary', + )} +
+ ) : ( + <> +
+ {appliesTo.map((type) => { + const Item = RESOURCE_TYPE_ITEM_COMPONENTS[type]; + return ( + onRemove(type)} + /> + ); + })} +
+ {availableTypes.length > 0 && renderAddResourceMenu( + 'attribute-applies-to-add-inline', + 'attributeAppliesToAddResourceButtonInline', + formatMessage(messages.addResourceInline), + 'tertiary', + )} + + )} +
+
+
+ ); +} + +export default AttributeAppliesTo; + +const messages = defineMessages({ + title: {id: 'admin.global_attributes.attribute_details.applies_to.title', defaultMessage: 'Applies to'}, + subtitle: {id: 'admin.global_attributes.attribute_details.applies_to.subtitle', defaultMessage: 'Resources this attribute applies to, and who can set the value on each.'}, + emptyStateHeading: {id: 'admin.global_attributes.attribute_details.applies_to.empty_state.heading', defaultMessage: 'No resources yet'}, + emptyStateHelperText: { + id: 'admin.global_attributes.attribute_details.applies_to.empty_state.helper_text', + defaultMessage: 'Add a resource to apply this attribute to users, channels, or posts.', + }, + addResourceHeader: {id: 'admin.global_attributes.attribute_details.applies_to.add_resource_header', defaultMessage: 'Add resource'}, + addResourceInline: {id: 'admin.global_attributes.attribute_details.applies_to.add_resource_inline', defaultMessage: 'Add another resource'}, +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_channel_item.test.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_channel_item.test.tsx new file mode 100644 index 000000000000..e8aab15d3847 --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_channel_item.test.tsx @@ -0,0 +1,80 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import {Client4} from 'mattermost-redux/client'; + +import {renderWithContext, screen, userEvent} from 'tests/react_testing_utils'; + +import AttributeAppliesToChannelItem from './attribute_applies_to_channel_item'; + +describe('AttributeAppliesToChannelItem', () => { + const onRemove = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + const renderComponent = (props: Partial> = {}) => { + return renderWithContext( + , + ); + }; + + it('renders the Channels label', () => { + renderComponent(); + expect(screen.getByTestId('attributeAppliesToRow-channel')).toHaveTextContent('Channels'); + }); + + it('starts collapsed, with no Remove button, and clicking the toggle reveals the placeholder body and Remove', async () => { + renderComponent(); + + expect(screen.queryByTestId('attributeAppliesToRow-channel-body')).not.toBeInTheDocument(); + expect(screen.queryByTestId('attributeAppliesToRow-channel-remove')).not.toBeInTheDocument(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-toggle')); + expect(screen.getByTestId('attributeAppliesToRow-channel-body')).toHaveTextContent('No additional settings for this resource yet.'); + expect(screen.getByTestId('attributeAppliesToRow-channel-remove')).toBeVisible(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-toggle')); + expect(screen.queryByTestId('attributeAppliesToRow-channel-body')).not.toBeInTheDocument(); + expect(screen.queryByTestId('attributeAppliesToRow-channel-remove')).not.toBeInTheDocument(); + }); + + it('calls onRemove exactly once when Remove is clicked, once expanded', async () => { + renderComponent(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-remove')); + expect(onRemove).toHaveBeenCalledTimes(1); + }); + + it('disables the toggle, and the Remove button once expanded', async () => { + const {rerender} = renderComponent(); + + // Expand while enabled, then disable -- isOpen is local state, so it survives + // the prop change, letting Remove's own disabled state be asserted directly. + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-toggle')); + const disabledProps = {onRemove, disabled: true}; + rerender(); + + expect(screen.getByTestId('attributeAppliesToRow-channel-toggle')).toBeDisabled(); + expect(screen.getByTestId('attributeAppliesToRow-channel-remove')).toBeDisabled(); + }); + + it('makes no Client4 calls and no data-mutating dispatch', async () => { + const createPropertyField = jest.spyOn(Client4, 'createPropertyField'); + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField'); + + renderComponent(); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-channel-remove')); + + expect(createPropertyField).not.toHaveBeenCalled(); + expect(deletePropertyField).not.toHaveBeenCalled(); + }); +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_channel_item.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_channel_item.tsx new file mode 100644 index 000000000000..d7e06290a631 --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_channel_item.tsx @@ -0,0 +1,100 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import classNames from 'classnames'; +import React, {useState} from 'react'; +import {defineMessages, FormattedMessage, useIntl} from 'react-intl'; + +import {ChevronDownIcon, ProductChannelsIcon} from '@mattermost/compass-icons/components'; +import {Button} from '@mattermost/shared/components/button'; + +import {resourceTypeLabels} from './attribute_applies_to_constants'; +import type {AttributeAppliesToItemProps} from './attribute_applies_to_constants'; + +import './attribute_applies_to_item.scss'; + +const BODY_ID = 'attribute-applies-to-channel-panel'; + +// The Channels row of the Applies-to list -- owns its own expand/collapse +// state (deliberately not the shared Accordion component, see the plan's +// Decisions table: AccordionCard renders the row itself from plain data with +// no slot for a child component to own it, and its open-row tracking is by +// array index, which misattributes state when a row is removed from the +// middle of the list). Remove is only reachable once expanded -- there is no +// collapsed-row remove affordance. +function AttributeAppliesToChannelItem({disabled = false, onRemove}: AttributeAppliesToItemProps): JSX.Element { + const {formatMessage} = useIntl(); + const [isOpen, setIsOpen] = useState(false); + + const label = formatMessage(resourceTypeLabels.channel); + const toggleLabel = formatMessage(isOpen ? messages.collapseLabel : messages.expandLabel, {label}); + + return ( +
+
+ + {isOpen && ( + + )} +
+ {isOpen && ( +
+
+ + + +
+
+ )} +
+ ); +} + +export default AttributeAppliesToChannelItem; + +const messages = defineMessages({ + expandLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.expand', defaultMessage: 'Expand {label}'}, + collapseLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.collapse', defaultMessage: 'Collapse {label}'}, + removeLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.remove', defaultMessage: 'Remove resource'}, + bodyPlaceholder: { + id: 'admin.global_attributes.attribute_details.applies_to.item.body_placeholder', + defaultMessage: 'No additional settings for this resource yet.', + }, +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_constants.ts b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_constants.ts new file mode 100644 index 000000000000..2dd377b1d431 --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_constants.ts @@ -0,0 +1,43 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {ComponentType} from 'react'; +import type {MessageDescriptor} from 'react-intl'; +import {defineMessages} from 'react-intl'; + +import {AccountOutlineIcon, MessageTextOutlineIcon, ProductChannelsIcon} from '@mattermost/compass-icons/components'; +import type IconProps from '@mattermost/compass-icons/components/props'; + +export type ResourceObjectType = 'user' | 'channel' | 'post'; + +// Fixed Users -> Channels -> Posts order used everywhere a resource list is +// rendered (the picker menu, and used to derive "available" options) -- not +// the insertion order of a saved appliesTo array, which is separate. +export const ALL_RESOURCE_TYPES: ResourceObjectType[] = ['user', 'channel', 'post']; + +// Every per-resource-type row component (AttributeAppliesToUserItem/ +// ChannelItem/PostItem) implements exactly this prop signature -- there's no +// resourceType prop, since each component already knows its own type. Sharing +// one type here (rather than each component declaring an identical local +// `Props`) is what AttributeAppliesTo relies on to treat all three +// interchangeably in its render switch. +export type AttributeAppliesToItemProps = { + disabled?: boolean; + onRemove: () => void; +}; + +// Shared between AttributeAppliesTo (which owns the button) and AttributeDetails +// (which moves focus back to it after a pre-save resource removal). +export const ATTRIBUTE_APPLIES_TO_ADD_HEADER_TRIGGER_ID = 'attribute-applies-to-add-header'; + +export const RESOURCE_TYPE_ICONS: Record> = { + user: AccountOutlineIcon, + channel: ProductChannelsIcon, + post: MessageTextOutlineIcon, +}; + +export const resourceTypeLabels: Record = defineMessages({ + user: {id: 'admin.global_attributes.attribute_details.applies_to.resource_type.user', defaultMessage: 'Users'}, + channel: {id: 'admin.global_attributes.attribute_details.applies_to.resource_type.channel', defaultMessage: 'Channels'}, + post: {id: 'admin.global_attributes.attribute_details.applies_to.resource_type.post', defaultMessage: 'Posts'}, +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_item.scss b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_item.scss new file mode 100644 index 000000000000..c29054bff798 --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_item.scss @@ -0,0 +1,95 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +.AttributeAppliesToItem { + overflow: hidden; + border: 1px solid rgba(var(--center-channel-color-rgb), 0.16); + border-radius: 4px; +} + +.AttributeAppliesToItem__header { + display: flex; + align-items: center; + justify-content: space-between; + padding: 0 4px 0 12px; + background: transparent; + gap: 8px; + transition: background 0.15s ease-in-out; +} + +.AttributeAppliesToItem--open .AttributeAppliesToItem__header { + background: rgba(var(--center-channel-color-rgb), 0.03); +} + +// Header already uses gap; Compass .btn + .btn would add a second 8px. +.AttributeAppliesToItem__header .btn + .btn { + margin-left: 0; +} + +// Compass Button always applies .btn.btn-quaternary (blue text, 40px height, +// hover tint). Repeat those classes here so this accordion chrome wins on +// specificity regardless of stylesheet order. +.btn.btn-quaternary.AttributeAppliesToItem__toggle { + min-width: 0; + height: auto; + flex: 1; + justify-content: flex-start; + padding: 10px 0; + border-radius: 0; + background: transparent; + color: var(--center-channel-color); + + &:hover:not(:disabled), + &:active:not(:disabled) { + background: transparent; + color: var(--center-channel-color); + } + + &:disabled { + cursor: not-allowed; + opacity: 0.5; + } +} + +.AttributeAppliesToItem__chevron { + color: rgba(var(--center-channel-color-rgb), 0.56); + transition: transform 150ms ease-in-out; +} + +.AttributeAppliesToItem__chevron--open { + transform: rotate(-180deg); +} + +.AttributeAppliesToItem__label { + font-size: 14px; + font-weight: 600; +} + +.btn.AttributeAppliesToItem__remove { + flex-shrink: 0; + margin-right: 12px; +} + +.AttributeAppliesToItem__body { + padding: 0 12px 12px; + color: rgba(var(--center-channel-color-rgb), 0.72); + font-size: 12px; +} + +.AttributeAppliesToItem__row { + display: grid; + align-items: start; + padding: 12px 0; + border-bottom: 1px solid rgba(var(--center-channel-color-rgb), 0.08); + column-gap: 12px; + grid-template-columns: 176px 1fr; + + &:last-child { + padding-bottom: 0; + border-bottom: none; + } + + > :only-child { + grid-column: 1 / -1; + } +} diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_post_item.test.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_post_item.test.tsx new file mode 100644 index 000000000000..076738ccebf1 --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_post_item.test.tsx @@ -0,0 +1,80 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import {Client4} from 'mattermost-redux/client'; + +import {renderWithContext, screen, userEvent} from 'tests/react_testing_utils'; + +import AttributeAppliesToPostItem from './attribute_applies_to_post_item'; + +describe('AttributeAppliesToPostItem', () => { + const onRemove = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + const renderComponent = (props: Partial> = {}) => { + return renderWithContext( + , + ); + }; + + it('renders the Posts label', () => { + renderComponent(); + expect(screen.getByTestId('attributeAppliesToRow-post')).toHaveTextContent('Posts'); + }); + + it('starts collapsed, with no Remove button, and clicking the toggle reveals the placeholder body and Remove', async () => { + renderComponent(); + + expect(screen.queryByTestId('attributeAppliesToRow-post-body')).not.toBeInTheDocument(); + expect(screen.queryByTestId('attributeAppliesToRow-post-remove')).not.toBeInTheDocument(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-post-toggle')); + expect(screen.getByTestId('attributeAppliesToRow-post-body')).toHaveTextContent('No additional settings for this resource yet.'); + expect(screen.getByTestId('attributeAppliesToRow-post-remove')).toBeVisible(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-post-toggle')); + expect(screen.queryByTestId('attributeAppliesToRow-post-body')).not.toBeInTheDocument(); + expect(screen.queryByTestId('attributeAppliesToRow-post-remove')).not.toBeInTheDocument(); + }); + + it('calls onRemove exactly once when Remove is clicked, once expanded', async () => { + renderComponent(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-post-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-post-remove')); + expect(onRemove).toHaveBeenCalledTimes(1); + }); + + it('disables the toggle, and the Remove button once expanded', async () => { + const {rerender} = renderComponent(); + + // Expand while enabled, then disable -- isOpen is local state, so it survives + // the prop change, letting Remove's own disabled state be asserted directly. + await userEvent.click(screen.getByTestId('attributeAppliesToRow-post-toggle')); + const disabledProps = {onRemove, disabled: true}; + rerender(); + + expect(screen.getByTestId('attributeAppliesToRow-post-toggle')).toBeDisabled(); + expect(screen.getByTestId('attributeAppliesToRow-post-remove')).toBeDisabled(); + }); + + it('makes no Client4 calls and no data-mutating dispatch', async () => { + const createPropertyField = jest.spyOn(Client4, 'createPropertyField'); + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField'); + + renderComponent(); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-post-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-post-remove')); + + expect(createPropertyField).not.toHaveBeenCalled(); + expect(deletePropertyField).not.toHaveBeenCalled(); + }); +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_post_item.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_post_item.tsx new file mode 100644 index 000000000000..cb939413eb4c --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_post_item.tsx @@ -0,0 +1,100 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import classNames from 'classnames'; +import React, {useState} from 'react'; +import {defineMessages, FormattedMessage, useIntl} from 'react-intl'; + +import {ChevronDownIcon, MessageTextOutlineIcon} from '@mattermost/compass-icons/components'; +import {Button} from '@mattermost/shared/components/button'; + +import {resourceTypeLabels} from './attribute_applies_to_constants'; +import type {AttributeAppliesToItemProps} from './attribute_applies_to_constants'; + +import './attribute_applies_to_item.scss'; + +const BODY_ID = 'attribute-applies-to-post-panel'; + +// The Posts row of the Applies-to list -- owns its own expand/collapse state +// (deliberately not the shared Accordion component, see the plan's Decisions +// table: AccordionCard renders the row itself from plain data with no slot +// for a child component to own it, and its open-row tracking is by array +// index, which misattributes state when a row is removed from the middle of +// the list). Remove is only reachable once expanded -- there is no +// collapsed-row remove affordance. +function AttributeAppliesToPostItem({disabled = false, onRemove}: AttributeAppliesToItemProps): JSX.Element { + const {formatMessage} = useIntl(); + const [isOpen, setIsOpen] = useState(false); + + const label = formatMessage(resourceTypeLabels.post); + const toggleLabel = formatMessage(isOpen ? messages.collapseLabel : messages.expandLabel, {label}); + + return ( +
+
+ + {isOpen && ( + + )} +
+ {isOpen && ( +
+
+ + + +
+
+ )} +
+ ); +} + +export default AttributeAppliesToPostItem; + +const messages = defineMessages({ + expandLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.expand', defaultMessage: 'Expand {label}'}, + collapseLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.collapse', defaultMessage: 'Collapse {label}'}, + removeLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.remove', defaultMessage: 'Remove resource'}, + bodyPlaceholder: { + id: 'admin.global_attributes.attribute_details.applies_to.item.body_placeholder', + defaultMessage: 'No additional settings for this resource yet.', + }, +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_user_item.test.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_user_item.test.tsx new file mode 100644 index 000000000000..5adfa5a65174 --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_user_item.test.tsx @@ -0,0 +1,80 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import {Client4} from 'mattermost-redux/client'; + +import {renderWithContext, screen, userEvent} from 'tests/react_testing_utils'; + +import AttributeAppliesToUserItem from './attribute_applies_to_user_item'; + +describe('AttributeAppliesToUserItem', () => { + const onRemove = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + const renderComponent = (props: Partial> = {}) => { + return renderWithContext( + , + ); + }; + + it('renders the Users label', () => { + renderComponent(); + expect(screen.getByTestId('attributeAppliesToRow-user')).toHaveTextContent('Users'); + }); + + it('starts collapsed, with no Remove button, and clicking the toggle reveals the placeholder body and Remove', async () => { + renderComponent(); + + expect(screen.queryByTestId('attributeAppliesToRow-user-body')).not.toBeInTheDocument(); + expect(screen.queryByTestId('attributeAppliesToRow-user-remove')).not.toBeInTheDocument(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-toggle')); + expect(screen.getByTestId('attributeAppliesToRow-user-body')).toHaveTextContent('No additional settings for this resource yet.'); + expect(screen.getByTestId('attributeAppliesToRow-user-remove')).toBeVisible(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-toggle')); + expect(screen.queryByTestId('attributeAppliesToRow-user-body')).not.toBeInTheDocument(); + expect(screen.queryByTestId('attributeAppliesToRow-user-remove')).not.toBeInTheDocument(); + }); + + it('calls onRemove exactly once when Remove is clicked, once expanded', async () => { + renderComponent(); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-remove')); + expect(onRemove).toHaveBeenCalledTimes(1); + }); + + it('disables the toggle, and the Remove button once expanded', async () => { + const {rerender} = renderComponent(); + + // Expand while enabled, then disable -- isOpen is local state, so it survives + // the prop change, letting Remove's own disabled state be asserted directly. + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-toggle')); + const disabledProps = {onRemove, disabled: true}; + rerender(); + + expect(screen.getByTestId('attributeAppliesToRow-user-toggle')).toBeDisabled(); + expect(screen.getByTestId('attributeAppliesToRow-user-remove')).toBeDisabled(); + }); + + it('makes no Client4 calls and no data-mutating dispatch', async () => { + const createPropertyField = jest.spyOn(Client4, 'createPropertyField'); + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField'); + + renderComponent(); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-remove')); + + expect(createPropertyField).not.toHaveBeenCalled(); + expect(deletePropertyField).not.toHaveBeenCalled(); + }); +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_user_item.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_user_item.tsx new file mode 100644 index 000000000000..09b0ea5620c6 --- /dev/null +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_applies_to_user_item.tsx @@ -0,0 +1,100 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import classNames from 'classnames'; +import React, {useState} from 'react'; +import {defineMessages, FormattedMessage, useIntl} from 'react-intl'; + +import {AccountOutlineIcon, ChevronDownIcon} from '@mattermost/compass-icons/components'; +import {Button} from '@mattermost/shared/components/button'; + +import {resourceTypeLabels} from './attribute_applies_to_constants'; +import type {AttributeAppliesToItemProps} from './attribute_applies_to_constants'; + +import './attribute_applies_to_item.scss'; + +const BODY_ID = 'attribute-applies-to-user-panel'; + +// The Users row of the Applies-to list -- owns its own expand/collapse state +// (deliberately not the shared Accordion component, see the plan's Decisions +// table: AccordionCard renders the row itself from plain data with no slot +// for a child component to own it, and its open-row tracking is by array +// index, which misattributes state when a row is removed from the middle of +// the list). Remove is only reachable once expanded -- there is no +// collapsed-row remove affordance. +function AttributeAppliesToUserItem({disabled = false, onRemove}: AttributeAppliesToItemProps): JSX.Element { + const {formatMessage} = useIntl(); + const [isOpen, setIsOpen] = useState(false); + + const label = formatMessage(resourceTypeLabels.user); + const toggleLabel = formatMessage(isOpen ? messages.collapseLabel : messages.expandLabel, {label}); + + return ( +
+
+ + {isOpen && ( + + )} +
+ {isOpen && ( +
+
+ + + +
+
+ )} +
+ ); +} + +export default AttributeAppliesToUserItem; + +const messages = defineMessages({ + expandLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.expand', defaultMessage: 'Expand {label}'}, + collapseLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.collapse', defaultMessage: 'Collapse {label}'}, + removeLabel: {id: 'admin.global_attributes.attribute_details.applies_to.item.remove', defaultMessage: 'Remove resource'}, + bodyPlaceholder: { + id: 'admin.global_attributes.attribute_details.applies_to.item.body_placeholder', + defaultMessage: 'No additional settings for this resource yet.', + }, +}); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.scss b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.scss index eddce759c25b..d8b1a517cc73 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.scss +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.scss @@ -75,7 +75,10 @@ } .AttributeDetails__fieldControl { + display: flex; min-width: 0; + flex-direction: column; + gap: 8px; } .AttributeDetails__uniqueName { @@ -87,7 +90,7 @@ .AttributeDetails__uniqueNameCaption { display: flex; align-items: center; - font-size: 14px; + font-size: 12px; gap: 8px; } @@ -100,12 +103,12 @@ .AttributeDetails__uniqueNameInput { width: 300px; max-width: 100%; - padding: 8px 12px; + padding: 4px 8px; border: 1px solid rgba(var(--center-channel-color-rgb), 0.16); border-radius: 4px; background: var(--center-channel-bg); color: var(--center-channel-color); - font-size: 14px; + font-size: 12px; &:hover { border-color: rgba(var(--center-channel-color-rgb), 0.32); @@ -132,7 +135,7 @@ background: none; color: var(--link-color); cursor: pointer; - font-size: 14px; + font-size: 12px; &:hover { text-decoration: underline; @@ -194,6 +197,15 @@ color: rgba(var(--center-channel-color-rgb), 0.56); font-size: 18px; } + + &:disabled { + cursor: default; + opacity: 0.64; + + &:hover { + border-color: rgba(var(--center-channel-color-rgb), 0.16); + } + } } .AttributeDetails__typeButtonInner { diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.test.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.test.tsx index fd62b3c7b34b..158f8faf5bec 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.test.tsx +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.test.tsx @@ -80,7 +80,7 @@ describe('AttributeDetails', () => { expect(screen.getByTestId('saveSetting')).toBeDisabled(); }); - it('reveals a focused editable Name input seeded with the current slug when Edit is clicked, and stops auto-updating it', async () => { + it('reveals a focused editable Name input seeded with the current slug when Edit is clicked', async () => { renderComponent(); await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); @@ -89,9 +89,6 @@ describe('AttributeDetails', () => { const nameInput = screen.getByTestId('attributeNameInput'); expect(nameInput).toHaveValue('my_attribute'); expect(nameInput).toHaveFocus(); - - await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), ' Extra'); - expect(nameInput).toHaveValue('my_attribute'); }); it('swaps the Edit link to Done while editing, and back to Edit (keeping the typed value) when Done is clicked', async () => { @@ -132,6 +129,39 @@ describe('AttributeDetails', () => { expect(screen.getByTestId('attributeUniqueNameValue')).toHaveTextContent('my_attribute_two'); }); + it('exits edit mode on blur without pinning if the seeded value was not changed', async () => { + renderComponent(); + + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await userEvent.click(screen.getByTestId('attributeNameEditLink')); + await userEvent.click(screen.getByTestId('attributeDisplayNameInput')); + + expect(screen.queryByTestId('attributeNameInput')).not.toBeInTheDocument(); + expect(screen.getByTestId('attributeNameEditLink')).toHaveTextContent('Edit'); + expect(screen.getByTestId('attributeUniqueNameValue')).toHaveTextContent('my_attribute'); + + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), ' Two'); + expect(screen.getByTestId('attributeUniqueNameValue')).toHaveTextContent('my_attribute_two'); + }); + + it('commits a typed Name on blur and stops auto-derivation', async () => { + renderComponent(); + + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await userEvent.click(screen.getByTestId('attributeNameEditLink')); + + const nameInput = screen.getByTestId('attributeNameInput'); + await userEvent.clear(nameInput); + await userEvent.type(nameInput, 'custom_name'); + await userEvent.click(screen.getByTestId('attributeDisplayNameInput')); + + expect(screen.queryByTestId('attributeNameInput')).not.toBeInTheDocument(); + expect(screen.getByTestId('attributeUniqueNameValue')).toHaveTextContent('custom_name'); + + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), ' Two'); + expect(screen.getByTestId('attributeUniqueNameValue')).toHaveTextContent('custom_name'); + }); + it('reverts to auto-derived mode if Done is clicked with an empty field on the very first edit', async () => { renderComponent(); @@ -183,7 +213,7 @@ describe('AttributeDetails', () => { expect(screen.getByTestId('saveSetting')).toBeDisabled(); }); - it('refuses to commit an invalid Name via Done or Enter, keeping the editor open', async () => { + it('refuses to commit an invalid Name via Done, Enter, or blur, keeping the editor open', async () => { renderComponent(); await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); @@ -204,6 +234,10 @@ describe('AttributeDetails', () => { await userEvent.type(screen.getByTestId('attributeNameInput'), '{Enter}'); expect(screen.getByTestId('attributeNameInput')).toHaveValue('for'); expect(screen.getByTestId('attributeUniqueNameError')).toHaveTextContent('reserved word'); + + await userEvent.click(screen.getByTestId('attributeDisplayNameInput')); + expect(screen.getByTestId('attributeNameInput')).toHaveValue('for'); + expect(screen.getByTestId('attributeNameEditLink')).toHaveTextContent('Done'); }); it('commits the Name once the invalid value is corrected', async () => { @@ -572,7 +606,7 @@ describe('AttributeDetails', () => { expect(screen.getByTestId('attributeUniqueNameValue')).toHaveTextContent('committed_name'); }); - it('disables the Display name input, Name input, and Edit/Done link while saving', async () => { + it('disables the Display name input and Edit link while saving', async () => { let resolveCreate: (value: PropertyField) => void = () => {}; jest.spyOn(Client4, 'createPropertyField').mockReturnValue(new Promise((resolve) => { resolveCreate = resolve; @@ -583,8 +617,9 @@ describe('AttributeDetails', () => { await userEvent.click(screen.getByTestId('attributeNameEditLink')); await userEvent.click(screen.getByTestId('saveSetting')); + // Clicking Save blurs the Name input first, which commits like Done. expect(screen.getByTestId('attributeDisplayNameInput')).toBeDisabled(); - expect(screen.getByTestId('attributeNameInput')).toBeDisabled(); + expect(screen.queryByTestId('attributeNameInput')).not.toBeInTheDocument(); expect(screen.getByTestId('attributeNameEditLink')).toBeDisabled(); resolveCreate({} as PropertyField); @@ -593,9 +628,10 @@ describe('AttributeDetails', () => { it('does not navigate or update state if the save resolves after the component has unmounted', async () => { let resolveCreate: (value: PropertyField) => void = () => {}; - jest.spyOn(Client4, 'createPropertyField').mockReturnValue(new Promise((resolve) => { + const createPromise = new Promise((resolve) => { resolveCreate = resolve; - })); + }); + jest.spyOn(Client4, 'createPropertyField').mockReturnValue(createPromise); const {unmount} = renderComponent(); await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); @@ -604,9 +640,11 @@ describe('AttributeDetails', () => { unmount(); resolveCreate({} as PropertyField); - // Flush the resolved promise's continuation without triggering an - // act() warning or a post-unmount navigation/dispatch. - await waitFor(() => Promise.resolve()); + // Await the hanging create, then one extra tick so handleSave's + // await resumes and finalizeSave runs -- waitFor(() => Promise.resolve()) + // returns on the first check and can pass before the mount guard. + await createPromise; + await Promise.resolve(); expect(mockHistoryPush).not.toHaveBeenCalled(); expect(mockSetNavigationBlocked).not.toHaveBeenCalledWith(false); }); @@ -670,6 +708,9 @@ describe('AttributeDetails', () => { await linkViaMenu(/AD\/LDAP/, 'employeeID'); expect(screen.getByTestId('attributeTypeMenuButton')).toHaveTextContent('Text'); + expect(screen.getByTestId('attributeTypeMenuButton')).toBeDisabled(); + expect(screen.getByTestId('attributeExternalSourceSynced')).toHaveTextContent(/^Synced with/); + expect(screen.queryByTestId('attributeOptionsHelp')).not.toBeInTheDocument(); expect(mockSetNavigationBlocked).toHaveBeenCalledWith(true); }); @@ -683,16 +724,32 @@ describe('AttributeDetails', () => { expect(screen.getByTestId('attributeExternalSourceChip-saml')).toBeInTheDocument(); }); - it('manually switching Type away from Text clears any linked sources', async () => { + it('disables the Type menu while a source is linked, and re-enables it after the last chip is removed', async () => { renderComponent(); + expect(screen.getByTestId('attributeTypeMenuButton')).not.toBeDisabled(); + expect(screen.getByTestId('attributeOptionsHelp')).toBeInTheDocument(); + await linkViaMenu(/AD\/LDAP/, 'employeeID'); - expect(screen.getByTestId('attributeExternalSourceChip-ldap')).toBeInTheDocument(); + expect(screen.getByTestId('attributeTypeMenuButton')).toBeDisabled(); + expect(screen.getByTestId('attributeTypeMenuButton')).toHaveTextContent('Text'); + expect(screen.queryByTestId('attributeOptionsHelp')).not.toBeInTheDocument(); + + await linkViaMenu(/^SAML/, 'position'); + expect(screen.getByTestId('attributeTypeMenuButton')).toBeDisabled(); + + // Removing one of two links must keep Type locked + await userEvent.click(screen.getByTestId('attributeExternalSourceChip-ldap-remove')); + expect(screen.getByTestId('attributeTypeMenuButton')).toBeDisabled(); + + await userEvent.click(screen.getByTestId('attributeExternalSourceChip-saml-remove')); + expect(screen.getByTestId('attributeTypeMenuButton')).not.toBeDisabled(); + expect(screen.getByTestId('attributeOptionsHelp')).toBeInTheDocument(); + expect(screen.queryByTestId('attributeExternalSourceSynced')).not.toBeInTheDocument(); await userEvent.click(screen.getByTestId('attributeTypeMenuButton')); await userEvent.click(screen.getByRole('menuitemradio', {name: 'Select'})); - - expect(screen.queryByTestId('attributeExternalSourceChip-ldap')).not.toBeInTheDocument(); + expect(screen.getByTestId('attributeTypeMenuButton')).toHaveTextContent('Select'); }); it('re-saving a linked chip with the unchanged value is a no-op -- no extra dirty-marking dispatch', async () => { @@ -727,4 +784,307 @@ describe('AttributeDetails', () => { expect(attrs).not.toHaveProperty('saml'); }); }); + + describe('applies to', () => { + // Opens the header Add-resource menu and picks `label` -- a non- + // checkbox/radio Menu.Item defers its onClick until after the menu's + // close transition (menu_item.tsx), so callers must await the row + // actually appearing rather than asserting immediately after the click. + const addResource = async (label: string, resourceType: string) => { + await userEvent.click(screen.getByTestId('attributeAppliesToAddResourceButtonHeader')); + await userEvent.click(screen.getByRole('menuitem', {name: label})); + await waitFor(() => expect(screen.getByTestId(`attributeAppliesToRow-${resourceType}`)).toBeInTheDocument()); + }; + + it('creates one linked field per selected resource, in selection order, with linked_field_id and attrs populated', async () => { + const createPropertyField = jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockResolvedValueOnce({id: 'user-field-id'} as PropertyField). + mockResolvedValueOnce({id: 'channel-field-id'} as PropertyField); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Users', 'user'); + await addResource('Channels', 'channel'); + await userEvent.click(screen.getByTestId('saveSetting')); + + await waitFor(() => expect(mockHistoryPush).toHaveBeenCalled()); + + expect(createPropertyField).toHaveBeenNthCalledWith(1, 'access_control', 'template', expect.objectContaining({ + name: 'my_attribute', + })); + expect(createPropertyField).toHaveBeenNthCalledWith(2, 'access_control', 'user', expect.objectContaining({ + name: 'my_attribute', + type: 'text', + target_type: 'system', + target_id: '', + linked_field_id: 'template-id', + attrs: {display_name: 'My Attribute'}, + })); + expect(createPropertyField).toHaveBeenNthCalledWith(3, 'access_control', 'channel', expect.objectContaining({ + linked_field_id: 'template-id', + attrs: {display_name: 'My Attribute'}, + })); + }); + + it('picks up the current appliesTo value even though canSave does not depend on it (stale-closure regression)', async () => { + const createPropertyField = jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockResolvedValueOnce({id: 'user-field-id'} as PropertyField). + mockResolvedValueOnce({id: 'channel-field-id'} as PropertyField); + + renderComponent(); + + // Display name is the only thing typed before resources are + // added -- no other handleSave dependency changes after this, + // so if appliesTo were missing from the dependency array, + // handleSave would still close over the empty array it captured + // on an earlier render. + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Users', 'user'); + await addResource('Channels', 'channel'); + await userEvent.click(screen.getByTestId('saveSetting')); + + await waitFor(() => expect(mockHistoryPush).toHaveBeenCalled()); + expect(createPropertyField).toHaveBeenCalledTimes(3); + }); + + it('rolls back the already-created linked field and the template when a later linked-field create fails, and re-enables Save', async () => { + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField').mockResolvedValue({status: 'OK'}); + jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockResolvedValueOnce({id: 'user-field-id'} as PropertyField). + mockRejectedValueOnce(new Error('boom')); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Users', 'user'); + await addResource('Channels', 'channel'); + await userEvent.click(screen.getByTestId('saveSetting')); + + await waitFor(() => expect(deletePropertyField).toHaveBeenCalledWith('access_control', 'user', 'user-field-id')); + await waitFor(() => expect(deletePropertyField).toHaveBeenCalledWith('access_control', 'template', 'template-id')); + + expect(await screen.findByTestId('attributeSaveError')).toHaveTextContent('Channels'); + expect(mockHistoryPush).not.toHaveBeenCalled(); + expect(screen.getByTestId('saveSetting')).not.toBeDisabled(); + }); + + it('continues rolling back every linked field even if one delete fails, skips the template delete, and names the survivor', async () => { + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField'). + mockImplementationOnce(() => Promise.reject(new Error('delete failed'))). + mockImplementationOnce(() => Promise.resolve({status: 'OK'})); + jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockResolvedValueOnce({id: 'user-field-id'} as PropertyField). + mockResolvedValueOnce({id: 'channel-field-id'} as PropertyField). + mockRejectedValueOnce(new Error('boom')); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Users', 'user'); + await addResource('Channels', 'channel'); + await addResource('Posts', 'post'); + await userEvent.click(screen.getByTestId('saveSetting')); + + await waitFor(() => expect(deletePropertyField).toHaveBeenCalledTimes(2)); + expect(deletePropertyField).toHaveBeenNthCalledWith(1, 'access_control', 'user', 'user-field-id'); + expect(deletePropertyField).toHaveBeenNthCalledWith(2, 'access_control', 'channel', 'channel-field-id'); + expect(deletePropertyField).not.toHaveBeenCalledWith('access_control', 'template', 'template-id'); + + const banner = await screen.findByTestId('attributeSaveError'); + expect(banner).toHaveTextContent('My Attribute'); + expect(banner).toHaveTextContent('Users'); + expect(mockHistoryPush).not.toHaveBeenCalled(); + expect(screen.getByTestId('saveSetting')).not.toBeDisabled(); + }); + + it.each([ + ['app.property_field.create.name_conflict.app_error', /already used by a User Attribute/i], + ['app.property_field.create.limit_reached.app_error', /maximum number of User Attributes/i], + ['app.property_field.create.group_limit_reached.app_error', /maximum number of User Attributes/i], + ])('renders the distinct CPA banner for a Users-linked-field %s', async (serverErrorId, expectedText) => { + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField').mockResolvedValue({status: 'OK'}); + jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockRejectedValueOnce(makeClientError(serverErrorId, 'server message')); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Users', 'user'); + await userEvent.click(screen.getByTestId('saveSetting')); + + expect(await screen.findByText(expectedText)).toBeInTheDocument(); + expect(mockHistoryPush).not.toHaveBeenCalled(); + expect(screen.getByTestId('saveSetting')).not.toBeDisabled(); + + // Rollback still runs even when the very first linked-field + // create is the one that fails (createdLinkedFields is empty, + // so the inner rollback loop runs zero iterations) -- confirms + // the template itself is still cleaned up in this zero-survivor case. + expect(deletePropertyField).toHaveBeenCalledWith('access_control', 'template', 'template-id'); + }); + + it('renders the generic applies-to banner for a Channels-linked-field name conflict, not the CPA copy', async () => { + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField').mockResolvedValue({status: 'OK'}); + jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockRejectedValueOnce(makeClientError('app.property_field.create.name_conflict.app_error', 'server message')); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Channels', 'channel'); + await userEvent.click(screen.getByTestId('saveSetting')); + + const banner = await screen.findByTestId('attributeSaveError'); + expect(banner).toHaveTextContent('Channels'); + expect(banner).toHaveTextContent('Nothing was saved'); + expect(banner).not.toHaveTextContent('User Attribute'); + expect(deletePropertyField).toHaveBeenCalledWith('access_control', 'template', 'template-id'); + }); + + it('reports the leftover template when linked-field rollback succeeds but the template delete fails', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + jest.spyOn(Client4, 'deletePropertyField').mockRejectedValue(new Error('template delete failed')); + jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockRejectedValueOnce(new Error('boom')); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Channels', 'channel'); + await userEvent.click(screen.getByTestId('saveSetting')); + + const banner = await screen.findByTestId('attributeSaveError'); + expect(banner).toHaveTextContent('My Attribute'); + expect(banner).toHaveTextContent('could not be cleaned up'); + expect(banner).not.toHaveTextContent('Nothing was saved'); + expect(mockHistoryPush).not.toHaveBeenCalled(); + expect(screen.getByTestId('saveSetting')).not.toBeDisabled(); + }); + + it('leaves Save enabled with zero Applies-to resources selected (unchanged from current behavior)', async () => { + jest.spyOn(Client4, 'createPropertyField').mockResolvedValue({id: 'template-id'} as PropertyField); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + expect(screen.getByTestId('saveSetting')).not.toBeDisabled(); + + await userEvent.click(screen.getByTestId('saveSetting')); + await waitFor(() => expect(mockHistoryPush).toHaveBeenCalled()); + }); + + it('calls markDirty (navigation-blocked, error cleared) when a resource is added', async () => { + jest.spyOn(Client4, 'createPropertyField').mockRejectedValue(makeClientError('app.property_field.create.name_conflict.app_error', 'already exists')); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await userEvent.click(screen.getByTestId('saveSetting')); + expect(await screen.findByText(/already exists/i)).toBeInTheDocument(); + + mockSetNavigationBlocked.mockClear(); + await addResource('Users', 'user'); + + expect(mockSetNavigationBlocked).toHaveBeenCalledWith(true); + expect(screen.queryByText(/already exists/i)).not.toBeInTheDocument(); + }); + + it('calls markDirty (navigation-blocked, error cleared) when a resource is removed', async () => { + jest.spyOn(Client4, 'createPropertyField').mockRejectedValue(makeClientError('app.property_field.create.name_conflict.app_error', 'already exists')); + + renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Users', 'user'); + await userEvent.click(screen.getByTestId('saveSetting')); + expect(await screen.findByText(/already exists/i)).toBeInTheDocument(); + + mockSetNavigationBlocked.mockClear(); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-remove')); + + expect(mockSetNavigationBlocked).toHaveBeenCalledWith(true); + expect(screen.queryByText(/already exists/i)).not.toBeInTheDocument(); + }); + + it('moves focus to the header Add-resource trigger after removing a resource', async () => { + renderComponent(); + await addResource('Users', 'user'); + + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-toggle')); + await userEvent.click(screen.getByTestId('attributeAppliesToRow-user-remove')); + + await waitFor(() => expect(screen.getByTestId('attributeAppliesToAddResourceButtonHeader')).toHaveFocus()); + }); + + it('moves focus to the just-added row after adding the 3rd (last) resource, since both triggers unmount in that same render', async () => { + renderComponent(); + await addResource('Users', 'user'); + await addResource('Channels', 'channel'); + await addResource('Posts', 'post'); + + expect(screen.queryByTestId('attributeAppliesToAddResourceButtonHeader')).not.toBeInTheDocument(); + expect(screen.queryByTestId('attributeAppliesToAddResourceButtonInline')).not.toBeInTheDocument(); + await waitFor(() => expect(screen.getByTestId('attributeAppliesToRow-post-toggle')).toHaveFocus()); + }); + + it('does not skip the linked-field loop or rollback when unmounted mid-save, on a failing save', async () => { + let resolveUserCreate: (value: PropertyField) => void = () => {}; + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField').mockResolvedValue({status: 'OK'}); + jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockReturnValueOnce(new Promise((resolve) => { + resolveUserCreate = resolve; + })). + mockRejectedValueOnce(new Error('boom')); + + const {unmount} = renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Users', 'user'); + await addResource('Channels', 'channel'); + await userEvent.click(screen.getByTestId('saveSetting')); + + // Unmount while the first linked-field create is still pending -- + // the rest of the sequence (this create resolving, the second + // linked-field create rejecting, and the full rollback) must + // still run to completion afterward. + unmount(); + resolveUserCreate({id: 'user-field-id'} as PropertyField); + + await waitFor(() => expect(deletePropertyField).toHaveBeenCalledWith('access_control', 'user', 'user-field-id')); + await waitFor(() => expect(deletePropertyField).toHaveBeenCalledWith('access_control', 'template', 'template-id')); + }); + + it('does not navigate or unblock navigation after unmounting mid-save, even when every create ultimately succeeds', async () => { + // Unlike the failing-save test above (whose assertions only prove + // the loop/rollback ran, since a failure outcome would never reach + // navigate/dispatch even without the isMountedRef guard), this + // scenario resolves every create successfully -- making + // mockHistoryPush/mockSetNavigationBlocked(false) genuinely + // load-bearing: without the guard in finalizeSave, both WOULD be + // called once the pending create resolves post-unmount. + let resolveUserCreate: (value: PropertyField) => void = () => {}; + const userCreatePromise = new Promise((resolve) => { + resolveUserCreate = resolve; + }); + jest.spyOn(Client4, 'createPropertyField'). + mockResolvedValueOnce({id: 'template-id'} as PropertyField). + mockReturnValueOnce(userCreatePromise); + + const {unmount} = renderComponent(); + await userEvent.type(screen.getByTestId('attributeDisplayNameInput'), 'My Attribute'); + await addResource('Users', 'user'); + await userEvent.click(screen.getByTestId('saveSetting')); + + unmount(); + resolveUserCreate({id: 'user-field-id'} as PropertyField); + + // Await the hanging create, then one extra tick so handleSave's + // await resumes and finalizeSave runs -- waitFor(() => Promise.resolve()) + // returns on the first check and can pass before the mount guard. + await userCreatePromise; + await Promise.resolve(); + expect(mockHistoryPush).not.toHaveBeenCalled(); + expect(mockSetNavigationBlocked).not.toHaveBeenCalledWith(false); + }); + }); }); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.tsx index ea83b37fbdb9..fc68f7441205 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.tsx +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_details.tsx @@ -27,6 +27,9 @@ import Constants from 'utils/constants'; import {CPA_FIELD_NAME_MAX_RUNES, filterCELIdentifier, slugifyForCEL, validateCPAFieldName} from 'utils/properties'; import type {CPAFieldNameValidationError} from 'utils/properties'; +import AttributeAppliesTo from './attribute_applies_to'; +import {ALL_RESOURCE_TYPES, ATTRIBUTE_APPLIES_TO_ADD_HEADER_TRIGGER_ID, resourceTypeLabels} from './attribute_applies_to_constants'; +import type {ResourceObjectType} from './attribute_applies_to_constants'; import AttributeExternalSource from './attribute_external_source'; import type {ExternalSource} from './attribute_external_source'; import AttributeOptionsRankValues from './attribute_options_rank_values'; @@ -34,7 +37,7 @@ import AttributeOptionsValues from './attribute_options_values'; import {getTypeIcon, getTypeLabel, typeLabels} from '../global_attributes_table'; import type {AttributeFieldType} from '../utils'; -import {createAttributeField} from '../utils'; +import {createAttributeField, createLinkedAttributeField, deleteAttributeField, deleteLinkedAttributeField} from '../utils'; import './attribute_details.scss'; @@ -80,7 +83,17 @@ function computeAutoSlugDisplay(displayName: string): string | null { return slug === '_copy' ? null : slug; } -type ErrorKind = 'name_conflict' | 'invalid_charset' | 'reserved_word' | 'limit_reached' | 'invalid_options' | 'generic'; +type ErrorKind = + | 'name_conflict' | + 'invalid_charset' | + 'reserved_word' | + 'limit_reached' | + 'invalid_options' | + 'generic' | + 'applies_to_failed' | + 'applies_to_rollback_failed' | + 'applies_to_name_conflict' | + 'applies_to_limit_reached'; function errorKindFromError(error: unknown): ErrorKind { const serverErrorId = (error as ClientError | undefined)?.server_error_id; @@ -101,6 +114,103 @@ function errorKindFromError(error: unknown): ErrorKind { } } +// A linked-field creation failure gets its own mapping rather than reusing +// errorKindFromError's cases above: the server error ids are identical to the +// template's own name-conflict/limit-reached ids (both write into the same +// access_control group), but the actionable copy is different -- a linked +// 'user'-object-type field conflicts with a Custom Profile Attributes field, +// not with another Global Attribute (see the plan's CPA-namespace-overlap +// Decision). Returns null for anything else, so the caller falls back to the +// generic applies-to-failed message. +function appliesToErrorKindFromError(error: unknown): 'applies_to_name_conflict' | 'applies_to_limit_reached' | null { + const serverErrorId = (error as ClientError | undefined)?.server_error_id; + switch (serverErrorId) { + case 'app.property_field.create.name_conflict.app_error': + return 'applies_to_name_conflict'; + case 'app.property_field.create.limit_reached.app_error': + case 'app.property_field.create.group_limit_reached.app_error': + return 'applies_to_limit_reached'; + default: + return null; + } +} + +// Formats a resource-type list for interpolation into an error banner, e.g. +// "Users, Channels" -- reuses the same labels the picker and rows already +// show, so the banner names resources the same way the UI does. +function resourceTypeListLabel(types: ResourceObjectType[], formatMessage: IntlShape['formatMessage']): string { + return types.map((type) => formatMessage(resourceTypeLabels[type])).join(', '); +} + +// The settled result of a handleSave attempt -- computed synchronously +// through the create-or-rollback sequence, then applied in one place +// (finalizeSave) behind the single isMountedRef check (see Decisions). +type SaveOutcome = + | {success: true} | + {success: false; errorKind: ErrorKind; serverErrorMessage: string | null; failedResourceTypes: ResourceObjectType[] | null}; + +// Rolls back everything created in a failed handleSave attempt: deletes every +// linked field already created so far (continuing through the full list even +// if one delete fails, so a single stuck field doesn't leave the rest +// orphaned too), then -- only if every one of those deletes succeeded -- +// attempts to delete the template itself (deletion-order protection +// guarantees the template delete would 409 otherwise, so it's skipped rather +// than attempted for a failure this banner can't explain). Returns the +// settled failure outcome for handleSave to hand to finalizeSave. +async function rollbackLinkedFields( + createdLinkedFields: Array<{type: ResourceObjectType; field: PropertyField}>, + templateFieldId: string, + failedType: ResourceObjectType, + creationError: unknown, +): Promise { + const survivingTypes: ResourceObjectType[] = []; + for (const created of createdLinkedFields) { + try { + // eslint-disable-next-line no-await-in-loop + await deleteLinkedAttributeField(created.type, created.field.id); + } catch { + survivingTypes.push(created.type); + } + } + + if (survivingTypes.length > 0) { + return { + success: false, + errorKind: 'applies_to_rollback_failed', + serverErrorMessage: null, + failedResourceTypes: survivingTypes, + }; + } + + try { + await deleteAttributeField(templateFieldId); + } catch (deleteTemplateError) { + // Linked fields are gone, but the template is still on the server. + // A retry under the same Unique name will conflict with it, so this + // is the same class of leftover as a linked-field that wouldn't + // delete -- not "nothing was saved". + // eslint-disable-next-line no-console + console.error('Failed to delete orphaned attribute template after rolling back its linked fields', deleteTemplateError); + return { + success: false, + errorKind: 'applies_to_rollback_failed', + serverErrorMessage: null, + failedResourceTypes: [], + }; + } + + // CPA name-conflict / cap banners only make sense for a Users-linked + // field (that namespace is shared with Custom Profile Attributes). + // Channels/Posts use the generic applies-to-failed copy. + const cpaErrorKind = failedType === 'user' ? appliesToErrorKindFromError(creationError) : null; + return { + success: false, + errorKind: cpaErrorKind ?? 'applies_to_failed', + serverErrorMessage: null, + failedResourceTypes: [failedType], + }; +} + function nameErrorMessage(error: CPAFieldNameValidationError, formatMessage: IntlShape['formatMessage']): string { if (error.kind === 'reserved_word') { return formatMessage(nameErrorMessages.reservedWord, {word: error.word}); @@ -154,9 +264,18 @@ function AttributeDetails({disabled = false}: Props): JSX.Element { const [ldapAttr, setLdapAttr] = useState(''); const [samlAttr, setSamlAttr] = useState(''); + // Pending Applies-to selection -- insertion order, not fixed Users/Channels/ + // Posts order (that fixed order only governs the picker's own offer list). + const [appliesTo, setAppliesTo] = useState([]); + const [saving, setSaving] = useState(false); const [errorKind, setErrorKind] = useState(null); + // Only populated for the applies_to_* error kinds -- interpolated into + // their banners to name which resource(s) failed to create (generic + // failure) or which survived a failed rollback (rollback-failed banner). + const [failedResourceTypes, setFailedResourceTypes] = useState(null); + // Only populated for 'name_conflict' -- the server's own message names the // specific conflicting field and the level it conflicts at (e.g. "system"), // which the canned copy below can't express since it doesn't know that detail. @@ -210,8 +329,55 @@ function AttributeDetails({disabled = false}: Props): JSX.Element { dispatch(setNavigationBlocked(true)); setErrorKind(null); setServerErrorMessage(null); + setFailedResourceTypes(null); }, [dispatch]); + // Defensive dedupe, independent of the picker's own filtering (which + // already only offers not-yet-selected types) -- a genuine no-op if the + // type is somehow already present, so it doesn't mark the page dirty or + // clear an error banner for nothing. + const handleAdd = useCallback((type: ResourceObjectType) => { + if (appliesTo.includes(type)) { + return; + } + setAppliesTo((prev) => [...prev, type]); + markDirty(); + }, [appliesTo, markDirty]); + + const handleRemove = useCallback((type: ResourceObjectType) => { + setAppliesTo((prev) => prev.filter((existing) => existing !== type)); + markDirty(); + }, [markDirty]); + + // Moves focus to the header Add-resource trigger after a pre-save removal + // (see the plan's Decisions table) -- via useEffect, not directly inside + // handleRemove, since the trigger may have just been re-rendered into + // existence this same update (e.g. removing the 3rd of 3 selected types + // un-hides both triggers), and a synchronous focus() call in the handler + // would run before that re-render commits. Mirrors the sibling external- + // source picker's own prevCountRef pattern (attribute_external_source.tsx). + // + // Also handles the mirror-image case on add: picking the 3rd (last) + // resource type unmounts BOTH "Add resource" triggers in this same + // render, including whichever one the admin just clicked -- MUI's + // Popover restores focus to that trigger once its close transition + // finishes, but the trigger is gone from the DOM by then, so focus + // silently drops to with no fix here. Landing on the + // just-added row's own toggle keeps focus on a real, newly-rendered + // element instead. Looked up by data-testid (not id) since the row + // doesn't otherwise need a stable element id. + const prevAppliesToLengthRef = useRef(appliesTo.length); + useEffect(() => { + const prevLength = prevAppliesToLengthRef.current; + if (appliesTo.length < prevLength) { + document.getElementById(ATTRIBUTE_APPLIES_TO_ADD_HEADER_TRIGGER_ID)?.focus(); + } else if (appliesTo.length > prevLength && appliesTo.length === ALL_RESOURCE_TYPES.length) { + const lastAddedType = appliesTo[appliesTo.length - 1]; + document.querySelector(`[data-testid="attributeAppliesToRow-${lastAddedType}-toggle"]`)?.focus(); + } + prevAppliesToLengthRef.current = appliesTo.length; + }, [appliesTo]); + // Switching into Rank from any other type (re)assigns rank = index + 1 to // every current option, overwriting any stale rank values -- mirrors CPA's // own handleTypeChange (user_properties_type_menu.tsx) exactly, and is @@ -274,9 +440,9 @@ function AttributeDetails({disabled = false}: Props): JSX.Element { setIsEditingName(true); }, [autoSlugDisplay, isNameManuallyEdited, manualName]); - // Done (and Enter, which routes here) is inert while the typed Name is - // invalid, so a reserved word or bad charset can never be committed into - // the field -- the admin must fix it first. This is not a focus trap: + // Done, Enter, and blur (clicking away) share this path. Inert while the + // typed Name is invalid, so a reserved word or bad charset can never be + // committed -- the admin must fix it first. This is not a focus trap: // clearing the field makes Done live again (an empty name has no // validation error, and Done then applies the revert rules below), and // Escape still discards the whole edit outright. @@ -320,6 +486,7 @@ function AttributeDetails({disabled = false}: Props): JSX.Element { } }, [handleDoneClick, handleCancelEdit]); + const hasExternalSource = Boolean(ldapAttr || samlAttr); const typeSupportsOptions = supportsOptions({type: fieldType} as PropertyField); // Defensive re-check, not the primary guard: both options editors already @@ -349,33 +516,98 @@ function AttributeDetails({disabled = false}: Props): JSX.Element { const canSave = !disabled && Boolean(displayName.trim()) && Boolean(currentName) && !nameValidationError && !saving && optionsIssue === null; + // Applies the fully-settled outcome of a save attempt -- the ONLY place in + // handleSave that reads isMountedRef, checked once after the entire + // create-or-rollback sequence below has finished (see the plan's Decisions + // table: inserting a mounted-check earlier, e.g. right after the template + // create, would skip the linked-field loop and its rollback entirely if + // the admin navigated away mid-save, leaving an orphaned template with no + // cleanup attempted). + const finalizeSave = useCallback((outcome: SaveOutcome) => { + if (!isMountedRef.current) { + return; + } + if (outcome.success) { + dispatch(setNavigationBlocked(false)); + getHistory().push(LIST_ROUTE); + return; + } + setErrorKind(outcome.errorKind); + setServerErrorMessage(outcome.serverErrorMessage); + setFailedResourceTypes(outcome.failedResourceTypes); + setSaving(false); + }, [dispatch]); + const handleSave = useCallback(async () => { if (!canSave) { return; } setSaving(true); setErrorKind(null); + setServerErrorMessage(null); + setFailedResourceTypes(null); + + let templateField: PropertyField; try { - await createAttributeField(displayName, currentName, fieldType, options, {ldapAttr, samlAttr}); - if (!isMountedRef.current) { - return; - } - dispatch(setNavigationBlocked(false)); - getHistory().push(LIST_ROUTE); + templateField = await createAttributeField(displayName, currentName, fieldType, options, {ldapAttr, samlAttr}); } catch (error) { - if (isMountedRef.current) { - setErrorKind(errorKindFromError(error)); - setServerErrorMessage((error as ClientError | undefined)?.message ?? null); - } - } finally { - if (isMountedRef.current) { - setSaving(false); + finalizeSave({ + success: false, + errorKind: errorKindFromError(error), + serverErrorMessage: (error as ClientError | undefined)?.message ?? null, + failedResourceTypes: null, + }); + return; + } + + // Serial, not Promise.all -- costs nothing at N<=3 calls and is what + // makes "which resource failed" deterministic (see Decisions). + const createdLinkedFields: Array<{type: ResourceObjectType; field: PropertyField}> = []; + let outcome: SaveOutcome = {success: true}; + + for (const type of appliesTo) { + try { + // eslint-disable-next-line no-await-in-loop + const linkedField = await createLinkedAttributeField(type, currentName, fieldType, displayName, templateField.id); + createdLinkedFields.push({type, field: linkedField}); + } catch (error) { + // eslint-disable-next-line no-await-in-loop + outcome = await rollbackLinkedFields(createdLinkedFields, templateField.id, type, error); + break; } } - }, [canSave, displayName, currentName, fieldType, options, ldapAttr, samlAttr, dispatch]); + + finalizeSave(outcome); + }, [canSave, displayName, currentName, fieldType, options, ldapAttr, samlAttr, appliesTo, finalizeSave]); const TypeIcon = getTypeIcon(fieldType); + // The two applies_to_* kinds below that interpolate resource names need + // their own copy path -- they can't go through the flat + // formatMessage(errorMessages[errorKind]) call every other kind uses, + // since that call takes no values. applies_to_name_conflict and + // applies_to_limit_reached use canned copy naming the actual cause + // ("already used by a User Attribute") rather than the server's raw + // message -- unlike the template's own name_conflict case below (which + // reuses the server's message because it already names the specific + // conflicting field/level), the server's generic name-conflict message + // has no notion of "User Attribute" to say, since that framing is + // specific to this feature's CPA-namespace overlap. + let errorContent: React.ReactNode = null; + if (errorKind === 'applies_to_failed') { + errorContent = formatMessage(errorMessages.applies_to_failed, {resources: resourceTypeListLabel(failedResourceTypes ?? [], formatMessage)}); + } else if (errorKind === 'applies_to_rollback_failed') { + const resources = resourceTypeListLabel(failedResourceTypes ?? [], formatMessage); + errorContent = resources ? formatMessage(errorMessages.applies_to_rollback_failed, { + name: displayName, + resources, + }) : formatMessage(errorMessages.applies_to_template_rollback_failed, {name: displayName}); + } else if (errorKind === 'name_conflict' && serverErrorMessage) { + errorContent = serverErrorMessage; + } else if (errorKind) { + errorContent = formatMessage(errorMessages[errorKind]); + } + return (
{ + // Blur runs before click. Without this, Done would + // commit on blur and the same click would re-open Edit. + if (isEditingName) { + e.preventDefault(); + } + }} disabled={saving || disabled} aria-disabled={isDoneBlocked || undefined} aria-describedby={isDoneBlocked ? 'attribute-unique-name-error' : undefined} @@ -530,15 +770,17 @@ function AttributeDetails({disabled = false}: Props): JSX.Element { menuButton={{ id: 'attribute-type-menu-button', class: 'AttributeDetails__typeButton', - disabled: saving || disabled, - 'aria-label': formatMessage(messages.typeFieldAriaLabel, {value: formatMessage(getTypeLabel(fieldType))}), + disabled: saving || disabled || hasExternalSource, + 'aria-label': hasExternalSource ? formatMessage(messages.typeFieldLockedAriaLabel) : formatMessage(messages.typeFieldAriaLabel, {value: formatMessage(getTypeLabel(fieldType))}), children: ( <> - + {!hasExternalSource && ( + + )} ), dataTestId: 'attributeTypeMenuButton', @@ -602,12 +844,14 @@ function AttributeDetails({disabled = false}: Props): JSX.Element { )} ) : ( -

- -

+ !hasExternalSource && ( +

+ +

+ ) )} +
@@ -644,7 +894,7 @@ function AttributeDetails({disabled = false}: Props): JSX.Element { data-testid='attributeSaveError' > - {errorKind === 'name_conflict' && serverErrorMessage ? serverErrorMessage : formatMessage(errorMessages[errorKind])} + {errorContent} )}
@@ -678,6 +928,7 @@ const messages = defineMessages({ typeLabel: {id: 'admin.global_attributes.attribute_details.type.label', defaultMessage: 'Type'}, typeMenuAriaLabel: {id: 'admin.global_attributes.attribute_details.type.menu_label', defaultMessage: 'Select type'}, typeFieldAriaLabel: {id: 'admin.global_attributes.attribute_details.type.field_aria_label', defaultMessage: 'Type: {value}'}, + typeFieldLockedAriaLabel: {id: 'admin.global_attributes.attribute_details.type.field_locked_aria_label', defaultMessage: 'Type: Text. Locked while linked to an external source.'}, optionsLabel: {id: 'admin.global_attributes.attribute_details.options.label', defaultMessage: 'Options'}, optionsHelp: { id: 'admin.global_attributes.attribute_details.options.help', @@ -735,4 +986,24 @@ const errorMessages = defineMessages({ id: 'admin.global_attributes.attribute_details.save_error.generic', defaultMessage: 'Something went wrong while saving this attribute. Please try again.', }, + applies_to_failed: { + id: 'admin.global_attributes.attribute_details.save_error.applies_to_failed', + defaultMessage: "Couldn't apply this attribute to {resources}. Nothing was saved — please try again.", + }, + applies_to_rollback_failed: { + id: 'admin.global_attributes.attribute_details.save_error.applies_to_rollback_failed', + defaultMessage: '"{name}" may have been partially created for {resources}. A retry under the same name will likely fail until those are cleaned up.', + }, + applies_to_template_rollback_failed: { + id: 'admin.global_attributes.attribute_details.save_error.applies_to_template_rollback_failed', + defaultMessage: '"{name}" was created but could not be cleaned up after a failed apply. A retry under the same name will likely fail until it is deleted.', + }, + applies_to_name_conflict: { + id: 'admin.global_attributes.attribute_details.save_error.applies_to_name_conflict', + defaultMessage: 'This name is already used by a User Attribute. Please choose a different name.', + }, + applies_to_limit_reached: { + id: 'admin.global_attributes.attribute_details.save_error.applies_to_limit_reached', + defaultMessage: 'The maximum number of User Attributes has been reached. Delete an existing one before applying this attribute to Users.', + }, }); diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.scss b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.scss index 10415855e9fa..6d06c6d10b2b 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.scss +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.scss @@ -4,18 +4,29 @@ .AttributeExternalSource { display: flex; flex-direction: column; - margin-top: 12px; gap: 8px; } -// The Divider is a section separator between the Options row above and this -// whole external-source block (chips and/or the "add" trigger) -- it's a -// real full-width element (components/divider) rather than a border on the -// trigger button itself, so it always spans the full row width regardless -// of the button's own (inline, content-sized) width, and still shows even -// when only chips are rendered (both sources linked, no trigger). +.AttributeExternalSource__synced { + display: flex; + min-height: 32px; + flex-wrap: wrap; + align-items: center; + gap: 8px; +} + +.AttributeExternalSource__syncedLabel { + color: rgba(var(--center-channel-color-rgb), 0.75); + font-size: 14px; + line-height: 20px; + white-space: nowrap; +} + +// Full-width hairline between the Options help/editor and the "add" trigger. +// Hidden once a source is linked, since the chips then occupy the Options +// line and the trigger sits directly under them. .AttributeExternalSource__divider { - margin-bottom: 12px; + margin-bottom: 4px; } .AttributeExternalSource__chips { diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.test.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.test.tsx index 1a5f25d124d7..fb7b06786957 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.test.tsx +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.test.tsx @@ -34,17 +34,21 @@ describe('AttributeExternalSource', () => { it('renders no chips and offers both sources when neither is linked', async () => { renderComponent(); + expect(screen.queryByTestId('attributeExternalSourceSynced')).not.toBeInTheDocument(); expect(screen.queryByTestId(/attributeExternalSourceChip-/)).not.toBeInTheDocument(); + expect(screen.getByRole('separator')).toBeInTheDocument(); await userEvent.click(screen.getByTestId('attributeExternalSourceTrigger')); expect(screen.getByRole('menuitem', {name: /AD\/LDAP/})).toBeInTheDocument(); expect(screen.getByRole('menuitem', {name: /^SAML/})).toBeInTheDocument(); }); - it('renders a chip for a linked source and offers only the remaining source', async () => { + it('renders a chip for a linked source prefixed by Synced with, and offers only the remaining source', async () => { renderComponent({ldapAttr: 'department'}); + expect(screen.getByTestId('attributeExternalSourceSynced')).toHaveTextContent(/^Synced with/); expect(screen.getByTestId('attributeExternalSourceChip-ldap')).toBeInTheDocument(); + expect(screen.queryByRole('separator')).not.toBeInTheDocument(); await userEvent.click(screen.getByTestId('attributeExternalSourceTrigger')); expect(screen.getByRole('menuitem', {name: /^SAML/})).toBeInTheDocument(); @@ -63,6 +67,7 @@ describe('AttributeExternalSource', () => { expect(screen.getByTestId('attributeExternalSourceChip-ldap')).toBeInTheDocument(); expect(screen.getByTestId('attributeExternalSourceChip-saml')).toBeInTheDocument(); expect(screen.queryByTestId('attributeExternalSourceTrigger')).not.toBeInTheDocument(); + expect(screen.queryByRole('separator')).not.toBeInTheDocument(); }); it('opens the modal pre-filled and empty when adding a new link, with no type-mismatch warning on a Text field', async () => { diff --git a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.tsx b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.tsx index 159745958049..3db10c1fabbd 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.tsx +++ b/webapp/channels/src/components/admin_console/global_attributes/attribute_details/attribute_external_source.tsx @@ -130,7 +130,31 @@ function AttributeExternalSource({ldapAttr, samlAttr, fieldType, onLink, disable className='AttributeExternalSource' data-testid='attributeExternalSource' > - + {linkedSources.length === 0 && ( + + )} + {linkedSources.length > 0 && ( +
+ + + +
+ {linkedSources.map((source) => ( + openLinkModal(source)} + onRemove={() => onLink(source, '')} + disabled={disabled} + /> + ))} +
+
+ )} {unlinkedSources.length > 0 && ( )} - {linkedSources.length > 0 && ( -
- {linkedSources.map((source) => ( - openLinkModal(source)} - onRemove={() => onLink(source, '')} - disabled={disabled} - /> - ))} -
- )} { describe('createAttributeField', () => { @@ -108,6 +108,20 @@ describe('global_attributes/utils', () => { await expect(createAttributeField('Name', 'name', 'text', [])).rejects.toThrow('boom'); }); + it('trims the display name and omits it when blank, same as createAttributeField', async () => { + const createPropertyField = jest.spyOn(Client4, 'createPropertyField').mockResolvedValue({} as PropertyField); + + await createLinkedAttributeField('user', 'my_attribute', 'text', ' My Attribute ', 'template-id'); + expect(createPropertyField).toHaveBeenCalledWith('access_control', 'user', expect.objectContaining({ + attrs: {display_name: 'My Attribute'}, + })); + + await createLinkedAttributeField('user', 'my_attribute', 'text', ' ', 'template-id'); + expect(createPropertyField).toHaveBeenCalledWith('access_control', 'user', expect.objectContaining({ + attrs: {display_name: undefined}, + })); + }); + it('omits ldap/saml entirely when the links parameter is not passed', async () => { const createPropertyField = jest.spyOn(Client4, 'createPropertyField').mockResolvedValue({} as PropertyField); @@ -166,4 +180,67 @@ describe('global_attributes/utils', () => { })); }); }); + + describe('deleteAttributeField', () => { + beforeEach(() => { + jest.restoreAllMocks(); + }); + + it('calls Client4.deletePropertyField against the template object type', async () => { + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField').mockResolvedValue({status: 'OK'}); + + await deleteAttributeField('field-id'); + + expect(deletePropertyField).toHaveBeenCalledWith('access_control', 'template', 'field-id'); + }); + }); + + describe('createLinkedAttributeField', () => { + beforeEach(() => { + jest.restoreAllMocks(); + }); + + it('calls Client4.createPropertyField against the given resource object type with linked_field_id set', async () => { + const createPropertyField = jest.spyOn(Client4, 'createPropertyField').mockResolvedValue({} as PropertyField); + + await createLinkedAttributeField('channel', 'my_attribute', 'text', 'My Attribute', 'template-id'); + + expect(createPropertyField).toHaveBeenCalledWith('access_control', 'channel', { + name: 'my_attribute', + type: 'text', + target_type: 'system', + target_id: '', + linked_field_id: 'template-id', + attrs: {display_name: 'My Attribute'}, + }); + }); + + it.each((['user', 'channel', 'post'] as const))('sends %s as the object_type path segment', async (objectType) => { + const createPropertyField = jest.spyOn(Client4, 'createPropertyField').mockResolvedValue({} as PropertyField); + + await createLinkedAttributeField(objectType, 'my_attribute', 'text', 'My Attribute', 'template-id'); + + expect(createPropertyField).toHaveBeenCalledWith('access_control', objectType, expect.anything()); + }); + + it('propagates a rejection from Client4', async () => { + jest.spyOn(Client4, 'createPropertyField').mockRejectedValue(new Error('boom')); + + await expect(createLinkedAttributeField('user', 'name', 'text', 'Name', 'template-id')).rejects.toThrow('boom'); + }); + }); + + describe('deleteLinkedAttributeField', () => { + beforeEach(() => { + jest.restoreAllMocks(); + }); + + it('calls Client4.deletePropertyField against the given resource object type', async () => { + const deletePropertyField = jest.spyOn(Client4, 'deletePropertyField').mockResolvedValue({status: 'OK'}); + + await deleteLinkedAttributeField('post', 'field-id'); + + expect(deletePropertyField).toHaveBeenCalledWith('access_control', 'post', 'field-id'); + }); + }); }); diff --git a/webapp/channels/src/components/admin_console/global_attributes/utils.ts b/webapp/channels/src/components/admin_console/global_attributes/utils.ts index 4877445a851b..d3f976dd7360 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/utils.ts +++ b/webapp/channels/src/components/admin_console/global_attributes/utils.ts @@ -5,6 +5,7 @@ import type {PropertyField, PropertyFieldOption} from '@mattermost/types/propert import {Client4} from 'mattermost-redux/client'; +import type {ResourceObjectType} from './attribute_details/attribute_applies_to_constants'; import {GLOBAL_ATTRIBUTES_GROUP_NAME, GLOBAL_ATTRIBUTES_OBJECT_TYPE, GLOBAL_ATTRIBUTES_TARGET_TYPE} from './constants'; export type AttributeFieldType = 'text' | 'select' | 'multiselect' | 'rank'; @@ -60,7 +61,41 @@ export function createAttributeField( // Deletes a template field from the access_control group. The server returns // 409 when the field still has active linked dependents (CountLinkedFields > 0); -// callers are expected to surface that case distinctly. +// callers are expected to surface that case distinctly (or, for a save-time +// rollback, to only delete linked fields first -- see createLinkedAttributeField). export function deleteAttributeField(fieldId: string): Promise { return Client4.deletePropertyField(GLOBAL_ATTRIBUTES_GROUP_NAME, GLOBAL_ATTRIBUTES_OBJECT_TYPE, fieldId); } + +// Creates a linked field for one Applies-to resource. The server validates +// linked_field_id against the template and copies its Type and attrs.options +// onto the new field (server/channels/app/properties/property_field.go) -- +// display_name is NOT copied, so it's sent explicitly here (see the plan's +// Decisions table). objectType is the resource type ('user'/'channel'/'post'), +// a URL path segment on the generic property-fields endpoint, not a separate +// route. +export function createLinkedAttributeField( + objectType: ResourceObjectType, + name: string, + fieldType: AttributeFieldType, + displayName: string, + linkedFieldId: string, +): Promise { + return Client4.createPropertyField(GLOBAL_ATTRIBUTES_GROUP_NAME, objectType, { + name, + type: fieldType as PropertyField['type'], + target_type: GLOBAL_ATTRIBUTES_TARGET_TYPE, + target_id: '', + linked_field_id: linkedFieldId, + attrs: { + display_name: displayName.trim() || undefined, + }, + }); +} + +// Deletes a linked field for one Applies-to resource. Must be called before +// deleteAttributeField on the template it points at -- the server blocks +// deleting a template with active linked dependents. +export function deleteLinkedAttributeField(objectType: ResourceObjectType, fieldId: string): Promise { + return Client4.deletePropertyField(GLOBAL_ATTRIBUTES_GROUP_NAME, objectType, fieldId); +} diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 5521b2ac0e47..d34b82d60fc1 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1428,6 +1428,19 @@ "admin.gitlab.siteUrlExample": "E.g.: https://", "admin.gitlab.tokenTitle": "Token Endpoint:", "admin.gitlab.userTitle": "User API Endpoint:", + "admin.global_attributes.attribute_details.applies_to.add_resource_header": "Add resource", + "admin.global_attributes.attribute_details.applies_to.add_resource_inline": "Add another resource", + "admin.global_attributes.attribute_details.applies_to.empty_state.heading": "No resources yet", + "admin.global_attributes.attribute_details.applies_to.empty_state.helper_text": "Add a resource to apply this attribute to users, channels, or posts.", + "admin.global_attributes.attribute_details.applies_to.item.body_placeholder": "No additional settings for this resource yet.", + "admin.global_attributes.attribute_details.applies_to.item.collapse": "Collapse {label}", + "admin.global_attributes.attribute_details.applies_to.item.expand": "Expand {label}", + "admin.global_attributes.attribute_details.applies_to.item.remove": "Remove resource", + "admin.global_attributes.attribute_details.applies_to.resource_type.channel": "Channels", + "admin.global_attributes.attribute_details.applies_to.resource_type.post": "Posts", + "admin.global_attributes.attribute_details.applies_to.resource_type.user": "Users", + "admin.global_attributes.attribute_details.applies_to.subtitle": "Resources this attribute applies to, and who can set the value on each.", + "admin.global_attributes.attribute_details.applies_to.title": "Applies to", "admin.global_attributes.attribute_details.back_link": "Back to Manage Attributes", "admin.global_attributes.attribute_details.cancel": "Cancel", "admin.global_attributes.attribute_details.definition.subtitle": "Display name, type, and options.", @@ -1446,6 +1459,7 @@ "admin.global_attributes.attribute_details.external_source.saml.modal_title": "Link to SAML", "admin.global_attributes.attribute_details.external_source.saml.subtitle": "Map values from SAML at sign-in", "admin.global_attributes.attribute_details.external_source.saml.title": "SAML", + "admin.global_attributes.attribute_details.external_source.synced_with": "Synced with", "admin.global_attributes.attribute_details.external_source.trigger_label": "Link to external source", "admin.global_attributes.attribute_details.name_error.invalid_charset": "Name must start with a letter or underscore, and contain only letters, numbers, and underscores.", "admin.global_attributes.attribute_details.name_error.reserved_word": "\"{word}\" is a reserved word and cannot be used as a name.", @@ -1464,6 +1478,11 @@ "admin.global_attributes.attribute_details.options.required": "At least one option is required.", "admin.global_attributes.attribute_details.options.values_unique": "Values must be unique.", "admin.global_attributes.attribute_details.save": "Save", + "admin.global_attributes.attribute_details.save_error.applies_to_failed": "Couldn't apply this attribute to {resources}. Nothing was saved — please try again.", + "admin.global_attributes.attribute_details.save_error.applies_to_limit_reached": "The maximum number of User Attributes has been reached. Delete an existing one before applying this attribute to Users.", + "admin.global_attributes.attribute_details.save_error.applies_to_name_conflict": "This name is already used by a User Attribute. Please choose a different name.", + "admin.global_attributes.attribute_details.save_error.applies_to_rollback_failed": "\"{name}\" may have been partially created for {resources}. A retry under the same name will likely fail until those are cleaned up.", + "admin.global_attributes.attribute_details.save_error.applies_to_template_rollback_failed": "\"{name}\" was created but could not be cleaned up after a failed apply. A retry under the same name will likely fail until it is deleted.", "admin.global_attributes.attribute_details.save_error.generic": "Something went wrong while saving this attribute. Please try again.", "admin.global_attributes.attribute_details.save_error.invalid_charset": "Name must start with a letter or underscore, and contain only letters, numbers, and underscores.", "admin.global_attributes.attribute_details.save_error.invalid_options": "There's a problem with one or more options — check for a duplicate or overly long name, or a missing/duplicate rank, then try again.", @@ -1473,6 +1492,7 @@ "admin.global_attributes.attribute_details.subtitle": "Add a display name, choose a type, and pick where it applies.", "admin.global_attributes.attribute_details.title": "New attribute", "admin.global_attributes.attribute_details.type.field_aria_label": "Type: {value}", + "admin.global_attributes.attribute_details.type.field_locked_aria_label": "Type: Text. Locked while linked to an external source.", "admin.global_attributes.attribute_details.type.label": "Type", "admin.global_attributes.attribute_details.type.menu_label": "Select type", "admin.global_attributes.attribute_details.unique_name.could_not_generate": "Couldn't generate a unique name from this display name. Click Edit to set one manually.", From 3e8afaa06400887dfbeb8ce77186bcb0ed72c9b9 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 20 Aug 2026 18:01:44 -0400 Subject: [PATCH 3/4] MM-69232 Enable concurrent React in E2E tests (#37037) * MM-69323 Enable Concurrent React * Fix Cypress tests involving post dot menu * Fix flaky Cypress tests involving Team Settings modal * Fix setState in permissions code when called twice in rapid succession by tests * Update fullname_spec.js to wait for suggestion list to close This is needed because the setState in SuggestionBox.clear isn't processed until after the Enter keypress registers. Alternatively, we could wrap that in flushSync, but since this seems to only occur during testing, I decided not to change the web app code. * Update more E2E tests * Fix accidental commented test code * Switch useContainerDimensions to useLayoutEffect to fix newly introduced layout shift * Skip post_height SVG test on all browsers and message attachment test on Firefox * Bump changes to feature flag for a future PR * Enable concurrent React in E2E tests --- e2e-tests/.ci/server.generate.sh | 1 + .../auth_sso/authentication_4_spec.ts | 8 +- ...ve_and_archive_channel_destructive_spec.ts | 1 + .../settings/sidebar/fullname_spec.js | 4 + .../channels/team_settings/teams_spec.js | 21 +++-- .../cypress/tests/support/ui_commands.ts | 14 ++- .../lib/src/server/default_config.ts | 1 + .../channels/post_list/post_height.spec.ts | 7 +- .../permission_system_scheme_settings.tsx | 62 ++++++------- .../permission_team_scheme_settings.tsx | 70 +++++++-------- .../channel/details/channel_details.tsx | 86 ++++++++++--------- .../media_gallery/use_container_dimensions.ts | 4 +- .../src/components/post/post_options.tsx | 4 +- 13 files changed, 161 insertions(+), 122 deletions(-) diff --git a/e2e-tests/.ci/server.generate.sh b/e2e-tests/.ci/server.generate.sh index 537616e2d04e..8ec5d0c774f2 100755 --- a/e2e-tests/.ci/server.generate.sh +++ b/e2e-tests/.ci/server.generate.sh @@ -73,6 +73,7 @@ services: MM_FEATUREFLAGS_ATTRIBUTEVALUEMASKING: "true" MM_FEATUREFLAGS_WYSIWYGEDITOR: "true" MM_FEATUREFLAGS_RECURRINGSCHEDULEDPOSTS: "true" + MM_FEATUREFLAGS_ENABLECONCURRENTREACT: "true" MM_LOGSETTINGS_ENABLEDIAGNOSTICS: "false" MM_LOGSETTINGS_CONSOLELEVEL: "DEBUG" network_mode: host diff --git a/e2e-tests/cypress/tests/integration/channels/auth_sso/authentication_4_spec.ts b/e2e-tests/cypress/tests/integration/channels/auth_sso/authentication_4_spec.ts index fd1694f84287..7a3d1fbddcf2 100644 --- a/e2e-tests/cypress/tests/integration/channels/auth_sso/authentication_4_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/auth_sso/authentication_4_spec.ts @@ -276,10 +276,14 @@ describe('Authentication', () => { cy.findByText('Copy invite link').click(); // # Input email, select member - cy.findByLabelText('Invite People').type(`test-${getRandomId()}@mattermost.com{downarrow}{downarrow}{enter}`, {force: true}); + cy.findByLabelText('Invite People').type(`test-${getRandomId()}@mattermost.com`); + + // # Wait a moment for the autocomplete and then press enter to select the email + cy.wait(100); + cy.findByLabelText('Invite People').type('{enter}'); // # Click invite members button - cy.findByRole('button', {name: 'Invite'}).click({force: true}); + cy.findByRole('button', {name: 'Invite'}).click(); // * Verify message is what you expect it to be cy.contains('The following email addresses do not belong to an accepted domain:', {timeout: TIMEOUTS.ONE_MIN}).should('be.visible').and('exist'); diff --git a/e2e-tests/cypress/tests/integration/channels/channel/leave_and_archive_channel_destructive_spec.ts b/e2e-tests/cypress/tests/integration/channels/channel/leave_and_archive_channel_destructive_spec.ts index b3e334607d38..9d7ca4e64238 100644 --- a/e2e-tests/cypress/tests/integration/channels/channel/leave_and_archive_channel_destructive_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/channel/leave_and_archive_channel_destructive_spec.ts @@ -47,6 +47,7 @@ describe('Leave and Archive channel actions display as destructive', () => { // * Move to... close menu option cy.findByText('Move to...').should('be.visible').trigger('mouseout'); + cy.findByRole('menuitem', {name: 'Move to...'}).should('have.attr', 'aria-expanded', 'false'); // * Notification Preferences menu option should be visible cy.get('#channelNotificationPreferences').should('be.visible'); diff --git a/e2e-tests/cypress/tests/integration/channels/settings/sidebar/fullname_spec.js b/e2e-tests/cypress/tests/integration/channels/settings/sidebar/fullname_spec.js index c03670affff0..3886ed36729c 100644 --- a/e2e-tests/cypress/tests/integration/channels/settings/sidebar/fullname_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/settings/sidebar/fullname_spec.js @@ -64,6 +64,10 @@ describe('Settings > Sidebar > General', () => { // * Verify that after enter user's username match cy.uiGetPostTextBox().should('have.value', `@${username} `); + // * Wait for the autocomplete list to close so that pressing enter posts the + // message instead of being captured to complete a suggestion + cy.get('#suggestionList').should('not.exist'); + // # Click enter in post textbox cy.uiGetPostTextBox().type('{enter}'); diff --git a/e2e-tests/cypress/tests/integration/channels/team_settings/teams_spec.js b/e2e-tests/cypress/tests/integration/channels/team_settings/teams_spec.js index 38746ed2ad3e..f37465566b11 100644 --- a/e2e-tests/cypress/tests/integration/channels/team_settings/teams_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/team_settings/teams_spec.js @@ -220,7 +220,7 @@ describe('Teams Suite', () => { cy.get('#teamName').should('be.visible').clear().type(teamName); // Save new team name annd close< - cy.uiSaveAndClose(); + saveAndCloseTeamSettings(); // Team display name shows as "Testing Team" at top of team menu cy.uiGetLHSHeader().findByText(teamName); @@ -245,8 +245,8 @@ describe('Teams Suite', () => { cy.get('#teamDescription').should('be.visible').clear().type(teamDescription); cy.get('#teamDescription').should('have.value', teamDescription); - // Save and close - cy.uiSaveAndClose(); + // # Save and close + saveAndCloseTeamSettings(); cy.wait(TIMEOUTS.ONE_HUNDRED_MILLIS); @@ -271,7 +271,7 @@ describe('Teams Suite', () => { cy.get('#public-private-selector-button-O').should('exist').and('not.have.class', 'selected').click(); // # Save and close - cy.uiSaveAndClose(); + saveAndCloseTeamSettings(); // # Login as new user cy.apiLogin(newUser); @@ -307,7 +307,7 @@ describe('Teams Suite', () => { // * Verify Private Team card is selected (open joining disabled by default) cy.get('#public-private-selector-button-P').should('exist').and('have.class', 'selected'); - // # Save and close + // # Close the modal cy.uiClose(); // # Login as new user @@ -337,6 +337,17 @@ describe('Teams Suite', () => { }); }); +function saveAndCloseTeamSettings() { + // # Save the changes + cy.uiSave(); + + // * Wait for changes to be saved so the modal can be closed + cy.get('.SaveChangesPanel').should('contain', 'Settings saved'); + + // # Close the modal + cy.uiClose(); +} + function removeTeamMember(teamName, username) { cy.apiAdminLogin(); cy.visit(`/${teamName}`); diff --git a/e2e-tests/cypress/tests/support/ui_commands.ts b/e2e-tests/cypress/tests/support/ui_commands.ts index 570e2d099ad5..f441c7a509fb 100644 --- a/e2e-tests/cypress/tests/support/ui_commands.ts +++ b/e2e-tests/cypress/tests/support/ui_commands.ts @@ -323,13 +323,19 @@ function clickPostHeaderItem(postId: string, location: string, item: string) { idPrefix = 'post'; } + const hoverPostAndClickItem = (id: string) => { + // # Hover over the post and then wait for the hovered class to apply to ensure the header items are visible + cy.get(`#${idPrefix}_${id}`).trigger('mouseover', {force: true}).should('have.class', 'post--hovered'); + + // # Ensure the header item is visible then click on it + cy.get(`#${location}_${item}_${id}`).scrollIntoView().trigger('mouseover', {force: true}).click({force: true}); + }; + if (postId) { - cy.get(`#${idPrefix}_${postId}`).trigger('mouseover', {force: true}). - get(`#${location}_${item}_${postId}`).scrollIntoView().trigger('mouseover', {force: true}).click({force: true}); + hoverPostAndClickItem(postId); } else { cy.getLastPostId().then((lastPostId) => { - cy.get(`#${idPrefix}_${lastPostId}`).trigger('mouseover', {force: true}). - get(`#${location}_${item}_${lastPostId}`).scrollIntoView().trigger('mouseover', {force: true}).click({force: true}); + hoverPostAndClickItem(lastPostId); }); } } diff --git a/e2e-tests/playwright/lib/src/server/default_config.ts b/e2e-tests/playwright/lib/src/server/default_config.ts index 57920db1ff6e..3a0be9b8e531 100644 --- a/e2e-tests/playwright/lib/src/server/default_config.ts +++ b/e2e-tests/playwright/lib/src/server/default_config.ts @@ -816,6 +816,7 @@ const defaultServerConfig: AdminConfig = { PropertyFieldRank: false, TeamMembershipAccessControl: true, MmBlocksEnabled: true, + EnableConcurrentReact: true, }, ImportSettings: { Directory: './import', diff --git a/e2e-tests/playwright/specs/functional/channels/post_list/post_height.spec.ts b/e2e-tests/playwright/specs/functional/channels/post_list/post_height.spec.ts index d4e534a14e68..55f83f6da82a 100644 --- a/e2e-tests/playwright/specs/functional/channels/post_list/post_height.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/post_list/post_height.spec.ts @@ -206,6 +206,9 @@ test.describe('Post height', () => { }, { name: 'post with a message attachment', + // For some reason, the font size of the attachment title changes slightly on Firefox with MM Blocks + // and concurrent React enabled at the same time. + skipProjects: ['firefox'], makePost: () => seedPost({ message: 'post with a message attachment', @@ -298,8 +301,8 @@ test.describe('Post height', () => { }, { name: 'post with an SVG Markdown image', - // Either Chrome preloads the SVG's dimensions early or Firefox doesn't allocate the height properly - skipProjects: ['firefox'], + // As of MM-67372, the server no longer provides dimensions for external SVGs + skipProjects: ['chrome', 'firefox', 'ipad'], makePost: ({fileServerUrl}) => seedPost({ message: `![icon](${fileServerUrl}/icon.svg)`, diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.tsx index 2555cdd39d63..49f19105aa7d 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.tsx @@ -325,41 +325,43 @@ export class PermissionSystemSchemeSettings extends React.PureComponent) => { - const roles = {...this.state.roles}; - const role = {...roles[roleId as keyof RolesState]} as Role; - const newPermissions = [...role.permissions!]; - for (const permission of permissions) { - if (newPermissions.indexOf(permission) === -1) { - newPermissions.push(permission); - } else { - newPermissions.splice(newPermissions.indexOf(permission), 1); - } - } - role.permissions = newPermissions; - roles[roleId as keyof RolesState] = role; - - if (roleId === 'all_users') { - const channelAdminRole = {...roles.channel_admin} as Role; - const channelAdminPermissions = [...channelAdminRole.permissions!]; - const teamAdminRole = {...roles.team_admin} as Role; - const teamAdminPermissions = [...teamAdminRole.permissions!]; + this.setState((state) => { + const roles = {...state.roles}; + const role = {...roles[roleId as keyof RolesState]} as Role; + const newPermissions = [...role.permissions!]; for (const permission of permissions) { - if (ModeratedPermissions.indexOf(permission) !== -1 && role.permissions.indexOf(permission) !== -1) { - if (channelAdminPermissions.indexOf(permission) === -1) { - channelAdminPermissions.push(permission); - } - if (teamAdminPermissions.indexOf(permission) === -1) { - teamAdminPermissions.push(permission); + if (newPermissions.indexOf(permission) === -1) { + newPermissions.push(permission); + } else { + newPermissions.splice(newPermissions.indexOf(permission), 1); + } + } + role.permissions = newPermissions; + roles[roleId as keyof RolesState] = role; + + if (roleId === 'all_users') { + const channelAdminRole = {...roles.channel_admin} as Role; + const channelAdminPermissions = [...channelAdminRole.permissions!]; + const teamAdminRole = {...roles.team_admin} as Role; + const teamAdminPermissions = [...teamAdminRole.permissions!]; + for (const permission of permissions) { + if (ModeratedPermissions.indexOf(permission) !== -1 && role.permissions.indexOf(permission) !== -1) { + if (channelAdminPermissions.indexOf(permission) === -1) { + channelAdminPermissions.push(permission); + } + if (teamAdminPermissions.indexOf(permission) === -1) { + teamAdminPermissions.push(permission); + } } } + channelAdminRole.permissions = channelAdminPermissions; + roles.channel_admin = channelAdminRole; + teamAdminRole.permissions = teamAdminPermissions; + roles.team_admin = teamAdminRole; } - channelAdminRole.permissions = channelAdminPermissions; - roles.channel_admin = channelAdminRole; - teamAdminRole.permissions = teamAdminPermissions; - roles.team_admin = teamAdminRole; - } - this.setState({roles, saveNeeded: true}); + return {roles, saveNeeded: true}; + }); this.props.actions.setNavigationBlocked(true); }; diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.tsx index 3722581b2949..135d11a9d651 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.tsx @@ -502,48 +502,50 @@ export default class PermissionTeamSchemeSettings extends React.PureComponent { - const roles = {...this.getStateRoles()} as RolesMap; - const rolesKey = Object.keys(roles).find((roleKey) => roles[roleKey].name === roleId); + this.setState((state) => { + const roles = {...(state.roles ?? this.getStateRoles())} as RolesMap; + const rolesKey = Object.keys(roles).find((roleKey) => roles[roleKey].name === roleId); - if (!rolesKey) { - return; - } + if (!rolesKey) { + return null; + } - const role = {...roles[rolesKey]} as Role; + const role = {...roles[rolesKey]} as Role; - const newPermissions = [...role.permissions]; - for (const permission of permissions) { - if (newPermissions.indexOf(permission) === -1) { - newPermissions.push(permission); - } else { - newPermissions.splice(newPermissions.indexOf(permission), 1); - } - } - role.permissions = newPermissions; - roles[rolesKey] = role; - - if (roleId === 'all_users') { - const channelAdminRole = {...roles.channel_admin} as Role; - const channelAdminPermissions = [...channelAdminRole.permissions!]; - const teamAdminRole = {...roles.team_admin} as Role; - const teamAdminPermissions = [...teamAdminRole.permissions!]; + const newPermissions = [...role.permissions]; for (const permission of permissions) { - if (ModeratedPermissions.indexOf(permission) !== -1 && role.permissions.indexOf(permission) !== -1) { - if (channelAdminPermissions.indexOf(permission) === -1) { - channelAdminPermissions.push(permission); - } - if (teamAdminPermissions.indexOf(permission) === -1) { - teamAdminPermissions.push(permission); + if (newPermissions.indexOf(permission) === -1) { + newPermissions.push(permission); + } else { + newPermissions.splice(newPermissions.indexOf(permission), 1); + } + } + role.permissions = newPermissions; + roles[rolesKey] = role; + + if (roleId === 'all_users') { + const channelAdminRole = {...roles.channel_admin} as Role; + const channelAdminPermissions = [...channelAdminRole.permissions!]; + const teamAdminRole = {...roles.team_admin} as Role; + const teamAdminPermissions = [...teamAdminRole.permissions!]; + for (const permission of permissions) { + if (ModeratedPermissions.indexOf(permission) !== -1 && role.permissions.indexOf(permission) !== -1) { + if (channelAdminPermissions.indexOf(permission) === -1) { + channelAdminPermissions.push(permission); + } + if (teamAdminPermissions.indexOf(permission) === -1) { + teamAdminPermissions.push(permission); + } } } + channelAdminRole.permissions = channelAdminPermissions; + roles.channel_admin = channelAdminRole; + teamAdminRole.permissions = teamAdminPermissions; + roles.team_admin = teamAdminRole; } - channelAdminRole.permissions = channelAdminPermissions; - roles.channel_admin = channelAdminRole; - teamAdminRole.permissions = teamAdminPermissions; - roles.team_admin = teamAdminRole; - } - this.setState({roles, saveNeeded: true}); + return {roles, saveNeeded: true}; + }); this.props.actions.setNavigationBlocked(true); }; diff --git a/webapp/channels/src/components/admin_console/team_channel_settings/channel/details/channel_details.tsx b/webapp/channels/src/components/admin_console/team_channel_settings/channel/details/channel_details.tsx index 150a7142d232..589d818b623e 100644 --- a/webapp/channels/src/components/admin_console/team_channel_settings/channel/details/channel_details.tsx +++ b/webapp/channels/src/components/admin_console/team_channel_settings/channel/details/channel_details.tsx @@ -366,51 +366,53 @@ export default class ChannelDetails extends React.PureComponent { - const currentValueIndex = this.state.channelPermissions.findIndex((element) => element.name === name); - const currentValue = this.state.channelPermissions[currentValueIndex].roles[channelRole]!.value; - const newValue = !currentValue; - let channelPermissions = [...this.state.channelPermissions]; - - if (name === Permissions.CHANNEL_MODERATED_PERMISSIONS.CREATE_POST) { - const originalObj = this.props.channelPermissions.find((element) => element.name === Permissions.CHANNEL_MODERATED_PERMISSIONS.USE_CHANNEL_MENTIONS)?.roles![channelRole]; - channelPermissions = channelPermissions.map((permission) => { - if (permission.name === Permissions.CHANNEL_MODERATED_PERMISSIONS.USE_CHANNEL_MENTIONS && !newValue) { - return { - name: permission.name, - roles: { - ...permission.roles, - [channelRole]: { - value: false, - enabled: false, + this.setState((state) => { + const currentValueIndex = state.channelPermissions.findIndex((element) => element.name === name); + const currentValue = state.channelPermissions[currentValueIndex].roles[channelRole]!.value; + const newValue = !currentValue; + let channelPermissions = [...state.channelPermissions]; + + if (name === Permissions.CHANNEL_MODERATED_PERMISSIONS.CREATE_POST) { + const originalObj = this.props.channelPermissions.find((element) => element.name === Permissions.CHANNEL_MODERATED_PERMISSIONS.USE_CHANNEL_MENTIONS)?.roles![channelRole]; + channelPermissions = channelPermissions.map((permission) => { + if (permission.name === Permissions.CHANNEL_MODERATED_PERMISSIONS.USE_CHANNEL_MENTIONS && !newValue) { + return { + name: permission.name, + roles: { + ...permission.roles, + [channelRole]: { + value: false, + enabled: false, + }, }, - }, - }; - } else if (permission.name === Permissions.CHANNEL_MODERATED_PERMISSIONS.USE_CHANNEL_MENTIONS) { - return { - name: permission.name, - roles: { - ...permission.roles, - [channelRole]: { - value: originalObj?.value, - enabled: originalObj?.enabled, + }; + } else if (permission.name === Permissions.CHANNEL_MODERATED_PERMISSIONS.USE_CHANNEL_MENTIONS) { + return { + name: permission.name, + roles: { + ...permission.roles, + [channelRole]: { + value: originalObj?.value, + enabled: originalObj?.enabled, + }, }, - }, - }; - } - return permission; - }); - } - channelPermissions[currentValueIndex] = { - ...channelPermissions[currentValueIndex], - roles: { - ...channelPermissions[currentValueIndex].roles, - [channelRole]: { - ...channelPermissions[currentValueIndex].roles[channelRole], - value: newValue, + }; + } + return permission; + }); + } + channelPermissions[currentValueIndex] = { + ...channelPermissions[currentValueIndex], + roles: { + ...channelPermissions[currentValueIndex].roles, + [channelRole]: { + ...channelPermissions[currentValueIndex].roles[channelRole], + value: newValue, + }, }, - }, - }; - this.setState({channelPermissions, saveNeeded: true}); + }; + return {channelPermissions, saveNeeded: true}; + }); this.props.actions.setNavigationBlocked(true); }; diff --git a/webapp/channels/src/components/file_attachment_list/media_gallery/use_container_dimensions.ts b/webapp/channels/src/components/file_attachment_list/media_gallery/use_container_dimensions.ts index aba4dd2350f3..ebc49fddd2ab 100644 --- a/webapp/channels/src/components/file_attachment_list/media_gallery/use_container_dimensions.ts +++ b/webapp/channels/src/components/file_attachment_list/media_gallery/use_container_dimensions.ts @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {useEffect, useState} from 'react'; +import {useLayoutEffect, useState} from 'react'; import type {RefObject} from 'react'; export type ContainerDimensions = { @@ -12,7 +12,7 @@ export type ContainerDimensions = { export function useContainerDimensions(ref: RefObject): ContainerDimensions { const [dimensions, setDimensions] = useState({width: 0, height: 0}); - useEffect(() => { + useLayoutEffect(() => { const node = ref.current; if (!node) { return undefined; diff --git a/webapp/channels/src/components/post/post_options.tsx b/webapp/channels/src/components/post/post_options.tsx index 663ddc27d095..92e98a4904ad 100644 --- a/webapp/channels/src/components/post/post_options.tsx +++ b/webapp/channels/src/components/post/post_options.tsx @@ -236,7 +236,9 @@ const PostOptions = (props: Props): JSX.Element => { } const dotMenu = ( -
  • + + // Use a key to keep the DotMenu mounted even if it changes position below +
  • Date: Fri, 21 Aug 2026 00:18:06 +0200 Subject: [PATCH 4/4] MM-70307: Change Postgres test password to mostest_password (#38060) * Change test password to mostest_password This makes the password compliant with the 112 bits minimum length requirement. Otherwise, FIPS-compliant OpenSSL implementations will panic when trying to connect from `lib/pq` with a shorter password. * Simplify test templates' POSTGRES_PASSWORD values * make generated * Modify missing "mostest" strings --- .github/workflows/mmctl-test-template.yml | 8 ++++---- .github/workflows/server-ci-nightly-race.yml | 2 +- .github/workflows/server-ci-weekly.yml | 6 +++--- .github/workflows/server-ci.yml | 16 ++++++++-------- .github/workflows/server-test-template.yml | 8 ++++---- .../develop/contribute/developer-setup/docker.md | 4 ++-- docs/develop/contribute/developer-setup/index.md | 2 +- .../contribute/more-info/webapp/e2e-testing.md | 2 +- .../configure/configuration-in-your-database.mdx | 4 ++-- .../manage/command-line-tools.mdx | 2 +- .../manage/mmctl-command-line-tool.mdx | 2 +- .../server/prepare-mattermost-mysql-database.mdx | 2 +- e2e-tests/.ci/dashboard.override.yml | 2 +- e2e-tests/.ci/server.generate.sh | 6 +++--- e2e-tests/.ci/server.prepare.sh | 8 ++++---- e2e-tests/.ci/server.start.sh | 2 +- e2e-tests/cypress/cypress.config.ts | 4 ++-- .../tests/support/api/cloud_default_config.json | 2 +- .../tests/support/api/keycloak_realm.json | 2 +- .../support/api/on_prem_default_config.json | 4 ++-- .../cypress/tests/support/keycloak_commands.ts | 2 +- .../tests/support/ldap_server_commands.ts | 4 ++-- .../playwright/lib/src/containers/constants.ts | 4 ++-- .../playwright/lib/src/server/default_config.ts | 2 +- e2e-tests/playwright/lib/src/test_config.ts | 2 +- server/Makefile | 8 ++++---- server/build/docker-compose.common.yml | 4 ++-- server/build/docker-preview/Dockerfile | 2 +- server/build/docker-preview/config_docker.json | 2 +- .../build/docker/keycloak/ldap.mmsettings.json | 2 +- server/build/dotenv/test.env | 2 +- server/build/local-test-env.sh | 14 +++++++------- server/channels/api4/apitestlib.go | 2 +- server/channels/store/sqlstore/store_test.go | 6 +++--- server/channels/store/storetest/settings.go | 2 +- server/cmd/mattermost/commands/db_ping.go | 2 +- server/cmd/mmctl/commands/config.go | 2 +- server/cmd/mmctl/commands/ldap_e2e_test.go | 2 +- server/cmd/mmctl/docs/mmctl_config_migrate.rst | 2 +- server/config/database_test.go | 2 +- server/docker-compose.yaml | 6 +++--- server/public/model/config.go | 2 +- server/public/model/config_test.go | 8 ++++---- server/scripts/psql-migration-test.sh | 4 ++-- server/tests/test-config.json | 2 +- tools/sharedchannel-test/server.go | 8 ++++---- .../.github/workflows/performance-benchmarks.yml | 4 ++-- .../admin_console/database_settings.test.tsx | 2 +- 48 files changed, 96 insertions(+), 96 deletions(-) diff --git a/.github/workflows/mmctl-test-template.yml b/.github/workflows/mmctl-test-template.yml index ffdddc482d81..3805a4cd2ae2 100644 --- a/.github/workflows/mmctl-test-template.yml +++ b/.github/workflows/mmctl-test-template.yml @@ -81,13 +81,13 @@ jobs: echo "$INPUT_PR_NUMBER" > server/pr-number - name: Run docker compose env: - POSTGRES_PASSWORD: ${{ inputs.fips-enabled && 'mostest-fips-test' || 'mostest' }} + POSTGRES_PASSWORD: 'mostest_password' run: | cd server/build docker compose --ansi never run --rm start_dependencies - cat ../tests/custom-schema-objectID.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest || true'; - cat ../tests/custom-schema-cpa.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest || true'; - cat ../tests/test-data.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest'; + cat ../tests/custom-schema-objectID.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest_password || true'; + cat ../tests/custom-schema-cpa.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest_password || true'; + cat ../tests/test-data.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password'; docker compose --ansi never exec -T minio sh -c 'mkdir -p /data/mattermost-test'; docker compose --ansi never ps diff --git a/.github/workflows/server-ci-nightly-race.yml b/.github/workflows/server-ci-nightly-race.yml index d21a978961c4..b0b78dab5a73 100644 --- a/.github/workflows/server-ci-nightly-race.yml +++ b/.github/workflows/server-ci-nightly-race.yml @@ -45,7 +45,7 @@ jobs: uses: ./.github/workflows/server-test-template.yml with: name: Race Detector - datasource: postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres logsartifact: race-detector-server-test-logs go-version: ${{ needs.buildenv.outputs.version }} diff --git a/.github/workflows/server-ci-weekly.yml b/.github/workflows/server-ci-weekly.yml index e7bb3dfe4dac..c20d70f650b3 100644 --- a/.github/workflows/server-ci-weekly.yml +++ b/.github/workflows/server-ci-weekly.yml @@ -46,7 +46,7 @@ jobs: uses: ./.github/workflows/server-test-template.yml with: name: Postgres with binary parameters - datasource: postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes drivername: postgres logsartifact: postgres-binary-server-test-logs go-version: ${{ needs.buildenv.outputs.version }} @@ -68,7 +68,7 @@ jobs: DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }} with: name: Postgres FIPS - datasource: postgres://mmuser:mostest-fips-test@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres logsartifact: postgres-server-fips-test-logs go-version: ${{ needs.buildenv.outputs.version }} @@ -89,7 +89,7 @@ jobs: MM_E2E_ZEPHYR_API_KEY: ${{ secrets.MM_E2E_ZEPHYR_API_KEY }} with: name: mmctl - datasource: postgres://mmuser:mostest-fips-test@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres logsartifact: mmctl-fips-test-logs go-version: ${{ needs.buildenv.outputs.version }} diff --git a/.github/workflows/server-ci.yml b/.github/workflows/server-ci.yml index 0b1ca9b112dd..a320129a80b4 100644 --- a/.github/workflows/server-ci.yml +++ b/.github/workflows/server-ci.yml @@ -85,7 +85,7 @@ jobs: image: postgres:15 env: POSTGRES_USER: mmuser - POSTGRES_PASSWORD: mostest + POSTGRES_PASSWORD: mostest_password POSTGRES_DB: mattermost_test ports: - 5432:5432 @@ -103,7 +103,7 @@ jobs: with: run: | export IS_CI=true - export TEST_DATABASE_POSTGRESQL_DSN="postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" + export TEST_DATABASE_POSTGRESQL_DSN="postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" make generated - name: Check generated files run: | @@ -179,7 +179,7 @@ jobs: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: name: "Postgres (shard ${{ matrix.shard }})" - datasource: postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres # Each shard gets a unique artifact name so they don't collide logsartifact: "postgres-server-test-logs-shard-${{ matrix.shard }}" @@ -213,7 +213,7 @@ jobs: uses: ./.github/workflows/server-test-template.yml with: name: Elasticsearch v8 Compatibility - datasource: postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres logsartifact: elasticsearch-v8-server-test-logs go-version: ${{ needs.go.outputs.version }} @@ -231,7 +231,7 @@ jobs: uses: ./.github/workflows/server-test-template.yml with: name: OpenSearch v2 Compatibility - datasource: postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres logsartifact: opensearch-v2-server-test-logs go-version: ${{ needs.go.outputs.version }} @@ -260,7 +260,7 @@ jobs: DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }} with: name: "Postgres FIPS (shard ${{ matrix.shard }})" - datasource: postgres://mmuser:mostest-fips-test@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres logsartifact: "postgres-server-fips-test-logs-shard-${{ matrix.shard }}" go-version: ${{ needs.go.outputs.version }} @@ -291,7 +291,7 @@ jobs: MM_E2E_ZEPHYR_API_KEY: ${{ secrets.MM_E2E_ZEPHYR_API_KEY }} with: name: mmctl - datasource: postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres logsartifact: mmctl-test-logs go-version: ${{ needs.go.outputs.version }} @@ -312,7 +312,7 @@ jobs: MM_E2E_ZEPHYR_API_KEY: ${{ secrets.MM_E2E_ZEPHYR_API_KEY }} with: name: mmctl - datasource: postgres://mmuser:mostest-fips-test@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + datasource: postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 drivername: postgres logsartifact: mmctl-fips-test-logs go-version: ${{ needs.go.outputs.version }} diff --git a/.github/workflows/server-test-template.yml b/.github/workflows/server-test-template.yml index 6bb4569c47a7..7e6e7787fcca 100644 --- a/.github/workflows/server-test-template.yml +++ b/.github/workflows/server-test-template.yml @@ -155,13 +155,13 @@ jobs: env: ELASTICSEARCH_VERSION: ${{ inputs.elasticsearch-version }} OPENSEARCH_VERSION: ${{ inputs.opensearch-version }} - POSTGRES_PASSWORD: ${{ inputs.fips-enabled && 'mostest-fips-test' || 'mostest' }} + POSTGRES_PASSWORD: 'mostest_password' run: | cd server/build docker compose --ansi never run --rm start_dependencies - cat ../tests/custom-schema-objectID.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest || true'; - cat ../tests/custom-schema-cpa.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest || true'; - cat ../tests/test-data.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest'; + cat ../tests/custom-schema-objectID.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest_password || true'; + cat ../tests/custom-schema-cpa.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest_password || true'; + cat ../tests/test-data.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password'; docker compose --ansi never exec -T minio sh -c 'mkdir -p /data/mattermost-test'; docker compose --ansi never ps diff --git a/docs/develop/contribute/developer-setup/docker.md b/docs/develop/contribute/developer-setup/docker.md index 6cdd8e8e4c73..6ee08d6cacfb 100644 --- a/docs/develop/contribute/developer-setup/docker.md +++ b/docs/develop/contribute/developer-setup/docker.md @@ -27,7 +27,7 @@ This is the default and recommended database to use with Mattermost. No addition ``` MM_SQLSETTINGS_DRIVERNAME=postgres -MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable\u0026connect_timeout=10 +MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable\u0026connect_timeout=10 ``` ## mysql @@ -41,7 +41,7 @@ This is an alternate database supported by Mattermost, but not recommended for n To use with Mattermost, be sure to configure the following settings: ``` MM_SQLSETTINGS_DRIVERNAME=mysql -MM_SQLSETTINGS_DATASOURCE=mmuser:mostest@tcp(localhost:3306)/mattermost_test?charset=utf8mb4,utf8\u0026readTimeout=30s\u0026writeTimeout=30s +MM_SQLSETTINGS_DATASOURCE=mmuser:mostest_password@tcp(localhost:3306)/mattermost_test?charset=utf8mb4,utf8\u0026readTimeout=30s\u0026writeTimeout=30s ``` ## inbucket diff --git a/docs/develop/contribute/developer-setup/index.md b/docs/develop/contribute/developer-setup/index.md index 74f2b3135dcb..4bfa0c68368b 100644 --- a/docs/develop/contribute/developer-setup/index.md +++ b/docs/develop/contribute/developer-setup/index.md @@ -141,7 +141,7 @@ The `make package` command will package the application and place it under the ` ``` 1. Copy the file `server/config.mk` as `server/config.override.mk` and set `MM_NO_DOCKER` to `true` in the copy. 1. Install [PostgreSQL](https://www.postgresql.org/download/) -1. Run `psql postgres`. Then create `mmuser` by running `CREATE ROLE mmuser WITH LOGIN PASSWORD 'mostest';` +1. Run `psql postgres`. Then create `mmuser` by running `CREATE ROLE mmuser WITH LOGIN PASSWORD 'mostest_password';` 1. Modify the role to give rights to create a database by running `ALTER ROLE mmuser CREATEDB;` 1. Confirm the role rights by running `\du` 1. Before creating the database, exit by running `\q` diff --git a/docs/develop/contribute/more-info/webapp/e2e-testing.md b/docs/develop/contribute/more-info/webapp/e2e-testing.md index 9d3366846a46..44cefa841a01 100644 --- a/docs/develop/contribute/more-info/webapp/e2e-testing.md +++ b/docs/develop/contribute/more-info/webapp/e2e-testing.md @@ -247,7 +247,7 @@ Environment variables are [defined in cypress.config.ts](https://github.com/matt | CYPRESS\_adminUsername | Admin's username for the test server.

    *Default*: `sysadmin` when server is seeded by `make test-data`. | | CYPRESS\_adminPassword | Admin's password for the test server.

    *Default*: `Sys@dmin-sample1` when server is seeded by `make test-data`. | | CYPRESS\_dbClient | The database of the test server. It should match the server config `SqlSettings.DriverName`.

    *Default*: `postgres`
    *Valid values*: `postgres` or `mysql` | -| CYPRESS\_dbConnection | The database connection string of the test server. It should match the server config `SqlSettings.DataSource`.

    *Default*: `"postgres://mmuser:mostest@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10"` | +| CYPRESS\_dbConnection | The database connection string of the test server. It should match the server config `SqlSettings.DataSource`.

    *Default*: `"postgres://mmuser:mostest_password@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10"` | | CYPRESS\_enableVisualTest | Use for visual regression testing.

    *Default*: `false`
    *Valid values*: `true` or `false` | | CYPRESS\_ldapServer | Host of the Lightweight Directory Access Protocol (LDAP) server.

    *Default*: `localhost` | | CYPRESS\_ldapPort | Port of the LDAP server.

    *Default*: `389` | diff --git a/docs/main/administration-guide/configure/configuration-in-your-database.mdx b/docs/main/administration-guide/configure/configuration-in-your-database.mdx index 1959c034d40a..0f064f94bfcc 100644 --- a/docs/main/administration-guide/configure/configuration-in-your-database.mdx +++ b/docs/main/administration-guide/configure/configuration-in-your-database.mdx @@ -46,7 +46,7 @@ The first step is to get your master database connection string. We recommend ac Create the file `/opt/mattermost/config/mattermost.environment` to set the `MM_CONFIG` environment variable to the database connection string. For example: ``` text -MM_CONFIG='postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10' +MM_CONFIG='postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10' ``` Run this command to verify the permissions on your Mattermost directory: @@ -72,7 +72,7 @@ sudo systemctl restart mattermost You can use the [mmctl config migrate](/administration-guide/manage/mmctl-command-line-tool#mmctl-config-migrate) command to migrate the configuration by running the following command: ``` sh -./bin/mmctl config migrate path/to/config.json "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" --local +./bin/mmctl config migrate path/to/config.json "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" --local ``` diff --git a/docs/main/administration-guide/manage/command-line-tools.mdx b/docs/main/administration-guide/manage/command-line-tools.mdx index 1dc0b5142730..0663f4c2081e 100644 --- a/docs/main/administration-guide/manage/command-line-tools.mdx +++ b/docs/main/administration-guide/manage/command-line-tools.mdx @@ -56,7 +56,7 @@ sudo -u mattermost bin/mattermost version - When running CLI commands on a Mattermost installation that has the configuration stored in the database, you might need to pass the database connection string as: ``` sh - bin/mattermost --config="postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable\u0026connect_timeout=10" + bin/mattermost --config="postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable\u0026connect_timeout=10" ``` diff --git a/docs/main/administration-guide/manage/mmctl-command-line-tool.mdx b/docs/main/administration-guide/manage/mmctl-command-line-tool.mdx index 9d1d2723f9c8..f75d429304cf 100644 --- a/docs/main/administration-guide/manage/mmctl-command-line-tool.mdx +++ b/docs/main/administration-guide/manage/mmctl-command-line-tool.mdx @@ -2533,7 +2533,7 @@ mmctl config migrate [from_config] [to_config] [flags] **Examples** ``` sh -mmctl config migrate path/to/config.json "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" --local +mmctl config migrate path/to/config.json "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" --local ``` **Options** diff --git a/docs/main/deployment-guide/server/prepare-mattermost-mysql-database.mdx b/docs/main/deployment-guide/server/prepare-mattermost-mysql-database.mdx index 774daac15886..b5e1cbd644d7 100644 --- a/docs/main/deployment-guide/server/prepare-mattermost-mysql-database.mdx +++ b/docs/main/deployment-guide/server/prepare-mattermost-mysql-database.mdx @@ -178,7 +178,7 @@ If you're running Mattermost in a High Availability cluster-based deployment, th Create the file `/opt/mattermost/config/mattermost.environment` to set the `MM_CONFIG` environment variable to the database connection string. For example: ``` text -MM_CONFIG='mysql://mmuser:mostest@tcp(127.0.0.1:3306)/mattermost?charset=utf8mb4,utf8&writeTimeout=30s' +MM_CONFIG='mysql://mmuser:mostest_password@tcp(127.0.0.1:3306)/mattermost?charset=utf8mb4,utf8&writeTimeout=30s' ``` diff --git a/e2e-tests/.ci/dashboard.override.yml b/e2e-tests/.ci/dashboard.override.yml index 2ae00bb08457..577795abc89d 100644 --- a/e2e-tests/.ci/dashboard.override.yml +++ b/e2e-tests/.ci/dashboard.override.yml @@ -3,7 +3,7 @@ services: dashboard: image: mattermostdevelopment/mirrored-node:18.17 environment: - PG_URI: postgres://mmuser:mostest@db:5432/automation_dashboard_db + PG_URI: postgres://mmuser:mostest_password@db:5432/automation_dashboard_db JWT_SECRET: s8gGBA3ujKRohSw1L8HLOY7Jjnu2ZYv8 # Generated with e.g. `dd if=/dev/urandom count=24 bs=1 2>/dev/null | base64 -w0` JWT_USER: cypress-test JWT_ROLE: integration diff --git a/e2e-tests/.ci/server.generate.sh b/e2e-tests/.ci/server.generate.sh index 8ec5d0c774f2..3fc7a52bf0ac 100755 --- a/e2e-tests/.ci/server.generate.sh +++ b/e2e-tests/.ci/server.generate.sh @@ -59,7 +59,7 @@ services: MM_CONNECTEDWORKSPACESSETTINGS_ENABLEREMOTECLUSTERSERVICE: "true" MM_CONNECTEDWORKSPACESSETTINGS_ENABLESHAREDWORKSPACES: "true" MM_FEATUREFLAGS_ENABLEREMOTECLUSTERSERVICE: "true" - MM_SQLSETTINGS_DATASOURCE: "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes" + MM_SQLSETTINGS_DATASOURCE: "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes" MM_SQLSETTINGS_DRIVERNAME: "postgres" MM_EMAILSETTINGS_SMTPSERVER: "localhost" MM_CLUSTERSETTINGS_READONLYCONFIG: "false" @@ -241,7 +241,7 @@ $(if mme2e_is_token_in_list "cypress" "$ENABLED_DOCKER_SERVICES"; then - "../../e2e-tests/.ci/.env.cypress" environment: CYPRESS_baseUrl: "http://localhost:8065" - CYPRESS_dbConnection: "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" + CYPRESS_dbConnection: "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" CYPRESS_smtpUrl: "http://localhost:9001" CYPRESS_webhookBaseUrl: "http://localhost:3000" CYPRESS_chromeWebSecurity: "false" @@ -319,7 +319,7 @@ $(if mme2e_is_token_in_list "playwright" "$ENABLED_DOCKER_SERVICES"; then PW_ADMIN_PASSWORD: Sys@dmin-sample1 PW_ADMIN_EMAIL: sysadmin@sample.mattermost.com PW_ENSURE_PLUGINS_INSTALLED: "" - MM_TEST_DB_URL: "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes" + MM_TEST_DB_URL: "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes" PW_HA_CLUSTER_ENABLED: "false" PW_RESET_BEFORE_TEST: "false" PW_HEADLESS: "true" diff --git a/e2e-tests/.ci/server.prepare.sh b/e2e-tests/.ci/server.prepare.sh index 863eee7f1132..632217213e4b 100755 --- a/e2e-tests/.ci/server.prepare.sh +++ b/e2e-tests/.ci/server.prepare.sh @@ -55,14 +55,14 @@ for SERVICE in $ENABLED_DOCKER_SERVICES; do openldap) LDIF_FILE=../../server/tests/test-data.ldif LDIF_CANARY=$(sed -n -E 's/^dn:[[:space:]]*(.*)$/\1/p' ${LDIF_FILE} | tail -n1) - if ${MME2E_DC_SERVER} exec -T -- openldap bash -c "ldapsearch -x -D \"cn=admin,dc=mm,dc=test,dc=com\" -w mostest -b \"$LDIF_CANARY\" >/dev/null"; then + if ${MME2E_DC_SERVER} exec -T -- openldap bash -c "ldapsearch -x -D \"cn=admin,dc=mm,dc=test,dc=com\" -w mostest_password -b \"$LDIF_CANARY\" >/dev/null"; then mme2e_log "Skipping configuration for the $SERVICE container: already initialized" continue fi mme2e_log "Configuring the $SERVICE container" - ${MME2E_DC_SERVER} exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest || true' <../../server/tests/custom-schema-objectID.ldif - ${MME2E_DC_SERVER} exec -T -- openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest || true' <../../server/tests/custom-schema-cpa.ldif - ${MME2E_DC_SERVER} exec -T -- openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest' <../../server/tests/test-data.ldif + ${MME2E_DC_SERVER} exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest_password || true' <../../server/tests/custom-schema-objectID.ldif + ${MME2E_DC_SERVER} exec -T -- openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest_password || true' <../../server/tests/custom-schema-cpa.ldif + ${MME2E_DC_SERVER} exec -T -- openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password' <../../server/tests/test-data.ldif ;; minio) mme2e_log "Configuring the $SERVICE container" diff --git a/e2e-tests/.ci/server.start.sh b/e2e-tests/.ci/server.start.sh index 1c0689cbc3ab..7713b6d5cffe 100755 --- a/e2e-tests/.ci/server.start.sh +++ b/e2e-tests/.ci/server.start.sh @@ -90,7 +90,7 @@ fi # shellcheck disable=SC2043 for MIGRATION in migration_advanced_permissions_phase_2; do # Query explanation: if it doesn't find the migration in the table, there are 0 results and the command fails with a divide-by-zero error. Otherwise the command succeeds - MIGRATION_CHECK_COMMAND="${MME2E_DC_SERVER} exec -T postgres sh -c 'PGPASSWORD=mostest psql -U mmuser mattermost_test -c \"select 1 / (select count(*) from Systems where name = '\''${MIGRATION}'\'' and value = '\''true'\'');\"'" + MIGRATION_CHECK_COMMAND="${MME2E_DC_SERVER} exec -T postgres sh -c 'PGPASSWORD=mostest_password psql -U mmuser mattermost_test -c \"select 1 / (select count(*) from Systems where name = '\''${MIGRATION}'\'' and value = '\''true'\'');\"'" if ! mme2e_wait_command_success "$MIGRATION_CHECK_COMMAND" "Waiting for migration to be completed: ${MIGRATION}" "10" "10"; then mme2e_log "Migration ${MIGRATION} not completed, retry attempts exhausted. Giving up." >&2 diff --git a/e2e-tests/cypress/cypress.config.ts b/e2e-tests/cypress/cypress.config.ts index cca142005845..2acc5e58f0f2 100644 --- a/e2e-tests/cypress/cypress.config.ts +++ b/e2e-tests/cypress/cypress.config.ts @@ -34,13 +34,13 @@ export default defineConfig({ cwsURL: 'http://localhost:8076', cwsAPIURL: 'http://localhost:8076', dbClient: 'postgres', - dbConnection: 'postgres://mmuser:mostest@localhost/mattermost_test?sslmode=disable&connect_timeout=10', + dbConnection: 'postgres://mmuser:mostest_password@localhost/mattermost_test?sslmode=disable&connect_timeout=10', elasticsearchConnectionURL: 'http://localhost:9200', firstTest: false, keycloakAppName: 'mattermost', keycloakBaseUrl: 'http://localhost:8484', keycloakUsername: 'mmuser', - keycloakPassword: 'mostest', + keycloakPassword: 'mostest_password', ldapServer: 'localhost', ldapPort: 389, minioAccessKey: 'minioaccesskey', diff --git a/e2e-tests/cypress/tests/support/api/cloud_default_config.json b/e2e-tests/cypress/tests/support/api/cloud_default_config.json index 99de903bee63..c3232d671986 100644 --- a/e2e-tests/cypress/tests/support/api/cloud_default_config.json +++ b/e2e-tests/cypress/tests/support/api/cloud_default_config.json @@ -267,7 +267,7 @@ "ConnectionSecurity": "", "BaseDN": "dc=mm,dc=test,dc=com", "BindUsername": "cn=admin,dc=mm,dc=test,dc=com", - "BindPassword": "mostest", + "BindPassword": "mostest_password", "UserFilter": "", "GroupFilter": "", "GuestFilter": "", diff --git a/e2e-tests/cypress/tests/support/api/keycloak_realm.json b/e2e-tests/cypress/tests/support/api/keycloak_realm.json index a0149da196a3..0d5f4dfe3072 100644 --- a/e2e-tests/cypress/tests/support/api/keycloak_realm.json +++ b/e2e-tests/cypress/tests/support/api/keycloak_realm.json @@ -1393,7 +1393,7 @@ "bindDn" : [ "cn=admin,dc=mm,dc=test,dc=com" ], "changedSyncPeriod" : [ "-1" ], "usernameLDAPAttribute" : [ "uid" ], - "bindCredential" : [ "mostest" ], + "bindCredential" : [ "mostest_password" ], "lastSync" : [ "1518169262" ], "vendor" : [ "other" ], "uuidLDAPAttribute" : [ "entryUUID" ], diff --git a/e2e-tests/cypress/tests/support/api/on_prem_default_config.json b/e2e-tests/cypress/tests/support/api/on_prem_default_config.json index 27bd956d616a..4a16df0ad944 100644 --- a/e2e-tests/cypress/tests/support/api/on_prem_default_config.json +++ b/e2e-tests/cypress/tests/support/api/on_prem_default_config.json @@ -144,7 +144,7 @@ }, "SqlSettings": { "DriverName": "postgres", - "DataSource": "postgres://mmuser:mostest@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10\u0026binary_parameters=yes", + "DataSource": "postgres://mmuser:mostest_password@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10\u0026binary_parameters=yes", "DataSourceReplicas": [], "DataSourceSearchReplicas": [], "ConnMaxLifetimeMilliseconds": 3600000, @@ -358,7 +358,7 @@ "ConnectionSecurity": "", "BaseDN": "dc=mm,dc=test,dc=com", "BindUsername": "cn=admin,dc=mm,dc=test,dc=com", - "BindPassword": "mostest", + "BindPassword": "mostest_password", "UserFilter": "", "GroupFilter": "", "GuestFilter": "", diff --git a/e2e-tests/cypress/tests/support/keycloak_commands.ts b/e2e-tests/cypress/tests/support/keycloak_commands.ts index 91869ac78f0b..6e4d37c6bc6f 100644 --- a/e2e-tests/cypress/tests/support/keycloak_commands.ts +++ b/e2e-tests/cypress/tests/support/keycloak_commands.ts @@ -36,7 +36,7 @@ function keycloakGetAccessTokenAPI(): ChainableT { path: '', method: 'post', headers: {'Content-type': 'application/x-www-form-urlencoded'}, - data: 'grant_type=password&username=mmuser&password=mostest&client_id=admin-cli', + data: 'grant_type=password&username=mmuser&password=mostest_password&client_id=admin-cli', // cy.task() returns untyped data }).then((response: any) => { expect(response.status).to.equal(200); diff --git a/e2e-tests/cypress/tests/support/ldap_server_commands.ts b/e2e-tests/cypress/tests/support/ldap_server_commands.ts index b0da7bb0937c..b19517052a1f 100644 --- a/e2e-tests/cypress/tests/support/ldap_server_commands.ts +++ b/e2e-tests/cypress/tests/support/ldap_server_commands.ts @@ -19,7 +19,7 @@ export interface LdapUser { } function modifyLDAPUsers(filename: string) { - cy.exec(`ldapmodify -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest -H ldap://${Cypress.expose('ldapServer')}:${Cypress.expose('ldapPort')} -f tests/fixtures/${filename} -c`, {failOnNonZeroExit: false}); + cy.exec(`ldapmodify -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password -H ldap://${Cypress.expose('ldapServer')}:${Cypress.expose('ldapPort')} -f tests/fixtures/${filename} -c`, {failOnNonZeroExit: false}); } Cypress.Commands.add('modifyLDAPUsers', modifyLDAPUsers); @@ -88,7 +88,7 @@ Cypress.Commands.add('ldapModify', ldapModify); function getLDAPCredentials() { const host = `ldap://${Cypress.expose('ldapServer')}:${Cypress.expose('ldapPort')}`; const bindDn = 'cn=admin,dc=mm,dc=test,dc=com'; - const password = 'mostest'; + const password = 'mostest_password'; return {host, bindDn, password}; } diff --git a/e2e-tests/playwright/lib/src/containers/constants.ts b/e2e-tests/playwright/lib/src/containers/constants.ts index 873bbe98ecea..9c696a78f00e 100644 --- a/e2e-tests/playwright/lib/src/containers/constants.ts +++ b/e2e-tests/playwright/lib/src/containers/constants.ts @@ -10,7 +10,7 @@ export const POSTGRES_ALIAS = 'postgres'; export const POSTGRES_PORT = 5432; export const POSTGRES_DB = 'mattermost_test'; export const POSTGRES_USER = 'mmuser'; -export const POSTGRES_PASSWORD = 'mostest'; +export const POSTGRES_PASSWORD = 'mostest_password'; export const INBUCKET_ALIAS = 'inbucket'; export const INBUCKET_WEB_PORT = 9001; @@ -28,7 +28,7 @@ export const WEBHOOK_PORT = 3000; export const OPENLDAP_ALIAS = 'openldap'; export const OPENLDAP_PORT = 389; export const OPENLDAP_ADMIN_DN = 'cn=admin,dc=mm,dc=test,dc=com'; -export const OPENLDAP_ADMIN_PASSWORD = 'mostest'; +export const OPENLDAP_ADMIN_PASSWORD = 'mostest_password'; export const OPENLDAP_BASE_DN = 'dc=mm,dc=test,dc=com'; export const KEYCLOAK_ALIAS = 'keycloak'; diff --git a/e2e-tests/playwright/lib/src/server/default_config.ts b/e2e-tests/playwright/lib/src/server/default_config.ts index 3a0be9b8e531..8febb0f8cfdc 100644 --- a/e2e-tests/playwright/lib/src/server/default_config.ts +++ b/e2e-tests/playwright/lib/src/server/default_config.ts @@ -265,7 +265,7 @@ const defaultServerConfig: AdminConfig = { SqlSettings: { DriverName: 'postgres', DataSource: - 'postgres://mmuser:mostest@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10\u0026binary_parameters=yes', + 'postgres://mmuser:mostest_password@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10\u0026binary_parameters=yes', DataSourceReplicas: [], DataSourceSearchReplicas: [], MaxIdleConns: 50, diff --git a/e2e-tests/playwright/lib/src/test_config.ts b/e2e-tests/playwright/lib/src/test_config.ts index 821018a451ce..7e66629d5fb2 100644 --- a/e2e-tests/playwright/lib/src/test_config.ts +++ b/e2e-tests/playwright/lib/src/test_config.ts @@ -153,7 +153,7 @@ export class TestConfig { this.smtpURL = process.env.PW_SMTP_URL || 'http://localhost:9001'; this.postgresUrl = process.env.PW_POSTGRES_URL || - 'postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes'; + 'postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes'; this.webhookBaseUrl = process.env.PW_WEBHOOK_BASE_URL || 'http://localhost:3000'; // Testcontainers diff --git a/server/Makefile b/server/Makefile index 1b27a330e0dc..70290d0a8a6e 100644 --- a/server/Makefile +++ b/server/Makefile @@ -259,9 +259,9 @@ endif docker compose rm start_dependencies $(GO) run ./build/docker-compose-generator/main.go $(ENABLED_DOCKER_SERVICES) | docker compose -f docker-compose.makefile.yml -f /dev/stdin $(DOCKER_COMPOSE_OVERRIDE) run -T --rm start_dependencies ifneq (,$(findstring openldap,$(ENABLED_DOCKER_SERVICES))) - cat tests/custom-schema-objectID.ldif | docker compose -f docker-compose.makefile.yml $(DOCKER_COMPOSE_OVERRIDE) exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest || true'; - cat tests/custom-schema-cpa.ldif | docker compose -f docker-compose.makefile.yml ${DOCKER_COMPOSE_OVERRIDE} exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest || true'; - cat tests/${LDAP_DATA}-data.ldif | docker compose -f docker-compose.makefile.yml ${DOCKER_COMPOSE_OVERRIDE} exec -T openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest || true'; + cat tests/custom-schema-objectID.ldif | docker compose -f docker-compose.makefile.yml $(DOCKER_COMPOSE_OVERRIDE) exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest_password || true'; + cat tests/custom-schema-cpa.ldif | docker compose -f docker-compose.makefile.yml ${DOCKER_COMPOSE_OVERRIDE} exec -T openldap bash -c 'ldapadd -Y EXTERNAL -H ldapi:/// -w mostest_password || true'; + cat tests/${LDAP_DATA}-data.ldif | docker compose -f docker-compose.makefile.yml ${DOCKER_COMPOSE_OVERRIDE} exec -T openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password || true'; endif endif @@ -740,7 +740,7 @@ run-node: export MM_SERVICESETTINGS_LISTENADDRESS=:8066 run-node: export MM_SERVICESETTINGS_ENABLELOCALMODE=true run-node: export MM_SERVICESETTINGS_LOCALMODESOCKETLOCATION=/var/tmp/mattermost_local_node.socket run-node: export MM_SQLSETTINGS_DRIVERNAME=postgres -run-node: export MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest@localhost/mattermost_node_test?sslmode=disable&sslmode=disable&connect_timeout=10&binary_parameters=yes +run-node: export MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest_password@localhost/mattermost_node_test?sslmode=disable&sslmode=disable&connect_timeout=10&binary_parameters=yes run-node: setup-go-work start-docker ## Runs a shared channel node. @echo Running mattermost node diff --git a/server/build/docker-compose.common.yml b/server/build/docker-compose.common.yml index 475464c30e07..c187f28c9ef1 100644 --- a/server/build/docker-compose.common.yml +++ b/server/build/docker-compose.common.yml @@ -12,7 +12,7 @@ services: - mm-test environment: POSTGRES_USER: mmuser - POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-mostest} + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-mostest_password} POSTGRES_DB: mattermost_test POSTGRES_INITDB_ARGS: "--auth-host=scram-sha-256 --auth-local=scram-sha-256" command: postgres -c 'config_file=/etc/postgresql/postgresql.conf' @@ -60,7 +60,7 @@ services: LDAP_TLS_VERIFY_CLIENT: "never" LDAP_ORGANISATION: "Mattermost Test" LDAP_DOMAIN: "mm.test.com" - LDAP_ADMIN_PASSWORD: "mostest" + LDAP_ADMIN_PASSWORD: "mostest_password" elasticsearch: build: context: . diff --git a/server/build/docker-preview/Dockerfile b/server/build/docker-preview/Dockerfile index 1b2f307cdc80..97c37a543a44 100644 --- a/server/build/docker-preview/Dockerfile +++ b/server/build/docker-preview/Dockerfile @@ -10,7 +10,7 @@ ARG MATTERMOST_VERSION # ENV POSTGRES_USER=mmuser -ENV POSTGRES_PASSWORD=mostest +ENV POSTGRES_PASSWORD=mostest_password ENV POSTGRES_DB=mattermost_test # diff --git a/server/build/docker-preview/config_docker.json b/server/build/docker-preview/config_docker.json index 3c7aa4b2d07d..5b95be53fcd1 100644 --- a/server/build/docker-preview/config_docker.json +++ b/server/build/docker-preview/config_docker.json @@ -6,7 +6,7 @@ }, "SqlSettings": { "DriverName": "postgres", - "DataSource": "postgres://mmuser:mostest@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10\u0026binary_parameters=yes", + "DataSource": "postgres://mmuser:mostest_password@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10\u0026binary_parameters=yes", "AtRestEncryptKey": "" }, "FileSettings": { diff --git a/server/build/docker/keycloak/ldap.mmsettings.json b/server/build/docker/keycloak/ldap.mmsettings.json index b635749de531..ba9e15b3c963 100644 --- a/server/build/docker/keycloak/ldap.mmsettings.json +++ b/server/build/docker/keycloak/ldap.mmsettings.json @@ -7,7 +7,7 @@ "ConnectionSecurity": "", "BaseDN": "dc=mm,dc=test,dc=com", "BindUsername": "cn=admin,dc=mm,dc=test,dc=com", - "BindPassword": "mostest", + "BindPassword": "mostest_password", "UserFilter": "", "GroupFilter": "", "GuestFilter": "", diff --git a/server/build/dotenv/test.env b/server/build/dotenv/test.env index 41eeb99cf3ae..09302ddccf11 100644 --- a/server/build/dotenv/test.env +++ b/server/build/dotenv/test.env @@ -1,4 +1,4 @@ -TEST_DATABASE_POSTGRESQL_DSN=postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 +TEST_DATABASE_POSTGRESQL_DSN=postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 GOBIN=/mattermost/server/bin CI_INBUCKET_HOST=inbucket diff --git a/server/build/local-test-env.sh b/server/build/local-test-env.sh index 3650710dce71..2dc409e4ed94 100755 --- a/server/build/local-test-env.sh +++ b/server/build/local-test-env.sh @@ -16,12 +16,12 @@ up() { docker compose run --rm start_dependencies - docker compose exec openldap bash -c 'echo -e "dn: ou=testusers,dc=mm,dc=test,dc=com\nobjectclass: organizationalunit" | ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest' - docker compose exec openldap bash -c 'echo -e "dn: uid=test.one,ou=testusers,dc=mm,dc=test,dc=com\nobjectclass: iNetOrgPerson\nsn: User\ncn: Test1\nmail: success+testone@simulator.amazonses.com" | ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest' - docker compose exec openldap bash -c 'ldappasswd -s Password1 -D "cn=admin,dc=mm,dc=test,dc=com" -x "uid=test.one,ou=testusers,dc=mm,dc=test,dc=com" -w mostest' - docker compose exec openldap bash -c 'echo -e "dn: uid=test.two,ou=testusers,dc=mm,dc=test,dc=com\nobjectclass: iNetOrgPerson\nsn: User\ncn: Test2\nmail: success+testtwo@simulator.amazonses.com" | ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest' - docker compose exec openldap bash -c 'ldappasswd -s Password1 -D "cn=admin,dc=mm,dc=test,dc=com" -x "uid=test.two,ou=testusers,dc=mm,dc=test,dc=com" -w mostest' - docker compose exec openldap bash -c 'echo -e "dn: cn=tgroup,ou=testusers,dc=mm,dc=test,dc=com\nobjectclass: groupOfUniqueNames\nuniqueMember: uid=test.one,ou=testusers,dc=mm,dc=test,dc=com" | ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest' + docker compose exec openldap bash -c 'echo -e "dn: ou=testusers,dc=mm,dc=test,dc=com\nobjectclass: organizationalunit" | ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password' + docker compose exec openldap bash -c 'echo -e "dn: uid=test.one,ou=testusers,dc=mm,dc=test,dc=com\nobjectclass: iNetOrgPerson\nsn: User\ncn: Test1\nmail: success+testone@simulator.amazonses.com" | ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password' + docker compose exec openldap bash -c 'ldappasswd -s Password1 -D "cn=admin,dc=mm,dc=test,dc=com" -x "uid=test.one,ou=testusers,dc=mm,dc=test,dc=com" -w mostest_password' + docker compose exec openldap bash -c 'echo -e "dn: uid=test.two,ou=testusers,dc=mm,dc=test,dc=com\nobjectclass: iNetOrgPerson\nsn: User\ncn: Test2\nmail: success+testtwo@simulator.amazonses.com" | ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password' + docker compose exec openldap bash -c 'ldappasswd -s Password1 -D "cn=admin,dc=mm,dc=test,dc=com" -x "uid=test.two,ou=testusers,dc=mm,dc=test,dc=com" -w mostest_password' + docker compose exec openldap bash -c 'echo -e "dn: cn=tgroup,ou=testusers,dc=mm,dc=test,dc=com\nobjectclass: groupOfUniqueNames\nuniqueMember: uid=test.one,ou=testusers,dc=mm,dc=test,dc=com" | ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest_password' docker run -it -u root \ --privileged \ @@ -30,7 +30,7 @@ up() --net ${COMPOSE_PROJECT_NAME}_mm-test \ --env-file=dotenv/test.env -e GOPATH="/go" \ - -e MM_SQLSETTINGS_DATASOURCE="postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10" \ + -e MM_SQLSETTINGS_DATASOURCE="postgres://mmuser:mostest_password@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10" \ -e MM_SQLSETTINGS_DRIVERNAME=postgres mattermost/mattermost-build-server:20210810_golang-1.16.7 bash } diff --git a/server/channels/api4/apitestlib.go b/server/channels/api4/apitestlib.go index cf6ce88db350..46c9df472562 100644 --- a/server/channels/api4/apitestlib.go +++ b/server/channels/api4/apitestlib.go @@ -782,7 +782,7 @@ func (th *TestHelper) SetupLdapConfig() { *cfg.LdapSettings.LdapServer = "dockerhost" *cfg.LdapSettings.BaseDN = "dc=mm,dc=test,dc=com" *cfg.LdapSettings.BindUsername = "cn=admin,dc=mm,dc=test,dc=com" - *cfg.LdapSettings.BindPassword = "mostest" + *cfg.LdapSettings.BindPassword = "mostest_password" *cfg.LdapSettings.FirstNameAttribute = "cn" *cfg.LdapSettings.LastNameAttribute = "sn" *cfg.LdapSettings.NicknameAttribute = "cn" diff --git a/server/channels/store/sqlstore/store_test.go b/server/channels/store/sqlstore/store_test.go index 5ba889a46919..c3e5f9ccbeff 100644 --- a/server/channels/store/sqlstore/store_test.go +++ b/server/channels/store/sqlstore/store_test.go @@ -630,7 +630,7 @@ func TestIsBinaryParamEnabled(t *testing.T) { store: SqlStore{ settings: &model.SqlSettings{ DriverName: model.NewPointer(model.DatabaseDriverPostgres), - DataSource: new("postgres://mmuser:mostest@localhost/loadtest?sslmode=disable\u0026binary_parameters=yes"), + DataSource: new("postgres://mmuser:mostest_password@localhost/loadtest?sslmode=disable\u0026binary_parameters=yes"), }, }, expected: true, @@ -639,7 +639,7 @@ func TestIsBinaryParamEnabled(t *testing.T) { store: SqlStore{ settings: &model.SqlSettings{ DriverName: model.NewPointer(model.DatabaseDriverPostgres), - DataSource: new("postgres://mmuser:mostest@localhost/loadtest?sslmode=disable&binary_parameters=yes"), + DataSource: new("postgres://mmuser:mostest_password@localhost/loadtest?sslmode=disable&binary_parameters=yes"), }, }, expected: true, @@ -648,7 +648,7 @@ func TestIsBinaryParamEnabled(t *testing.T) { store: SqlStore{ settings: &model.SqlSettings{ DriverName: model.NewPointer(model.DatabaseDriverPostgres), - DataSource: new("postgres://mmuser:mostest@localhost/loadtest?sslmode=disable"), + DataSource: new("postgres://mmuser:mostest_password@localhost/loadtest?sslmode=disable"), }, }, expected: false, diff --git a/server/channels/store/storetest/settings.go b/server/channels/store/storetest/settings.go index 6692a3df3244..a236f0437355 100644 --- a/server/channels/store/storetest/settings.go +++ b/server/channels/store/storetest/settings.go @@ -19,7 +19,7 @@ import ( ) const ( - defaultPostgresqlDSN = "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" + defaultPostgresqlDSN = "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" ) func getDefaultPostgresqlDSN() string { diff --git a/server/cmd/mattermost/commands/db_ping.go b/server/cmd/mattermost/commands/db_ping.go index a79ae303e5a2..1eb9e101d9a9 100644 --- a/server/cmd/mattermost/commands/db_ping.go +++ b/server/cmd/mattermost/commands/db_ping.go @@ -37,7 +37,7 @@ Resolves the DSN exactly like 'mattermost db migrate' / 'mattermost db init': the --config flag, then MM_CONFIG, then config.json (which is then loaded as a config store and SqlSettings.DataSource is used).`, Example: ` # Database DSN passed via --config (preferred for readiness probes) - $ mattermost db ping --config postgres://mmuser:mostest@localhost/mattermost --timeout 2m + $ mattermost db ping --config postgres://mmuser:mostest_password@localhost/mattermost --timeout 2m # Or via MM_CONFIG $ MM_CONFIG=postgres://localhost/mattermost mattermost db ping`, diff --git a/server/cmd/mmctl/commands/config.go b/server/cmd/mmctl/commands/config.go index e2904c1e9527..dc53049e8801 100644 --- a/server/cmd/mmctl/commands/config.go +++ b/server/cmd/mmctl/commands/config.go @@ -99,7 +99,7 @@ var ConfigMigrateCmd = &cobra.Command{ Use: "migrate [from_config] [to_config]", Short: "Migrate existing config between backends", Long: "Migrate a file-based configuration to (or from) a database-based configuration. Point the Mattermost server at the target configuration to start using it. Note that this command is only available in `--local` mode.", - Example: `config migrate path/to/config.json "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10"`, + Example: `config migrate path/to/config.json "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10"`, Args: cobra.ExactArgs(2), RunE: withClient(configMigrateCmdF), } diff --git a/server/cmd/mmctl/commands/ldap_e2e_test.go b/server/cmd/mmctl/commands/ldap_e2e_test.go index 69368f03b48e..0e4497c9b3ee 100644 --- a/server/cmd/mmctl/commands/ldap_e2e_test.go +++ b/server/cmd/mmctl/commands/ldap_e2e_test.go @@ -30,7 +30,7 @@ func configForLdap(th *api4.TestHelper) { *cfg.LdapSettings.BaseDN = "dc=mm,dc=test,dc=com" *cfg.LdapSettings.LdapServer = ldapHost *cfg.LdapSettings.BindUsername = "cn=admin,dc=mm,dc=test,dc=com" - *cfg.LdapSettings.BindPassword = "mostest" + *cfg.LdapSettings.BindPassword = "mostest_password" *cfg.LdapSettings.FirstNameAttribute = "cn" *cfg.LdapSettings.LastNameAttribute = "sn" *cfg.LdapSettings.NicknameAttribute = "cn" diff --git a/server/cmd/mmctl/docs/mmctl_config_migrate.rst b/server/cmd/mmctl/docs/mmctl_config_migrate.rst index 9124ef308ba9..6145542b5cd3 100644 --- a/server/cmd/mmctl/docs/mmctl_config_migrate.rst +++ b/server/cmd/mmctl/docs/mmctl_config_migrate.rst @@ -20,7 +20,7 @@ Examples :: - config migrate path/to/config.json "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" + config migrate path/to/config.json "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" Options ~~~~~~~ diff --git a/server/config/database_test.go b/server/config/database_test.go index ec7e5431a8a8..60a2ec0e01c0 100644 --- a/server/config/database_test.go +++ b/server/config/database_test.go @@ -1057,7 +1057,7 @@ func TestDatabaseStoreString(t *testing.T) { maskedDSN := ds.String() assert.True(t, strings.HasPrefix(maskedDSN, "postgres://")) assert.False(t, strings.Contains(maskedDSN, "mmuser")) - assert.False(t, strings.Contains(maskedDSN, "mostest")) + assert.False(t, strings.Contains(maskedDSN, "mostest_password")) } func TestCleanUp(t *testing.T) { diff --git a/server/docker-compose.yaml b/server/docker-compose.yaml index a553909b9818..2fc02bfcc7b9 100644 --- a/server/docker-compose.yaml +++ b/server/docker-compose.yaml @@ -117,7 +117,7 @@ services: working_dir: '/home/mattermost-server/server' environment: - "MM_SQLSETTINGS_DRIVERNAME=postgres" - - "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest@postgres/mattermost_test?sslmode=disable&connect_timeout=10" + - "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest_password@postgres/mattermost_test?sslmode=disable&connect_timeout=10" - "MM_NO_DOCKER=true" - "RUN_SERVER_IN_BACKGROUND=false" - "MM_CLUSTERSETTINGS_ENABLE=true" @@ -151,7 +151,7 @@ services: working_dir: '/home/mattermost-server/server' environment: - "MM_SQLSETTINGS_DRIVERNAME=postgres" - - "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest@postgres/mattermost_test?sslmode=disable&connect_timeout=10" + - "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest_password@postgres/mattermost_test?sslmode=disable&connect_timeout=10" - "MM_NO_DOCKER=true" - "RUN_SERVER_IN_BACKGROUND=false" - "MM_CLUSTERSETTINGS_ENABLE=true" @@ -185,7 +185,7 @@ services: working_dir: '/home/mattermost-server/server' environment: - "MM_SQLSETTINGS_DRIVERNAME=postgres" - - "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest@postgres/mattermost_test?sslmode=disable&connect_timeout=10" + - "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest_password@postgres/mattermost_test?sslmode=disable&connect_timeout=10" - "MM_NO_DOCKER=true" - "RUN_SERVER_IN_BACKGROUND=false" - "MM_CLUSTERSETTINGS_ENABLE=true" diff --git a/server/public/model/config.go b/server/public/model/config.go index 686336782d4d..a2cdb15f0db0 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -150,7 +150,7 @@ const ( TeamSettingsLockProfileFieldsNameAndUsername = "name_and_username" TeamSettingsLockProfileFieldsAll = "all" - SqlSettingsDefaultDataSource = "postgres://mmuser:mostest@localhost/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes" + SqlSettingsDefaultDataSource = "postgres://mmuser:mostest_password@localhost/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes" FileSettingsDefaultDirectory = "./data/" FileSettingsDefaultS3UploadPartSizeBytes = 5 * 1024 * 1024 // 5MB diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index 793a5790a72b..0a328061f8f2 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -2052,7 +2052,7 @@ func TestConfigSanitize(t *testing.T) { t.Run("partially sanitize DataSource", func(t *testing.T) { c := Config{} c.SetDefaults() - *c.SqlSettings.DataSource = "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable" + *c.SqlSettings.DataSource = "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable" c.Sanitize(nil, &SanitizeOptions{PartiallyRedactDataSources: true}) expectedURL := "postgres://" + SanitizedPassword + ":" + SanitizedPassword + "@localhost:5432/mattermost_test?sslmode=disable" @@ -2334,15 +2334,15 @@ func TestSanitizeDataSource(t *testing.T) { "", }, { - "postgres://mmuser:mostest@localhost", + "postgres://mmuser:mostest_password@localhost", "postgres://" + SanitizedPassword + ":" + SanitizedPassword + "@localhost", }, { - "postgres://mmuser:mostest@localhost/dummy?sslmode=disable", + "postgres://mmuser:mostest_password@localhost/dummy?sslmode=disable", "postgres://" + SanitizedPassword + ":" + SanitizedPassword + "@localhost/dummy?sslmode=disable", }, { - "postgres://localhost/dummy?sslmode=disable&user=mmuser&password=mostest", + "postgres://localhost/dummy?sslmode=disable&user=mmuser&password=mostest_password", "postgres://" + SanitizedPassword + ":" + SanitizedPassword + "@localhost/dummy?sslmode=disable", }, } diff --git a/server/scripts/psql-migration-test.sh b/server/scripts/psql-migration-test.sh index 1a17228dc682..102cae687c69 100755 --- a/server/scripts/psql-migration-test.sh +++ b/server/scripts/psql-migration-test.sh @@ -14,7 +14,7 @@ docker exec -i mattermost-postgres psql -U mmuser -d migrated -c "INSERT INTO Sy echo "Setting up config for db migration" cat config/config.json | \ - jq '.SqlSettings.DataSource = "postgres://mmuser:mostest@localhost:5432/migrated?sslmode=disable&connect_timeout=10"'| \ + jq '.SqlSettings.DataSource = "postgres://mmuser:mostest_password@localhost:5432/migrated?sslmode=disable&connect_timeout=10"'| \ jq '.SqlSettings.DriverName = "postgres"' > $TMPDIR/config.json echo "Running the migration" @@ -22,7 +22,7 @@ make ARGS="db migrate --config $TMPDIR/config.json" run-cli echo "Setting up config for fresh db setup" cat config/config.json | \ - jq '.SqlSettings.DataSource = "postgres://mmuser:mostest@localhost:5432/latest?sslmode=disable&connect_timeout=10"'| \ + jq '.SqlSettings.DataSource = "postgres://mmuser:mostest_password@localhost:5432/latest?sslmode=disable&connect_timeout=10"'| \ jq '.SqlSettings.DriverName = "postgres"' > $TMPDIR/config.json echo "Setting up fresh db" diff --git a/server/tests/test-config.json b/server/tests/test-config.json index 780e905b53d9..99203377b09f 100644 --- a/server/tests/test-config.json +++ b/server/tests/test-config.json @@ -91,7 +91,7 @@ }, "SqlSettings": { "DriverName": "postgres", - "DataSource": "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable\u0026connect_timeout=10", + "DataSource": "postgres://mmuser:mostest_password@localhost:5432/mattermost_test?sslmode=disable\u0026connect_timeout=10", "DataSourceReplicas": [], "DataSourceSearchReplicas": [], "Trace": false, diff --git a/tools/sharedchannel-test/server.go b/tools/sharedchannel-test/server.go index 398b98082c49..d7c7c2580536 100644 --- a/tools/sharedchannel-test/server.go +++ b/tools/sharedchannel-test/server.go @@ -99,7 +99,7 @@ func (sm *ServerManager) resetDatabases(ctx context.Context) error { fmt.Sprintf("CREATE DATABASE %s", db), } { cmd := exec.CommandContext(ctx, "docker", "exec", - "-e", "PGPASSWORD=mostest", + "-e", "PGPASSWORD=mostest_password", "mattermost-postgres", "psql", "-U", "mmuser", "-d", "postgres", "-c", sql, ) @@ -143,7 +143,7 @@ func (sm *ServerManager) launchServerB(logsDir string, truncate bool) error { sm.procB.Env = append(sm.procB.Env, "MM_SERVICESETTINGS_SITEURL="+sm.cfg.ServerBURL, "MM_SERVICESETTINGS_LISTENADDRESS=:9066", - "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest@localhost/mattermost_node_test?sslmode=disable&connect_timeout=10&binary_parameters=yes", + "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest_password@localhost/mattermost_node_test?sslmode=disable&connect_timeout=10&binary_parameters=yes", "MM_LOGSETTINGS_FILELOCATION="+filepath.Join(logsDir, "server_b.log"), ) flags := os.O_CREATE | os.O_WRONLY | os.O_APPEND @@ -176,7 +176,7 @@ func (sm *ServerManager) startServers(ctx context.Context) error { sm.procA.Env = append(sm.procA.Env, "MM_SERVICESETTINGS_SITEURL="+sm.cfg.ServerAURL, "MM_SERVICESETTINGS_LISTENADDRESS=:9065", - "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest@localhost/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes", + "MM_SQLSETTINGS_DATASOURCE=postgres://mmuser:mostest_password@localhost/mattermost_test?sslmode=disable&connect_timeout=10&binary_parameters=yes", "MM_LOGSETTINGS_FILELOCATION="+filepath.Join(logsDir, "server_a.log"), ) outA, err := os.Create(filepath.Join(logsDir, "server_a_stdout.log")) @@ -265,7 +265,7 @@ func ProvisionAdmin(ctx context.Context, serverURL, dbName, username, email, pas // (psql -v variable substitution is not supported with -c) cmd := exec.CommandContext(ctx, "docker", "exec", "-i", - "-e", "PGPASSWORD=mostest", + "-e", "PGPASSWORD=mostest_password", "mattermost-postgres", "psql", "-U", "mmuser", "-d", dbName, "-v", "uname="+username, diff --git a/webapp/channels/.github/workflows/performance-benchmarks.yml b/webapp/channels/.github/workflows/performance-benchmarks.yml index 7d5fd60e55df..7f33a80b648a 100644 --- a/webapp/channels/.github/workflows/performance-benchmarks.yml +++ b/webapp/channels/.github/workflows/performance-benchmarks.yml @@ -10,7 +10,7 @@ jobs: postgres: image: postgres env: - POSTGRES_PASSWORD: mostest + POSTGRES_PASSWORD: mostest_password POSTGRES_USER: mmuser options: >- --health-cmd pg_isready @@ -68,7 +68,7 @@ jobs: env: MM_LOGSETTINGS_ENABLECONSOLE: false MM_LOGSETTINGS_FILELEVEL: debug - MM_SQLSETTINGS_DATASOURCE: postgres://mmuser:mostest@localhost/postgres?sslmode=disable&connect_timeout=10&binary_parameters=yes + MM_SQLSETTINGS_DATASOURCE: postgres://mmuser:mostest_password@localhost/postgres?sslmode=disable&connect_timeout=10&binary_parameters=yes MM_TEAMSETTINGS_ENABLEOPENSERVER: true - name: Upload Cypress logs if: ${{ always() }} diff --git a/webapp/channels/src/components/admin_console/database_settings.test.tsx b/webapp/channels/src/components/admin_console/database_settings.test.tsx index b8b903146726..9850f44675b3 100644 --- a/webapp/channels/src/components/admin_console/database_settings.test.tsx +++ b/webapp/channels/src/components/admin_console/database_settings.test.tsx @@ -33,7 +33,7 @@ describe('components/DatabaseSettings', () => { MaxOpenConns: 100, Trace: false, DisableDatabaseSearch: true, - DataSource: 'postgres://mmuser:mostest@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10', + DataSource: 'postgres://mmuser:mostest_password@localhost/mattermost_test?sslmode=disable\u0026connect_timeout=10', QueryTimeout: 10, AnalyticsQueryTimeout: 300, ConnMaxLifetimeMilliseconds: 10,