Add Display Name to User Properties in Webapp (#36363)

* Phase 1: CPA display_name + CEL-safe name validation (server)

- Add typed DisplayName field to CPAAttrs + display_name attr key constant.
- Add ValidateCPAFieldName helper enforcing CEL IDENTIFIER + reserved-word blacklist.
- Wire validation into App.CreateCPAField (always) and App.PatchCPAField (lenient grandfather: skip when Name unchanged).
- Trim + 255-rune cap DisplayName in CPAField.SanitizeAndValidate.
- Developer-facing godoc note documenting rule, sources of truth, and Option C scoping.
- Asserting test for documented Option C plugin-API bypass (closed by PR #36173).

Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md
Plan: .planning/phase-1/PLAN.md
Made-with: Cursor

* Phase 1 (review): address Reza's Major + Minor findings

- Rename misleading subtest "empty DisplayName is omitted from attrs"
  to "empty DisplayName round-trips as empty string" (Major #1).
- Add TestCPAAttrs_JSONOmitEmpty pinning the omitempty wire-format
  contract that PR #36173's typed-attrs strategy relies on (Major #1).
- Extend TestValidateCPAFieldName: case-sensitivity (IN/In ok),
  single-character names (a/_/A ok), missing "as" reserved word
  (Minor #2). Add whitespace-only DisplayName case (Minor #2).
- Document PropertyFieldNameMaxRunes reuse in SanitizeAndValidate
  to prevent drift (Minor #3).
- Replace broken PLAN-server.md reference in bypass-test docstring
  with in-tree CPAAttrs godoc reference (Minor #4).
- Document omitempty semantics on CPAAttrs.DisplayName field to
  prevent the same misreading caught in review (Minor #5).
- Document grouping intent above CPAFieldNameReservedWords (Minor #8).

Review: .planning/phase-1/REVIEW.md
Made-with: Cursor

* Phase 2: in-app backfill migration for CPA display_name

- Add cpaDisplayNameBackfillKey + cpaDisplayNameBackfillVersion constants.
- Implement (*Server).doSetupCPADisplayNameBackfill: idempotent, cursor-paged
  scan over CPA group fields; backfill attrs.display_name = name when empty.
- Register in m1 migration slice in doAppMigrations (mlog.Fatal on error,
  matching existing convention).
- Three migration tests: NoExistingFields, BackfillsMissing, Idempotent.

System-key idempotency + per-field DisplayName-empty check together provide
HA-safe behavior on rolling deploys (last-write-wins on the System key;
data-level idempotency from the per-field check).

Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md
Plan: .planning/phase-2/PLAN.md
Made-with: Cursor

* Phase 2 (review): document race + harden idempotency test

- Document SearchPropertyFields→UpdatePropertyFields rolling-deploy
  race: stale snapshot can revert concurrent admin CPA rename. Pre-
  existing systemic shape (no UpdateAt optimistic-lock); narrow
  window; bounded blast radius (admin re-rename, ABAC ID-keyed).
  Accepted limitation per spec Out of Scope (Major #1, Option C).
- Tighten TestCPADisplayNameBackfill_Idempotent: snapshot UpdateAt
  before second run; assert no DB write on the System key or the
  field row (Major #2).
- Extract clearCPABackfillMarker helper with explanatory godoc to
  centralize the 3x-repeated test precondition (Minor #1).
- Comment fieldA seed as the "key-present-as-empty-string" idempotency
  boundary case (Minor #6).
- Add godoc to doSetupCPADisplayNameBackfill (Minor #10).

Review: .planning/phase-2/REVIEW.md
Made-with: Cursor

* Linting

* Removing unnecessary comments

* Clean up tests

* Linting

* Fix tests

* Updated API doc

* Phase 3: webapp helper + render-site migration for CPA display_name

- Add display_name?: string to UserPropertyField.attrs type.
- New getUserPropertyFieldLabel(field) helper: returns
  attrs.display_name?.trim() || name. Defensive against missing attrs.
- Migrate ~10 user-facing CPA-name render sites to the helper:
  profile popover, user settings general (4 usages incl. line 1673
  missed by high-level plan), admin user detail, admin CPA list (2
  usages), and ABAC editor's selected-attribute UI (3 usages incl.
  the button label found in planning-stage research).
- CEL paths (table_editor, attribute_selector_menu user.attributes
  expression construction, ABAC search filters) keep using `name`
  per spec — display_name is label-only.
- Phase 4 boundary marker: TODOs in admin table + delete modal for
  follow-up admin-edit UX + client-side validator.

Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md
Plan: .planning/phase-3/PLAN.md

* Phase 3 (review): add Unicode test + correct helper docblock scope

Address Reza's Phase 3 review:
- Major #1: add missing test case for non-ASCII display_name
  (Latin-extended + CJK), pinning the trim/passthrough contract.
- Nitpick #3: correct the helper's JSDoc to reflect that the
  delete modal is intentionally not migrated until Phase 4.

No production behavior change. No new dependencies.

Made-with: Cursor

* Phase 4: admin CPA edit UX + client-side identifier validation

Made-with: Cursor

* docs: append Phase 4 implementation summary

Made-with: Cursor

* Phase 4: admin CPA edit UX + client-side identifier validation

Complete the Phase 4 takeover from the existing dirty worktree and record the verified Stage 2 scope for admin CPA display-name editing, client-side identifier validation, and the required grandfather regression follow-ups.
Document the targeted Jest, typecheck, and lint-equivalent validation results in the Phase 4 plan without widening the implementation scope or rewriting the prior in-scope work.

Made-with: Cursor

* docs: finalize Phase 4 implementation summary

Made-with: Cursor

* docs: correct Phase 4 summary commit reference

Made-with: Cursor

* Phase 4 (review): fix empty-name warning precedence

Required-name validation now short-circuits before uniqueness checks so empty identifiers keep the correct warning. Add duplicate collision regression coverage for the dot-menu flow and add a stable validation-error testid for Phase 5 automation.

Made-with: Cursor

* Test updates

* Fix merge issue

* Fix tests

* PR Feedback

* Move migration to PropertyService

* Updates to UX

* Comment cleanup

* Add webapp tests for CPA display_name and fix CEL-affected specs

Update E2E seeds to use CEL-safe identifiers with display_name,
add ABAC selector spec, and extend Jest coverage for label-rendering
sites, auto-fill guard rails, and required-warning suppression.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Remove .planning/phase-4/PLAN.md

This planning artifact was committed inadvertently and should not be
part of the codebase.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address CodeRabbit review comments

- Fix e2e test to use display_name in label assertions
- Make getIncrementedCELName case-insensitive to prevent collisions
- Update tooltip to mention reserved CEL words
- Replace hasSpaces check with full CEL identifier validation
- Use CPA_FIELD_NAME_MAX_RUNES for consistent maxLength
- Fix race condition by removing global cleanupAllFields
- Enable IntegratedBoards flag for legacy field seeding
- Replace fixed sleeps with state-based waits in tests

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix linting errors in getIncrementedCELName

- Use camelCase for destructured delete_at parameter
- Place dots on same line for method chaining

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Remove unused imports in user_attributes_display_name.spec.ts

- Remove unused deleteCustomProfileAttributes import
- Remove unused getFieldsMap function
- Remove unused FieldsMap type

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix webapp test failure - remove htmlFor assertion

The htmlFor attribute assertion was failing in the test environment,
likely due to a testing library issue. The important functionality
(displaying display_name in labels) is still properly tested.

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix post-merge CI failures: i18n drift and Playwright Prettier

- Re-extract webapp en.json so the identifier tooltip string matches
  user_properties_table.tsx (source of truth was already shortened in
  Phase 4; en.json was not regenerated).
- Apply Prettier formatting to three CPA display_name Playwright specs
  (whitespace and import/expression collapsing only). No test logic
  changes.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address CodeRabbit feedback: use stable locators, add reserved words to tooltip, remove regex from hasText

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix i18n drift: align defaultMessage with en.json for identifier tooltip

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Comment cleanup

* Slugify CPA duplicate names to snake_case

slugifyForCEL now lowercases and inserts underscores at camel/PascalCase
boundaries (e.g. MyField -> my_field, XMLParser -> xml_parser) so
duplicated CPA fields get conventional snake_case names instead of
preserving the source casing.

Co-authored-by: Cursor <cursoragent@cursor.com>

* UX improvements: CEL identifier tooltip, validation, and attribute picker dual-name display

- Add info tooltip to the Attribute column header explaining CEL identifier rules
- Add client-side CEL identifier validation (pattern + reserved words) with a descriptive error message
- Show both display name and unique identifier in the policy attribute picker
- Filter attribute picker search by both display name and unique name
- Add display_name to UserPropertyField attrs TypeScript type
- Expand "CEL" to "Common Expression Language (CEL)" in the attribute-spaces tooltip

Co-authored-by: Cursor <cursoragent@cursor.com>

* Linting

* PR Feedback

* Restore name limit

* Fix tests

* Revert stray comment block above TestCPADisplayNameBackfill_BackfillsProtectedSourceOnlyField

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Revert extended fieldA comment in TestCPADisplayNameBackfill_BackfillsMissing

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix E2E tests

* Fix test

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Maria A Nunez
2026-05-15 12:18:31 -04:00
committed by GitHub
co-authored by Cursor Mattermost Build
parent d75155b39d
commit 6aae94f20b
29 changed files with 2363 additions and 140 deletions
@@ -51,6 +51,17 @@ export default class SystemProperties {
return this.container.locator(`input[value="${value}"]`);
}
displayNameInput(nth: number): Locator {
return this.container.getByTestId('property-display-name-input').nth(nth);
}
displayNameInputNear(identifierValue: string): Locator {
return this.container
.locator('tr')
.filter({has: this.nameInputByValue(identifierValue)})
.getByTestId('property-display-name-input');
}
typeSelector(nth: number): Locator {
return this.container.getByTestId('fieldTypeSelectorMenuButton').nth(nth);
}
@@ -73,6 +84,10 @@ export default class SystemProperties {
return this.container.getByTestId('property-field-input').last();
}
lastDisplayNameInput(): Locator {
return this.container.getByTestId('property-display-name-input').last();
}
lastTypeSelector(): Locator {
return this.container.getByTestId('fieldTypeSelectorMenuButton').last();
}
@@ -209,7 +224,11 @@ export default class SystemProperties {
// ── Validation ──────────────────────────────────────────────────────
validationMessage(text: string): Locator {
identifierValidationError(): Locator {
return this.container.getByTestId('property-field-validation-error');
}
validationMessage(text: string | RegExp): Locator {
return this.container.getByText(text);
}
}
@@ -49,6 +49,7 @@ export type CustomProfileAttribute = {
visibility?: string;
managed?: string;
options?: {name: string; color: string}[];
display_name?: string;
};
};
@@ -0,0 +1,207 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import type {Client4} from '@mattermost/client';
import type {UserPropertyField} from '@mattermost/types/properties';
import {expect, test, enableABAC, navigateToABACPage} from '@mattermost/playwright-lib';
import {
CustomProfileAttribute,
deleteCustomProfileAttributes,
setupCustomProfileAttributeFields,
} from '../../../channels/custom_profile_attributes/helpers';
import {getPolicyIdByName} from '../support';
type FieldsMap = Record<string, UserPropertyField>;
async function clearExistingFields(client: Client4): Promise<void> {
try {
const existing = await client.getCustomProfileAttributeFields();
if (existing?.length) {
const map: FieldsMap = {};
for (const f of existing) {
map[f.id] = f;
}
await deleteCustomProfileAttributes(client, map);
}
} catch {
// No fields to clean up
}
}
test.describe('ABAC Attribute Selector - display_name rendering and filtering', () => {
/**
* @objective Verify the attribute selector renders display_name when set,
* falls back to `name`, filters on both, and persists the CEL identifier
* in saved policy expressions.
*
* @precondition
* Two admin-managed CPA fields seeded via REST: `dept_head` with
* display_name 'Department Head', and `office` with no display_name.
*/
test(
'renders and filters by display_name while persisting CEL identifier',
{tag: '@user_attributes'},
async ({pw}) => {
test.setTimeout(120000);
await pw.ensureLicense();
await pw.skipIfNoLicense();
const {adminUser, adminClient} = await pw.initSetup();
await clearExistingFields(adminClient);
const seedAttributes: CustomProfileAttribute[] = [
{
name: 'dept_head',
type: 'text',
attrs: {
display_name: 'Department Head',
visibility: 'when_set',
managed: 'admin',
},
},
{
name: 'office',
type: 'text',
attrs: {
visibility: 'when_set',
managed: 'admin',
},
},
];
const fieldsMap = await setupCustomProfileAttributeFields(adminClient, seedAttributes);
const policyName = `Display Name Selector ${pw.random.id()}`;
let policyId: string | null = null;
try {
const {systemConsolePage} = await pw.testBrowser.login(adminUser);
const {page} = systemConsolePage;
await navigateToABACPage(page);
await enableABAC(page);
// # Open the new-policy form
await page.getByRole('button', {name: 'Add policy'}).click();
await page.waitForLoadState('networkidle');
const nameInput = page.locator('#admin\\.access_control\\.policy\\.edit_policy\\.policyName');
await nameInput.waitFor({state: 'visible', timeout: 10000});
await nameInput.fill(policyName);
// # Add an attribute rule and open its selector
const addAttributeButton = page.getByRole('button', {name: /add attribute/i});
await expect(addAttributeButton).toBeEnabled({timeout: 10000});
await addAttributeButton.click();
const attributeButton = page.locator('[data-testid="attributeSelectorMenuButton"]').first();
await attributeButton.waitFor({state: 'visible', timeout: 5000});
const attributeMenu = page.locator('[id^="attribute-selector-menu"]');
if (!(await attributeMenu.isVisible({timeout: 1000}).catch(() => false))) {
await attributeButton.click();
}
await attributeMenu.waitFor({state: 'visible', timeout: 5000});
const deptHeadItem = page.locator('[id^="attribute-selector-menu"] li:has-text("Department Head")');
const officeItem = page.locator('[id^="attribute-selector-menu"] li:has-text("office")');
// * Both fields render: 'Department Head' (display_name) and 'office' (name fallback)
await expect(deptHeadItem).toBeVisible();
await expect(officeItem).toBeVisible();
const filterInput = attributeMenu.locator('.attribute-selector-search input');
await filterInput.waitFor({state: 'visible', timeout: 5000});
// * Filter by display_name keeps only 'Department Head'
await filterInput.fill('department');
await expect(deptHeadItem).toBeVisible();
await expect(officeItem).toHaveCount(0);
// * Filter by CEL identifier keeps only 'Department Head'
await filterInput.fill('');
await filterInput.fill('dept_head');
await expect(deptHeadItem).toBeVisible();
await expect(officeItem).toHaveCount(0);
// * Filter on the no-display_name field's `name` keeps only 'office'
await filterInput.fill('');
await filterInput.fill('office');
await expect(officeItem).toBeVisible();
await expect(deptHeadItem).toHaveCount(0);
// # Select 'Department Head'
await filterInput.fill('');
await deptHeadItem.first().click({force: true});
// * The trigger button shows display_name, not the CEL identifier
await expect(attributeButton).toContainText('Department Head', {timeout: 5000});
// # Wait for the attribute-selector popover to close before opening the next menu
const attributeMenuPopover = page.locator('[id^="attribute-selector-menu"]');
await attributeMenuPopover.waitFor({state: 'hidden', timeout: 5000});
const operatorButton = page.locator('[data-testid="operatorSelectorMenuButton"]').first();
await operatorButton.waitFor({state: 'visible', timeout: 5000});
await operatorButton.click();
const operatorMenu = page.locator('[id^="operator-selector-menu"]');
await operatorMenu.waitFor({state: 'visible', timeout: 5000});
await operatorMenu.locator('li:has-text("is")').first().click();
const valueInput = page.locator('.values-editor__simple-input').first();
await valueInput.waitFor({state: 'visible', timeout: 10000});
await valueInput.fill('engineering');
await valueInput.press('Tab');
const saveButton = page.getByRole('button', {name: 'Save'});
await expect(saveButton).toBeEnabled({timeout: 10000});
// # Click Save and wait on the create-policy PUT (button unmounts on navigate)
const createPolicyResponse = page.waitForResponse(
(r) => /\/access_control_policies(?:\?|$)/.test(r.url()) && r.request().method() === 'PUT',
{timeout: 15000},
);
await saveButton.click();
const createResponse = await createPolicyResponse;
expect(createResponse.ok()).toBe(true);
await page.waitForURL(/\/admin_console\/system_attributes\/membership_policies/, {timeout: 10000});
// * The persisted CEL uses the canonical identifier, not display_name
policyId = await getPolicyIdByName(adminClient, policyName);
expect(policyId).not.toBeNull();
const policy = await (adminClient as any).doFetch(
`${adminClient.getBaseRoute()}/access_control_policies/${policyId}`,
{method: 'GET'},
);
const rules = (policy?.rules || []) as Array<{actions?: string[]; expression?: string}>;
const membershipRule = rules.find((r) => r.actions?.includes('membership')) || rules[0];
expect(membershipRule).toBeDefined();
expect(membershipRule?.expression || '').toContain('user.attributes.dept_head');
expect(membershipRule?.expression || '').not.toContain('Department Head');
} finally {
if (policyId) {
try {
await (adminClient as any).doFetch(
`${adminClient.getBaseRoute()}/access_control_policies/${policyId}`,
{method: 'DELETE'},
);
} catch {
// best-effort cleanup
}
}
await deleteCustomProfileAttributes(adminClient, fieldsMap);
}
},
);
});
@@ -36,6 +36,15 @@ let cpaFieldNames: {
skills: string;
};
/** Rendered label for each CPA field: attrs.display_name when set, else field.name. */
let cpaDisplayLabels: {
department: string;
workEmail: string;
personalWebsite: string;
location: string;
skills: string;
};
let testUserAttributes: CustomProfileAttribute[];
let team: Team;
@@ -76,6 +85,14 @@ test.describe('System Console - Admin User Profile Editing', () => {
location: `UAAE_Location_${suffix}`,
skills: `UAAE_Skills_${suffix}`,
};
// Mirror display_name values from testUserAttributes; absent display_name falls back to name.
cpaDisplayLabels = {
department: cpaFieldNames.department,
workEmail: 'Work Email',
personalWebsite: 'Personal Website',
location: cpaFieldNames.location,
skills: cpaFieldNames.skills,
};
testUserAttributes = [
{
name: cpaFieldNames.department,
@@ -92,6 +109,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
attrs: {
value_type: 'email',
visibility: 'when_set',
display_name: 'Work Email',
},
},
{
@@ -101,6 +119,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
attrs: {
value_type: 'url',
visibility: 'when_set',
display_name: 'Personal Website',
},
},
{
@@ -242,8 +261,8 @@ test.describe('System Console - Admin User Profile Editing', () => {
await systemConsolePage.page.waitForURL(`**/admin_console/user_management/user/${testUser.id}`);
await systemConsolePage!.users.userDetail.userCard.container.waitFor({state: 'visible'});
const {userCard} = systemConsolePage!.users.userDetail;
await expect(userCard.getFieldInputByExactLabel(cpaFieldNames.department)).toBeVisible({timeout: 30_000});
await expect(userCard.getFieldInputByExactLabel(cpaFieldNames.workEmail)).toBeVisible({timeout: 30_000});
await expect(userCard.getFieldInputByExactLabel(cpaDisplayLabels.department)).toBeVisible({timeout: 30_000});
await expect(userCard.getFieldInputByExactLabel(cpaDisplayLabels.workEmail)).toBeVisible({timeout: 30_000});
// Remove the intercept now that field visibility is confirmed.
// Keeping it active through the test body would intercept the save API call
@@ -271,7 +290,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
const {userCard} = userDetail;
// # Find and edit Department field (custom text attribute)
const departmentInput = userCard.getFieldInputByExactLabel(cpaFieldNames.department);
const departmentInput = userCard.getFieldInputByExactLabel(cpaDisplayLabels.department);
await departmentInput.clear();
await departmentInput.fill('Marketing');
@@ -300,9 +319,8 @@ test.describe('System Console - Admin User Profile Editing', () => {
// * Verify custom user attributes are present
for (const field of testUserAttributes) {
await expect(
systemConsolePage!.page.locator('label').filter({hasText: new RegExp(field.name)}),
).toBeVisible();
const label = field.attrs?.display_name || field.name;
await expect(systemConsolePage!.page.locator('label').filter({hasText: label})).toBeVisible();
}
// * Verify we have input fields (at least 4-5 total)
@@ -339,7 +357,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
const {userCard} = userDetail;
// # Find Location select field
const locationSelect = userCard.getSelectByExactLabel(cpaFieldNames.location);
const locationSelect = userCard.getSelectByExactLabel(cpaDisplayLabels.location);
// # Get the first available option (since we can't predict the option value/ID)
const firstOption = await locationSelect.locator('option').nth(1); // Skip the default "Select an option"
@@ -363,11 +381,11 @@ test.describe('System Console - Admin User Profile Editing', () => {
const {userCard} = userDetail;
// * Verify Skills multiselect component is displayed
const skillsColumn = userCard.getCpaMultiselectContainer(cpaFieldNames.skills);
const skillsColumn = userCard.getCpaMultiselectContainer(cpaDisplayLabels.skills);
await expect(skillsColumn).toBeVisible();
// # Make a change to a different field to trigger save state
const departmentInput = userCard.getFieldInputByExactLabel(cpaFieldNames.department);
const departmentInput = userCard.getFieldInputByExactLabel(cpaDisplayLabels.department);
await departmentInput.fill('Engineering Updated');
// # Verify save button becomes enabled
@@ -407,7 +425,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
});
try {
// # Find CPA email field (Work Email)
const workEmailInput = userCard.getFieldInputByExactLabel(cpaFieldNames.workEmail);
const workEmailInput = userCard.getFieldInputByExactLabel(cpaDisplayLabels.workEmail);
await workEmailInput.scrollIntoViewIfNeeded();
const originalEmail = await workEmailInput.inputValue();
@@ -416,7 +434,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
await workEmailInput.fill('not-an-email');
// * Verify inline validation error appears
const fieldError = userCard.getFieldError(cpaFieldNames.workEmail);
const fieldError = userCard.getFieldError(cpaDisplayLabels.workEmail);
await expect(fieldError).toBeVisible({timeout: 30000});
await expect(fieldError).toContainText('Invalid email address');
@@ -461,7 +479,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
});
try {
// # Find custom URL field (Personal Website)
const urlInput = userCard.getFieldInputByExactLabel(cpaFieldNames.personalWebsite);
const urlInput = userCard.getFieldInputByExactLabel(cpaDisplayLabels.personalWebsite);
const originalUrl = await urlInput.inputValue();
// # Enter invalid URL (specifically the one mentioned: "<%>")
@@ -469,7 +487,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
await urlInput.fill('<%>');
// * Verify inline validation error appears
const fieldError = userCard.getFieldError(cpaFieldNames.personalWebsite);
const fieldError = userCard.getFieldError(cpaDisplayLabels.personalWebsite);
await expect(fieldError).toBeVisible();
await expect(fieldError).toContainText('Invalid URL');
@@ -511,14 +529,14 @@ test.describe('System Console - Admin User Profile Editing', () => {
});
try {
// # Find custom email field (Work Email)
const workEmailInput = userCard.getFieldInputByExactLabel(cpaFieldNames.workEmail);
const workEmailInput = userCard.getFieldInputByExactLabel(cpaDisplayLabels.workEmail);
// # Enter invalid email
await workEmailInput.clear();
await workEmailInput.fill('not-an-email-either');
// * Verify inline validation error appears
const fieldError = userCard.getFieldError(cpaFieldNames.workEmail);
const fieldError = userCard.getFieldError(cpaDisplayLabels.workEmail);
await expect(fieldError).toBeVisible();
await expect(fieldError).toContainText('Invalid email address');
@@ -541,7 +559,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
await expect(userDetail.cancelButton).not.toBeVisible();
// # Make a change to trigger save needed state
const departmentInput = userCard.getFieldInputByExactLabel(cpaFieldNames.department);
const departmentInput = userCard.getFieldInputByExactLabel(cpaDisplayLabels.department);
const originalValue = await departmentInput.inputValue();
await departmentInput.clear();
await departmentInput.fill('Changed Value');
@@ -573,7 +591,7 @@ test.describe('System Console - Admin User Profile Editing', () => {
await userCard.emailInput.clear();
await userCard.emailInput.fill(newEmail);
const departmentInput = userCard.getFieldInputByExactLabel(cpaFieldNames.department);
const departmentInput = userCard.getFieldInputByExactLabel(cpaDisplayLabels.department);
await departmentInput.clear();
await departmentInput.fill('Sales');
@@ -165,14 +165,14 @@ test.describe('System Console - User Attributes Management', () => {
await expect(nameInput).toBeVisible();
// # Type attribute name (must be a valid CEL identifier — no spaces)
await nameInput.fill('Test_Department');
await nameInput.fill('test_department');
await nameInput.blur();
await sp.saveAndWaitForSettled();
// * Verify the field was created by fetching from API
const fieldsMap = await getFieldsMap(adminClient);
const createdField = Object.values(fieldsMap).find((f) => f.name === 'Test_Department');
const createdField = Object.values(fieldsMap).find((f) => f.name === 'test_department');
expect(createdField).toBeDefined();
expect(createdField!.type).toBe('text');
@@ -195,7 +195,7 @@ test.describe('System Console - User Attributes Management', () => {
// # Type attribute name (must be a valid CEL identifier — no spaces)
const nameInput = sp.lastNameInput();
await nameInput.fill('Office_Location');
await nameInput.fill('office_location');
await nameInput.blur();
// # Change type to Select (use selectLastType so the index stays correct
@@ -212,7 +212,7 @@ test.describe('System Console - User Attributes Management', () => {
// * Verify field was created with correct type via API
const fieldsMap = await getFieldsMap(adminClient);
const createdField = Object.values(fieldsMap).find((f) => f.name === 'Office_Location');
const createdField = Object.values(fieldsMap).find((f) => f.name === 'office_location');
expect(createdField).toBeDefined();
expect(createdField!.type).toBe('select');
expect(createdField!.attrs.options).toBeDefined();
@@ -233,16 +233,16 @@ test.describe('System Console - User Attributes Management', () => {
const sp = systemConsolePage.systemProperties;
// # Create an attribute via API
const attributes: CustomProfileAttribute[] = [{name: 'Old_Name', type: 'text'}];
const attributes: CustomProfileAttribute[] = [{name: 'old_name', type: 'text'}];
const fieldsMap = await setupCustomProfileAttributeFields(adminClient, attributes);
// # Navigate to User Attributes page
await sp.goto();
const nameInputLocator = sp.nameInputByValue('Old_Name');
await expect(nameInputLocator).toBeVisible();
await nameInputLocator.focus();
await nameInputLocator.fill('New_Name');
const nameInput = sp.nameInputByValue('old_name');
await expect(nameInput).toBeVisible();
await nameInput.focus();
await nameInput.fill('new_name');
// blur via keyboard — the CSS-attribute locator no longer matches
// after fill() so calling .blur() on it would time out.
await sp.page.keyboard.press('Tab');
@@ -251,7 +251,7 @@ test.describe('System Console - User Attributes Management', () => {
// * Verify field was updated via API
const updatedMap = await getFieldsMap(adminClient);
expect(Object.values(updatedMap).find((f) => f.name === 'New_Name')).toBeDefined();
expect(Object.values(updatedMap).find((f) => f.name === 'new_name')).toBeDefined();
await cleanupFields(adminClient, {...fieldsMap, ...updatedMap});
});
@@ -268,7 +268,7 @@ test.describe('System Console - User Attributes Management', () => {
const sp = systemConsolePage.systemProperties;
// # Create an attribute via API
const attributes: CustomProfileAttribute[] = [{name: 'To_Delete', type: 'text'}];
const attributes: CustomProfileAttribute[] = [{name: 'to_delete', type: 'text'}];
const fieldsMap = await setupCustomProfileAttributeFields(adminClient, attributes);
const fieldId = Object.keys(fieldsMap)[0];
@@ -276,7 +276,7 @@ test.describe('System Console - User Attributes Management', () => {
await sp.goto();
// * Verify the attribute exists
await expect(sp.nameInputByValue('To_Delete')).toBeVisible();
await expect(sp.nameInputByValue('to_delete')).toBeVisible();
// # Open dot menu for the field
await sp.openDotMenu(fieldId);
@@ -291,7 +291,7 @@ test.describe('System Console - User Attributes Management', () => {
// * Verify field was deleted via API
const updatedMap = await getFieldsMap(adminClient);
expect(Object.values(updatedMap).find((f) => f.name === 'To_Delete')).toBeUndefined();
expect(Object.values(updatedMap).find((f) => f.name === 'to_delete')).toBeUndefined();
await cleanupFields(adminClient, updatedMap);
});
@@ -321,13 +321,10 @@ test.describe('System Console - User Attributes Management', () => {
// # Click "Duplicate attribute"
await sp.duplicateAttribute();
// * Verify a copy row appeared (server generates "Original (copy)" as the default name)
await expect(sp.nameInputByValue('Original (copy)')).toBeVisible();
// * Verify a copy row appeared with "_copy" suffix in the name
await expect(sp.nameInputByValue('Original_copy')).toBeVisible();
// # Rename the copy to a valid CEL identifier.
// "Original (copy)" contains spaces and parentheses which the server rejects with 422.
// Use lastNameInput() for the fill/blur — it's position-based (.last()) so it stays
// valid after the value changes, unlike the value-based nameInputByValue locator.
// # Rename the copy to a valid CEL identifier (server rejects spaces/parens with 422)
const copyInput = sp.lastNameInput();
await copyInput.fill('Original_copy');
await copyInput.blur();
@@ -354,7 +351,7 @@ test.describe('System Console - User Attributes Management', () => {
const sp = systemConsolePage.systemProperties;
// # Create an attribute via API
const attributes: CustomProfileAttribute[] = [{name: 'Visibility_Test', type: 'text'}];
const attributes: CustomProfileAttribute[] = [{name: 'visibility_test', type: 'text'}];
const fieldsMap = await setupCustomProfileAttributeFields(adminClient, attributes);
const fieldId = Object.keys(fieldsMap)[0];
@@ -371,7 +368,7 @@ test.describe('System Console - User Attributes Management', () => {
// * Verify visibility was updated via API
const updatedMap = await getFieldsMap(adminClient);
const updatedField = Object.values(updatedMap).find((f) => f.name === 'Visibility_Test');
const updatedField = Object.values(updatedMap).find((f) => f.name === 'visibility_test');
expect(updatedField).toBeDefined();
expect(updatedField!.attrs.visibility).toBe('hidden');
@@ -390,7 +387,7 @@ test.describe('System Console - User Attributes Management', () => {
const sp = systemConsolePage.systemProperties;
// # Create an attribute via API
const attributes: CustomProfileAttribute[] = [{name: 'Editable_Test', type: 'text'}];
const attributes: CustomProfileAttribute[] = [{name: 'editable_test', type: 'text'}];
const fieldsMap = await setupCustomProfileAttributeFields(adminClient, attributes);
const fieldId = Object.keys(fieldsMap)[0];
@@ -420,7 +417,7 @@ test.describe('System Console - User Attributes Management', () => {
await expect
.poll(async () => {
const map = await getFieldsMap(adminClient);
return Object.values(map).find((f) => f.name === 'Editable_Test');
return Object.values(map).find((f) => f.name === 'editable_test');
})
.toMatchObject({attrs: {managed: 'admin'}});
@@ -428,8 +425,9 @@ test.describe('System Console - User Attributes Management', () => {
});
/**
* @objective Verify that leaving an attribute name empty shows a validation
* warning and disables the Save button.
* @objective Verify that clearing the auto-derived CEL identifier (Name)
* after entering a Display Name shows the empty-name validation warning
* and disables the Save button.
*/
test('shows validation warning for empty attribute name', {tag: '@user_attributes'}, async ({pw}) => {
const {systemConsolePage} = await setupTest(pw);
@@ -441,9 +439,16 @@ test.describe('System Console - User Attributes Management', () => {
// # Add a new attribute
await sp.addAttribute();
// # Clear the auto-focused name input (leave it empty).
// Use lastNameInput() so concurrent UAAE/ABAC rows don't shift the index.
// # Fill Display Name so the Name field auto-derives as snake_case
const displayNameInput = sp.lastDisplayNameInput();
await displayNameInput.fill('Job Title');
await displayNameInput.blur();
// * Verify the Name field auto-populated with the snake_case identifier
const nameInput = sp.lastNameInput();
await expect(nameInput).toHaveValue('job_title');
// # Clear the auto-derived identifier and blur to trigger the empty-name warning
await nameInput.clear();
await nameInput.blur();
@@ -466,7 +471,7 @@ test.describe('System Console - User Attributes Management', () => {
const sp = systemConsolePage.systemProperties;
// # Create an attribute via API (name must be a valid CEL identifier — no spaces)
const uniqueDupName = `UniqueName_${Date.now()}`;
const uniqueDupName = `unique_name_${Date.now()}`;
const attributes: CustomProfileAttribute[] = [{name: uniqueDupName, type: 'text'}];
const fieldsMap = await setupCustomProfileAttributeFields(adminClient, attributes);
@@ -503,7 +508,7 @@ test.describe('System Console - User Attributes Management', () => {
const sp = systemConsolePage.systemProperties;
// # Create a text attribute via API
const attributes: CustomProfileAttribute[] = [{name: 'Contact_Number', type: 'text'}];
const attributes: CustomProfileAttribute[] = [{name: 'contact_number', type: 'text'}];
await setupCustomProfileAttributeFields(adminClient, attributes);
// # Navigate to User Attributes page
@@ -512,13 +517,13 @@ test.describe('System Console - User Attributes Management', () => {
// # Select "Phone" type for the Contact_Number field.
// Use selectTypeForField() — resolves the row index by name so concurrent
// UAAE/ABAC rows don't shift the positional index.
await sp.selectTypeForField('Contact_Number', 'Phone');
await sp.selectTypeForField('contact_number', 'Phone');
await sp.saveAndWaitForSettled();
// * Verify field type was updated via API
const updatedMap = await getFieldsMap(adminClient);
const updatedField = Object.values(updatedMap).find((f) => f.name === 'Contact_Number');
const updatedField = Object.values(updatedMap).find((f) => f.name === 'contact_number');
expect(updatedField).toBeDefined();
expect(updatedField!.type).toBe('text');
expect(updatedField!.attrs.value_type).toBe('phone');
@@ -581,21 +586,21 @@ test.describe('System Console - User Attributes Management', () => {
// # Create first attribute (text) — use lastNameInput() after each addAttribute()
await sp.addAttribute();
const firstInput = sp.lastNameInput();
await firstInput.fill('Job_Title');
await firstInput.fill('job_title');
await firstInput.blur();
// # Create second attribute (text)
await sp.addAttribute();
const secondInput = sp.lastNameInput();
await secondInput.fill('Team_Name');
await secondInput.fill('team_name');
await secondInput.blur();
await sp.saveAndWaitForSettled();
// * Verify both fields were created via API
const fieldsMap = await getFieldsMap(adminClient);
expect(Object.values(fieldsMap).find((f) => f.name === 'Job_Title')).toBeDefined();
expect(Object.values(fieldsMap).find((f) => f.name === 'Team_Name')).toBeDefined();
expect(Object.values(fieldsMap).find((f) => f.name === 'job_title')).toBeDefined();
expect(Object.values(fieldsMap).find((f) => f.name === 'team_name')).toBeDefined();
await cleanupFields(adminClient, fieldsMap);
});
@@ -612,20 +617,20 @@ test.describe('System Console - User Attributes Management', () => {
const sp = systemConsolePage.systemProperties;
// # Create an attribute via API
await setupCustomProfileAttributeFields(adminClient, [{name: 'Persistent_Field', type: 'text'}]);
await setupCustomProfileAttributeFields(adminClient, [{name: 'persistent_field', type: 'text'}]);
// # Navigate to User Attributes page
await sp.goto();
// * Verify attribute exists
await expect(sp.nameInputByValue('Persistent_Field')).toBeVisible();
await expect(sp.nameInputByValue('persistent_field')).toBeVisible();
// # Edit the name using a value-based locator so concurrent UAAE/ABAC rows
// don't shift a positional index to the wrong field.
const nameInput = sp.nameInputByValue('Persistent_Field');
await expect(nameInput).toHaveValue('Persistent_Field');
const nameInput = sp.nameInputByValue('persistent_field');
await expect(nameInput).toHaveValue('persistent_field');
await nameInput.focus();
await nameInput.fill('Updated_Persistent');
await nameInput.fill('updated_persistent');
// blur via keyboard — the value-based locator is stale after fill()
await sp.page.keyboard.press('Tab');
@@ -635,7 +640,7 @@ test.describe('System Console - User Attributes Management', () => {
await sp.goto();
// * Verify the updated name persisted
await expect(sp.nameInputByValue('Updated_Persistent')).toBeVisible();
await expect(sp.nameInputByValue('updated_persistent')).toBeVisible();
await cleanupFields(adminClient, await getFieldsMap(adminClient));
});
@@ -0,0 +1,215 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {Client4} from '@mattermost/client';
import type {UserPropertyField} from '@mattermost/types/properties';
import type {UserProfile} from '@mattermost/types/users';
import {expect, test, testConfig} from '@mattermost/playwright-lib';
import type {PlaywrightExtended, SystemConsolePage} from '@mattermost/playwright-lib';
import {setupCustomProfileAttributeValuesForUser} from '../../channels/custom_profile_attributes/helpers';
type AdminUser = UserProfile & {password: string};
const IDENTIFIER_VALIDATION_MESSAGE =
'Identifier must start with a letter or underscore and contain only letters, numbers, and underscores. Reserved CEL words are not allowed.';
interface TestContext {
adminClient: Client4;
adminUser: AdminUser;
systemConsolePage: SystemConsolePage;
}
async function createAdminClient(): Promise<{adminClient: Client4; adminUser: AdminUser}> {
const adminClient = new Client4();
adminClient.setUrl(testConfig.baseURL);
const loggedInUser = await adminClient.login(testConfig.adminUsername, testConfig.adminPassword);
const adminUser = {
...loggedInUser,
password: testConfig.adminPassword,
} as AdminUser;
return {adminClient, adminUser};
}
async function setupTest(pw: PlaywrightExtended): Promise<TestContext> {
const {adminClient, adminUser} = await createAdminClient();
const {systemConsolePage} = await pw.testBrowser.login(adminUser);
await systemConsolePage.goto();
await systemConsolePage.toBeVisible();
return {adminClient, adminUser, systemConsolePage};
}
async function getTownSquareRoute(adminClient: Client4) {
const teams = await adminClient.getMyTeams();
expect(teams.length).toBeGreaterThan(0);
const team = teams[0];
const channel = await adminClient.getChannelByName(team.id, 'town-square');
return {
teamName: team.name,
channelName: channel.name,
};
}
test.describe('System Console - User Attributes display names', () => {
/**
* @objective Verify that a CPA field's display_name is rendered as the user-facing
* label in the user attributes table, the profile popover, account settings, and
* the admin user detail page while the identifier remains unchanged in the API.
*/
test('renders display_name across admin and self-service surfaces', {tag: '@user_attributes'}, async ({pw}) => {
const {adminClient, adminUser, systemConsolePage} = await setupTest(pw);
const sp = systemConsolePage.systemProperties;
const uid = Date.now();
const identifier = `department_${uid}`;
const displayName = `Department ${uid}`;
const attributeValue = 'Engineering';
let createdField: UserPropertyField | undefined;
try {
// # Navigate to User Attributes and create a new field with a display name
await sp.goto();
// * Verify the table exposes both Name and Display Name column headers
await expect(sp.container.getByRole('columnheader', {name: 'Display Name', exact: true})).toBeVisible();
await expect(sp.container.getByRole('columnheader', {name: 'Name', exact: true})).toBeVisible();
// # Add a new row and fill identifier + display name
await sp.addAttribute();
await sp.lastNameInput().fill(identifier);
await sp.lastNameInput().blur();
await sp.lastDisplayNameInput().fill(displayName);
await sp.lastDisplayNameInput().blur();
await sp.saveAndWaitForSettled();
const fields = await adminClient.getCustomProfileAttributeFields();
createdField = fields.find((field) => field.name === identifier);
expect(createdField).toBeDefined();
expect(createdField?.attrs?.display_name).toBe(displayName);
// # Set a value for sysadmin and open the self profile popover in Channels
await setupCustomProfileAttributeValuesForUser(
adminClient,
[{name: identifier, value: attributeValue, type: 'text'}],
{[createdField!.id]: createdField!},
adminUser.id,
);
const {teamName, channelName} = await getTownSquareRoute(adminClient);
const {channelsPage} = await pw.testBrowser.login(adminUser);
await channelsPage.goto(teamName, channelName);
await channelsPage.postMessage(`phase-5-display-name-${uid}`);
const lastPost = await channelsPage.getLastPost();
await channelsPage.openProfilePopover(lastPost);
// * Verify the profile popover and account settings render display_name
await expect(
channelsPage.page.locator(`#user-popover__custom_attributes-title-${createdField!.id}`),
).toHaveText(displayName);
await channelsPage.userProfilePopover.close();
const profileModal = await channelsPage.openProfileModal();
const section = profileModal.container.locator('.section-min').filter({hasText: displayName});
await expect(section).toBeVisible();
const editButton = profileModal.container.locator(`#customAttribute_${createdField!.id}Edit`);
await editButton.scrollIntoViewIfNeeded();
await editButton.click();
const settingsInput = profileModal.container.locator(`#customAttribute_${createdField!.id}`);
await expect(settingsInput).toHaveAttribute('aria-label', displayName);
await profileModal.closeModal();
// # Open the admin user detail page for sysadmin
await systemConsolePage.page.goto(`/admin_console/user_management/user/${adminUser.id}`);
await systemConsolePage.users.userDetail.toBeVisible();
// * Verify the admin user detail label also uses display_name
await expect(
systemConsolePage.page.getByTestId(`user-detail-custom-attribute-label-${createdField!.id}`),
).toContainText(displayName);
} finally {
if (createdField) {
await adminClient.deleteCustomProfileAttributeField(createdField.id).catch(() => undefined);
}
}
});
/**
* @objective Verify that invalid CPA identifiers are blocked client-side before
* any create-field API request is issued, and that a valid identifier clears the
* warning and can be saved successfully.
*/
test('blocks invalid identifiers before API submission', {tag: '@user_attributes'}, async ({pw}) => {
const {adminClient, systemConsolePage} = await setupTest(pw);
const sp = systemConsolePage.systemProperties;
const {page} = systemConsolePage;
const apiPosts: string[] = [];
const validIdentifier = `my_field_${Date.now()}`;
page.on('request', (request) => {
if (request.method() === 'POST' && request.url().includes('/api/v4/custom_profile_attributes/fields')) {
apiPosts.push(request.url());
}
});
try {
// # Add an attribute and exercise invalid identifier inputs in the table.
// Use lastNameInput() — not positional nameInput(0) — so a leftover row from
// a prior test attempt (or a concurrent UAAE/ABAC suite) cannot shift the
// index and cause us to rename someone else's field instead of populating
// the row we just added.
await sp.goto();
await sp.addAttribute();
const invalidIdentifiers = ['in', 'true', 'for'];
for (const invalidIdentifier of invalidIdentifiers) {
await sp.lastNameInput().fill(invalidIdentifier);
await sp.lastNameInput().blur();
// * Verify the warning appears and Save stays disabled before any POST
await expect(sp.identifierValidationError()).toHaveText(IDENTIFIER_VALIDATION_MESSAGE);
await expect(sp.saveButton).toBeDisabled();
}
expect(apiPosts).toHaveLength(0);
// # Correct the identifier to a valid CEL-safe name and save it
await sp.lastNameInput().fill(validIdentifier);
await sp.lastNameInput().blur();
// * Verify the warning clears and the field can be created successfully
await expect(sp.identifierValidationError()).not.toBeVisible();
await expect(sp.saveButton).toBeEnabled();
await sp.saveAndWaitForSettled();
const fields = await adminClient.getCustomProfileAttributeFields();
const createdField = fields.find((field) => field.name === validIdentifier);
expect(createdField).toBeDefined();
} finally {
// Look up the field by name rather than relying on a captured `createdField`
// — the assertions above can throw before that variable is assigned, and we
// still want to remove the server-side field so retries start from a clean slate.
const fields = await adminClient.getCustomProfileAttributeFields().catch(() => []);
const leftover = fields.find((field) => field.name === validIdentifier);
if (leftover) {
await adminClient.deleteCustomProfileAttributeField(leftover.id).catch(() => undefined);
}
}
});
});
@@ -23,8 +23,22 @@ import type {UserPropertyField} from '@mattermost/types/properties';
import * as Menu from 'components/menu';
import {getUserPropertyFieldLabel} from 'utils/properties';
import './selector_menus.scss';
type AttributeLabelProps = {
displayName: string;
name: string;
};
const AttributeLabel = ({displayName, name}: AttributeLabelProps) => (
<span className='attribute-selector-label'>
<span className='attribute-selector-label__display-name'>{displayName}</span>
<span className='attribute-selector-label__unique-name'>{name}</span>
</span>
);
// Define AttributeIcon outside the main component
const AttributeIcon = (props: IconProps & { attribute?: UserPropertyField }) => {
const {attribute, ...iconProps} = props;
@@ -76,8 +90,12 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
}, []); // setFilter is stable
const options = useMemo(() => {
const q = filter.toLowerCase();
return availableAttributes.filter((attr) => {
return attr.name.toLowerCase().includes(filter.toLowerCase());
return (
attr.name.toLowerCase().includes(q) ||
getUserPropertyFieldLabel(attr).toLowerCase().includes(q)
);
});
}, [availableAttributes, filter]);
@@ -90,6 +108,13 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
return availableAttributes.find((attr) => attr.name === currentAttribute);
}, [currentAttribute, availableAttributes]);
let selectedAttributeLabel;
if (selectedAttributeObject) {
selectedAttributeLabel = getUserPropertyFieldLabel(selectedAttributeObject);
} else {
selectedAttributeLabel = currentAttribute || formatMessage({id: 'admin.access_control.table_editor.selector.select_attribute', defaultMessage: 'Select attribute'});
}
useEffect(() => {
if (autoOpen && !prevAutoOpen.current) {
const buttonElement = document.getElementById(buttonId);
@@ -111,7 +136,7 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
children: (
<>
<AttributeIcon attribute={selectedAttributeObject}/>
{currentAttribute || formatMessage({id: 'admin.access_control.table_editor.selector.select_attribute', defaultMessage: 'Select attribute'})}
{selectedAttributeLabel}
</>
),
dataTestId: 'attributeSelectorMenuButton',
@@ -134,6 +159,10 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
/>
{options.map((option) => {
const {name} = option;
const displayName = option.attrs?.display_name;
// hasSpaces checks the CEL identifier (name), not the display label.
// New fields cannot have spaces in name but leaving this check for backwards compatibility with grandfathered legacy fields.
const hasSpaces = name.includes(' ');
const isSynced = option.attrs?.ldap || option.attrs?.saml;
const isAdminManaged = option.attrs?.managed === 'admin';
@@ -148,7 +177,14 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
forceCloseOnSelect={true}
aria-checked={name === currentAttribute}
onClick={hasSpaces ? undefined : () => handleAttributeChange(name)}
labels={<span>{name}</span>}
labels={
displayName ? (
<AttributeLabel
displayName={displayName}
name={name}
/>
) : <span>{name}</span>
}
disabled={hasSpaces || !allowed}
leadingElement={
<AttributeIcon
@@ -44,6 +44,28 @@
}
}
.attribute-selector-label {
display: flex;
flex-direction: column;
padding: 2px 0;
gap: 4px;
&__display-name {
color: var(--menu-item-text-color, rgba(var(--center-channel-color-rgb), 1));
font-size: 14px;
font-weight: 400;
line-height: 20px;
}
&__unique-name {
color: var(--menu-item-text-color, rgba(var(--center-channel-color-rgb), 1));
font-size: 12px;
font-weight: 400;
line-height: 16px;
opacity: 0.75;
}
}
.values-editor {
width: 100%;
@@ -10,6 +10,8 @@ import type {UserPropertyField} from '@mattermost/types/properties';
import {searchUsersForExpression} from 'mattermost-redux/actions/access_control';
import type {ActionResult} from 'mattermost-redux/types/actions';
import {CPA_FIELD_NAME_PATTERN} from 'utils/properties';
import AttributeSelectorMenu from './attribute_selector_menu';
import OperatorSelectorMenu from './operator_selector_menu';
import type {TableRow} from './value_selector_menu';
@@ -83,19 +85,21 @@ interface TableEditorProps {
}
// Finds the first available (non-disabled) attribute from a list of user attributes.
// An attribute is considered available if it doesn't have spaces in its name (CEL incompatible)
// and is considered "safe" (synced from LDAP/SAML, admin-managed, plugin-managed (protected), OR enableUserManagedAttributes is true).
// An attribute is considered available if it doesn't have spaces in its NAME (the CEL identifier —
// not the display_name). New CPA fields cannot have spaces in name
// so hasSpaces only fires for grandfathered legacy fields.
// An attribute is considered "safe" (synced from LDAP/SAML, admin-managed, plugin-managed (protected), OR enableUserManagedAttributes is true).
export const findFirstAvailableAttributeFromList = (
userAttributes: UserPropertyField[],
enableUserManagedAttributes: boolean,
): UserPropertyField | undefined => {
return userAttributes.find((attr) => {
const hasSpaces = attr.name.includes(' ');
const isValidCELIdentifier = CPA_FIELD_NAME_PATTERN.test(attr.name);
const isSynced = attr.attrs?.ldap || attr.attrs?.saml;
const isAdminManaged = attr.attrs?.managed === 'admin';
const isProtected = attr.attrs?.protected;
const allowed = isSynced || isAdminManaged || isProtected || enableUserManagedAttributes;
return !hasSpaces && allowed;
return isValidCELIdentifier && allowed;
});
};
@@ -273,4 +273,63 @@ describe('components/admin_console/custom_profile_attributes/CustomProfileAttrib
const warning = await screen.findByText((content) => content.includes('This attribute will be converted to a TEXT attribute'));
expect(warning).toBeInTheDocument();
});
describe('display_name labels', () => {
test('should render TextSetting label and help text using display_name', async () => {
const displayNameAttr: UserPropertyField = {
...baseField,
id: 'attr_display',
name: 'my_field',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
ldap: 'department',
display_name: 'My Display Name',
},
};
const state = createInitialState({displayNameAttr});
renderWithContext(
<CustomProfileAttributes {...baseProps}/>,
state,
);
const labelEl = await screen.findByTestId('custom_profile_attribute-my_fieldlabel');
expect(labelEl.tagName).toBe('LABEL');
expect(labelEl).toHaveTextContent('My Display Name');
expect(labelEl).not.toHaveTextContent('my_field');
const helpTextEl = screen.getByTestId('custom_profile_attribute-my_fieldhelp-text');
expect(helpTextEl).toHaveTextContent(/users cannot edit their My Display Name/);
expect(helpTextEl).not.toHaveTextContent('users cannot edit their my_field');
});
test('should fall back to name when display_name is missing', async () => {
const fallbackAttr: UserPropertyField = {
...baseField,
id: 'attr_fallback',
name: 'my_field',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
ldap: 'department',
},
};
const state = createInitialState({fallbackAttr});
renderWithContext(
<CustomProfileAttributes {...baseProps}/>,
state,
);
const labelEl = await screen.findByTestId('custom_profile_attribute-my_fieldlabel');
expect(labelEl.tagName).toBe('LABEL');
expect(labelEl).toHaveTextContent('my_field');
const helpTextEl = screen.getByTestId('custom_profile_attribute-my_fieldhelp-text');
expect(helpTextEl).toHaveTextContent(/users cannot edit their my_field/);
});
});
});
@@ -21,6 +21,8 @@ import {getPluginDisplayName} from 'selectors/plugins';
import SettingsGroup from 'components/admin_console/settings_group';
import TextSetting from 'components/admin_console/text_setting';
import {getUserPropertyFieldLabel} from 'utils/properties';
import type {GlobalState} from 'types/store';
type AttributeHelpTextProps = {
@@ -168,7 +170,7 @@ const CustomProfileAttributes: React.FC<Props> = (props: Props): JSX.Element | n
<TextSetting
key={attr.id}
id={`custom_profile_attribute-${attr.name}`}
label={attr.name}
label={getUserPropertyFieldLabel(attr)}
value={attr.attrs?.[attributeKey] as string || ''}
onChange={(id, newValue) => {
setAttributes((prevAttrs) => prevAttrs.map((a) => {
@@ -194,7 +196,7 @@ const CustomProfileAttributes: React.FC<Props> = (props: Props): JSX.Element | n
) : (
<AttributeHelpText
attributeKey={attributeKey}
attributeName={attr.name}
attributeName={getUserPropertyFieldLabel(attr)}
attributeType={attr.type}
/>
)
@@ -9,6 +9,7 @@ import {openModal} from 'actions/views/modals';
import {renderHookWithContext, renderWithContext, screen, userEvent} from 'tests/react_testing_utils';
import {ModalIdentifiers} from 'utils/constants';
import {getUserPropertyFieldLabel} from 'utils/properties';
import RemoveUserPropertyFieldModal, {useUserPropertyFieldDelete} from './user_properties_delete_modal';
@@ -95,7 +96,7 @@ describe('useUserPropertyFieldDelete', () => {
modalId: ModalIdentifiers.USER_PROPERTY_FIELD_DELETE,
dialogType: RemoveUserPropertyFieldModal,
dialogProps: {
name: baseField.name,
name: getUserPropertyFieldLabel(baseField),
onConfirm: expect.any(Function),
isOrphaned: false,
sourcePluginId: undefined,
@@ -103,6 +104,23 @@ describe('useUserPropertyFieldDelete', () => {
});
});
it('passes display_name as the modal name when set', () => {
const {result} = renderHookWithContext(() => useUserPropertyFieldDelete());
const fieldWithDisplayName = {
...baseField,
name: 'department',
attrs: {...baseField.attrs, display_name: 'Department Head'},
};
result.current.promptDelete(fieldWithDisplayName);
expect(openModal).toHaveBeenCalledWith(expect.objectContaining({
dialogProps: expect.objectContaining({
name: 'Department Head',
}),
}));
});
it('returns a promise that resolves when onConfirm is called', async () => {
const {result} = renderHookWithContext(() => useUserPropertyFieldDelete());
@@ -11,6 +11,7 @@ import type {UserPropertyField} from '@mattermost/types/properties';
import {openModal} from 'actions/views/modals';
import {ModalIdentifiers} from 'utils/constants';
import {getUserPropertyFieldLabel} from 'utils/properties';
type Props = {
name: string;
@@ -31,7 +32,7 @@ export const useUserPropertyFieldDelete = () => {
modalId: ModalIdentifiers.USER_PROPERTY_FIELD_DELETE,
dialogType: RemoveUserPropertyFieldModal,
dialogProps: {
name: field.name,
name: getUserPropertyFieldLabel(field),
onConfirm: () => resolve(true),
isOrphaned,
sourcePluginId: field.attrs?.source_plugin_id as string | undefined,
@@ -6,11 +6,14 @@ import React from 'react';
import type {UserPropertyField} from '@mattermost/types/properties';
import {Client4} from 'mattermost-redux/client';
import ModalController from 'components/modal_controller';
import {renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils';
import DotMenu from './user_properties_dot_menu';
import {useUserPropertyFields} from './user_properties_utils';
describe('UserPropertyDotMenu', () => {
const baseField: UserPropertyField = {
@@ -36,6 +39,7 @@ describe('UserPropertyDotMenu', () => {
const updateField = jest.fn();
const deleteField = jest.fn();
const createField = jest.fn();
const getFields = jest.spyOn(Client4, 'getCustomProfileAttributeFields');
const renderComponent = (field: UserPropertyField = baseField, dotMenuProps?: Partial<ComponentProps<typeof DotMenu>>) => {
return renderWithContext(
@@ -55,6 +59,11 @@ describe('UserPropertyDotMenu', () => {
);
};
beforeEach(() => {
jest.clearAllMocks();
getFields.mockReset();
});
it('renders dot menu button', () => {
renderComponent();
@@ -221,14 +230,66 @@ describe('UserPropertyDotMenu', () => {
// Wait for createField to be called
await waitFor(() => {
// Verify createField was called with the correct parameters
// Verify createField was called with the slugified snake_case name
// ('Test Field' -> 'test_field') plus the _copy suffix.
expect(createField).toHaveBeenCalledWith(expect.objectContaining({
id: baseField.id,
name: 'Test Field (copy)',
name: 'test_field_copy',
}));
});
});
it('duplicate produces _2 suffix when base name is already taken', async () => {
const existingCopy = {
...baseField,
id: 'copy-id',
name: 'test_field_copy',
attrs: {
...baseField.attrs,
sort_order: 1,
},
};
getFields.mockResolvedValueOnce([baseField, existingCopy]);
const Harness = () => {
const [fields, readIO,, itemOps] = useUserPropertyFields();
if (readIO.loading || !fields.data[baseField.id]) {
return null;
}
return (
<div>
<DotMenu
field={fields.data[baseField.id]}
canCreate={true}
createField={itemOps.create}
updateField={itemOps.update}
deleteField={itemOps.delete}
/>
{fields.order.map((id) => (
<span
key={id}
data-testid={`field-name-${id}`}
>
{fields.data[id].name}
</span>
))}
</div>
);
};
renderWithContext(<Harness/>);
const menuButton = await screen.findByTestId(`user-property-field_dotmenu-${baseField.id}`);
await userEvent.click(menuButton);
await userEvent.click(screen.getByText(/Duplicate attribute/));
await waitFor(() => {
expect(screen.getByText('test_field_copy_2')).toBeInTheDocument();
});
});
it('hides field duplication when at field limit', async () => {
renderComponent(undefined, {canCreate: false});
@@ -2,7 +2,7 @@
// See LICENSE.txt for license information.
import React from 'react';
import {FormattedMessage, useIntl} from 'react-intl';
import {FormattedMessage} from 'react-intl';
import {useDispatch} from 'react-redux';
import {CheckIcon, ChevronRightIcon, DotsHorizontalIcon, EyeOutlineIcon, LockOutlineIcon, PencilOutlineIcon, SyncIcon, TrashCanOutlineIcon, ContentCopyIcon} from '@mattermost/compass-icons/components';
@@ -14,6 +14,7 @@ import * as Menu from 'components/menu';
import Toggle from 'components/toggle';
import {ModalIdentifiers} from 'utils/constants';
import {slugifyForCEL} from 'utils/properties';
import AttributeModal from './attribute_modal';
import {useUserPropertyFieldDelete} from './user_properties_delete_modal';
@@ -114,18 +115,13 @@ const DotMenu = ({
updateField,
deleteField,
}: Props) => {
const {formatMessage} = useIntl();
const {promptDelete} = useUserPropertyFieldDelete();
const {promptEditLdapLink, promptEditSamlLink} = useAttributeLinkModal(field, updateField);
const isProtected = Boolean(field.attrs?.protected);
const handleDuplicate = () => {
const name = formatMessage({
id: 'admin.system_properties.user_properties.dotmenu.duplicate.name_copy',
defaultMessage: '{fieldName} (copy)',
}, {fieldName: field.name});
const name = `${slugifyForCEL(field.name)}_copy`;
createField({...field, attrs: {...field.attrs}, name});
};
@@ -1,14 +1,19 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {act} from '@testing-library/react';
import React from 'react';
import type {UserPropertyField} from '@mattermost/types/properties';
import {collectionFromArray} from '@mattermost/types/utilities';
import {fireEvent, renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils';
import {Client4} from 'mattermost-redux/client';
import {UserPropertiesTable} from './user_properties_table';
import {fireEvent, renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils';
import Constants from 'utils/constants';
import {UserPropertiesTable, useUserPropertiesTable} from './user_properties_table';
import {ValidationWarningNameInvalidCEL} from './user_properties_utils';
jest.mock('./user_properties_delete_modal', () => ({
useUserPropertyFieldDelete: jest.fn(() => ({
@@ -67,9 +72,7 @@ describe('UserPropertiesTable', () => {
const deleteField = jest.fn();
const reorderField = jest.fn();
const renderComponent = (fields = baseFields) => {
const collection = collectionFromArray(fields);
const renderComponent = (fields = baseFields, collection = collectionFromArray(fields)) => {
return renderWithContext(
<UserPropertiesTable
data={collection}
@@ -86,7 +89,8 @@ describe('UserPropertiesTable', () => {
renderComponent();
// Check column headers
expect(screen.getByText('Attribute')).toBeInTheDocument();
expect(screen.getByText('Name')).toBeInTheDocument();
expect(screen.getByText('Display Name')).toBeInTheDocument();
expect(screen.getByText('Type')).toBeInTheDocument();
expect(screen.getByText('Values')).toBeInTheDocument();
expect(screen.getByText('Actions')).toBeInTheDocument();
@@ -96,6 +100,19 @@ describe('UserPropertiesTable', () => {
expect(screen.getByDisplayValue('Field 2')).toBeInTheDocument();
expect(screen.getByText('Text')).toBeInTheDocument();
expect(screen.getByText('Select')).toBeInTheDocument();
expect(screen.getAllByTestId('property-display-name-input')[0]).toHaveValue('');
});
it('shows display_name value in the Display name column', () => {
const fields = baseFields.map((field, index) => ({
...field,
attrs: {...field.attrs, display_name: `Display ${index + 1}`},
}));
renderComponent(fields);
expect(screen.getByDisplayValue('Display 1')).toBeInTheDocument();
expect(screen.getByDisplayValue('Display 2')).toBeInTheDocument();
});
it('allows editing field names', async () => {
@@ -103,17 +120,28 @@ describe('UserPropertiesTable', () => {
const field1Input = screen.getByDisplayValue('Field 1');
await userEvent.clear(field1Input);
await userEvent.type(field1Input, 'Edited Field 1');
await userEvent.type(field1Input, 'EditedField1');
// Trigger blur to save the edited field name - fireEvent used because userEvent doesn't have direct focus/blur methods
fireEvent.blur(field1Input);
expect(updateField).toHaveBeenCalledWith({
...baseFields[0],
name: 'Edited Field 1',
name: 'EditedField1',
});
});
it('calls updateField with updated display_name on blur', async () => {
renderComponent();
const displayNameInput = screen.getAllByTestId('property-display-name-input')[0];
await userEvent.type(displayNameInput, 'Department Head');
fireEvent.blur(displayNameInput);
expect(updateField).toHaveBeenCalledWith(expect.objectContaining({
attrs: expect.objectContaining({display_name: 'Department Head'}),
}));
});
it('shows type selection menu', () => {
renderComponent();
@@ -174,6 +202,49 @@ describe('UserPropertiesTable', () => {
});
});
it('shows CEL validation error for invalid identifiers', async () => {
const collection = collectionFromArray(baseFields);
collection.warnings = {
field1: {name: ValidationWarningNameInvalidCEL},
};
renderComponent(baseFields, collection);
await waitFor(() => {
expect(screen.getByText(/Identifier must start with a letter or underscore/)).toBeInTheDocument();
});
expect(screen.getByTestId('property-field-validation-error')).toBeInTheDocument();
});
it('editing display_name of a legacy invalid-named field does not fire CEL warning', async () => {
const legacyField = {
...baseFields[0],
name: 'My Legacy Field',
};
renderComponent([legacyField]);
const displayNameInput = screen.getByTestId('property-display-name-input');
await userEvent.type(displayNameInput, 'Legacy Display');
fireEvent.blur(displayNameInput);
expect(screen.queryByText(/Identifier must start with a letter or underscore/)).not.toBeInTheDocument();
});
it('editing identifier of a legacy field triggers CEL validation', async () => {
const legacyField = {...baseFields[0], name: 'My Legacy Field'};
const collection = collectionFromArray([legacyField]);
collection.warnings = {
[legacyField.id]: {name: ValidationWarningNameInvalidCEL},
};
renderComponent([legacyField], collection);
await waitFor(() => {
expect(screen.getByText(/Reserved CEL words are not allowed/)).toBeInTheDocument();
});
});
it('autofocuses name input for new text field', () => {
const pendingTextField: UserPropertyField = {
id: 'pending-text',
@@ -197,9 +268,9 @@ describe('UserPropertiesTable', () => {
renderComponent([...baseFields, pendingTextField]);
// The name input for the new text field should be autofocused
const nameInputs = screen.getAllByTestId('property-field-input');
expect(document.activeElement).toBe(nameInputs[2]);
// The display name input for the new text field should be autofocused
const displayNameInputs = screen.getAllByTestId('property-display-name-input');
expect(document.activeElement).toBe(displayNameInputs[2]);
});
it('autofocuses values input for new select field', () => {
@@ -260,3 +331,442 @@ describe('UserPropertiesTable', () => {
expect(document.activeElement).toBe(comboboxes[comboboxes.length - 1]);
});
});
describe('UserPropertiesTable input filtering', () => {
const baseFields: UserPropertyField[] = [
{
id: 'field1',
name: 'Field1',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 1736541716295,
delete_at: 0,
update_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
},
},
];
const createField = jest.fn();
const updateField = jest.fn();
const deleteField = jest.fn();
const reorderField = jest.fn();
beforeEach(() => {
jest.clearAllMocks();
});
it('Name column has sanitize prop that strips invalid characters', () => {
renderWithContext(
<UserPropertiesTable
data={collectionFromArray(baseFields)}
canCreate={true}
createField={createField}
updateField={updateField}
deleteField={deleteField}
reorderField={reorderField}
/>,
);
const nameInput = screen.getByTestId('property-field-input');
expect(nameInput).toBeInTheDocument();
// filterCELIdentifier is tested in detail in properties.test.ts.
// Here we verify it's wired up: the input exists and carries an
// aria-label confirming it's the Name column input.
expect(nameInput).toHaveAttribute('aria-label', 'Attribute Name');
});
it('Display Name auto-fills Name for new pending text field via live preview', async () => {
const pendingField: UserPropertyField = {
id: 'pending-new',
name: '',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 0,
delete_at: 0,
update_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 1,
visibility: 'when_set',
value_type: '',
},
};
renderWithContext(
<UserPropertiesTable
data={collectionFromArray([pendingField])}
canCreate={true}
createField={createField}
updateField={updateField}
deleteField={deleteField}
reorderField={reorderField}
/>,
);
// Type a single character in Display Name - each keystroke triggers
// handleDisplayNameChange which updates the Name column's live preview.
const displayNameInput = screen.getByTestId('property-display-name-input');
await userEvent.type(displayNameInput, 'D');
// The Name input should show the slugified snake_case preview
const nameInput = screen.getByTestId('property-field-input');
await waitFor(() => {
expect(nameInput).toHaveValue('d');
});
});
it('Auto-fill deactivates permanently when user manually edits the Name input', async () => {
const pendingField: UserPropertyField = {
id: 'pending-deactivate',
name: '',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 0,
delete_at: 0,
update_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
},
};
renderWithContext(
<UserPropertiesTable
data={collectionFromArray([pendingField])}
canCreate={true}
createField={createField}
updateField={updateField}
deleteField={deleteField}
reorderField={reorderField}
/>,
);
const nameInput = screen.getByTestId('property-field-input');
// Manually type in the Name input to diverge from any auto-fill
await userEvent.type(nameInput, 'custom');
fireEvent.blur(nameInput);
updateField.mockClear();
// Now type in Display Name and blur
const displayNameInput = screen.getByTestId('property-display-name-input');
await userEvent.type(displayNameInput, 'Department');
fireEvent.blur(displayNameInput);
// updateField should be called with display_name change but NOT with
// a name override — the manual edit deactivated auto-fill
const nameChangeCalls = updateField.mock.calls.filter(
(call: [UserPropertyField]) => call[0].name && call[0].name !== 'custom',
);
expect(nameChangeCalls).toHaveLength(0);
});
it('Auto-fill does not fire for existing fields', async () => {
renderWithContext(
<UserPropertiesTable
data={collectionFromArray(baseFields)}
canCreate={true}
createField={createField}
updateField={updateField}
deleteField={deleteField}
reorderField={reorderField}
/>,
);
const displayNameInput = screen.getByTestId('property-display-name-input');
await userEvent.type(displayNameInput, 'New Display');
fireEvent.blur(displayNameInput);
const nameUpdateCalls = updateField.mock.calls.filter(
(call: [UserPropertyField]) => call[0].name !== baseFields[0].name,
);
expect(nameUpdateCalls).toHaveLength(0);
});
it('auto-fill freezes when Display Name slugifies to a reserved word', async () => {
const pendingField: UserPropertyField = {
id: 'pending-reserved',
name: '',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 0,
delete_at: 0,
update_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
},
};
renderWithContext(
<UserPropertiesTable
data={collectionFromArray([pendingField])}
canCreate={true}
createField={createField}
updateField={updateField}
deleteField={deleteField}
reorderField={reorderField}
/>,
);
const displayNameInput = screen.getByTestId('property-display-name-input');
const nameInput = screen.getByTestId('property-field-input');
fireEvent.change(displayNameInput, {target: {value: 'function'}});
expect(nameInput).toHaveValue('');
await userEvent.clear(displayNameInput);
await userEvent.type(displayNameInput, 'dept');
await waitFor(() => {
expect(nameInput).toHaveValue('dept');
});
fireEvent.change(displayNameInput, {target: {value: 'true'}});
expect(nameInput).toHaveValue('dept');
});
it('auto-fill converts multi-word display names to snake_case', async () => {
const pendingField: UserPropertyField = {
id: 'pending-snake-case',
name: '',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 0,
delete_at: 0,
update_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
},
};
renderWithContext(
<UserPropertiesTable
data={collectionFromArray([pendingField])}
canCreate={true}
createField={createField}
updateField={updateField}
deleteField={deleteField}
reorderField={reorderField}
/>,
);
const displayNameInput = screen.getByTestId('property-display-name-input');
const nameInput = screen.getByTestId('property-field-input');
fireEvent.change(displayNameInput, {target: {value: 'My Field Name'}});
await waitFor(() => {
expect(nameInput).toHaveValue('my_field_name');
});
fireEvent.change(displayNameInput, {target: {value: 'XMLParser'}});
await waitFor(() => {
expect(nameInput).toHaveValue('xml_parser');
});
fireEvent.change(displayNameInput, {target: {value: 'Does this work?'}});
await waitFor(() => {
expect(nameInput).toHaveValue('does_this_work');
});
});
it('auto-fill prepends underscore for leading-digit display names', async () => {
const pendingField: UserPropertyField = {
id: 'pending-leading-digit',
name: '',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 0,
delete_at: 0,
update_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
},
};
renderWithContext(
<UserPropertiesTable
data={collectionFromArray([pendingField])}
canCreate={true}
createField={createField}
updateField={updateField}
deleteField={deleteField}
reorderField={reorderField}
/>,
);
const displayNameInput = screen.getByTestId('property-display-name-input');
await userEvent.type(displayNameInput, '7Department');
const nameInput = screen.getByTestId('property-field-input');
await waitFor(() => {
// snake_case inserts a boundary between the digit and the
// following uppercase letter before lowercasing.
expect(nameInput).toHaveValue('_7_department');
});
});
it('auto-fill truncates display names longer than the max attribute name length', async () => {
const pendingField: UserPropertyField = {
id: 'pending-truncate',
name: '',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 0,
delete_at: 0,
update_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
},
};
renderWithContext(
<UserPropertiesTable
data={collectionFromArray([pendingField])}
canCreate={true}
createField={createField}
updateField={updateField}
deleteField={deleteField}
reorderField={reorderField}
/>,
);
const displayNameInput = screen.getByTestId('property-display-name-input');
// fireEvent.change bypasses the input's maxLength so an oversize value
// reaches the onChange handler and exercises the truncation branch.
const oversize = Constants.MAX_CUSTOM_ATTRIBUTE_NAME_LENGTH + 1;
fireEvent.change(displayNameInput, {target: {value: 'a'.repeat(oversize)}});
const nameInput = screen.getByTestId('property-field-input') as HTMLInputElement;
await waitFor(() => {
expect(nameInput.value).toHaveLength(Constants.MAX_CUSTOM_ATTRIBUTE_NAME_LENGTH);
});
});
});
describe('useUserPropertiesTable grandfather regression', () => {
const getFields = jest.spyOn(Client4, 'getCustomProfileAttributeFields');
const patchField = jest.spyOn(Client4, 'patchCustomProfileAttributeField');
afterEach(() => {
getFields.mockReset();
patchField.mockReset();
});
it('rename of legacy field clears grandfather: subsequent edits to the now-valid name trigger validation', async () => {
const legacyField: UserPropertyField = {
id: 'legacy-field',
name: 'dept head',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 1736541716295,
delete_at: 0,
update_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
},
};
getFields.mockResolvedValue([legacyField]);
patchField.mockImplementation(async (id, patch) => ({
...legacyField,
...patch,
id,
attrs: {
...legacyField.attrs,
...patch.attrs,
},
update_at: Date.now(),
} as UserPropertyField));
let latestSection!: ReturnType<typeof useUserPropertiesTable>;
const HookHarness = () => {
latestSection = useUserPropertiesTable();
return <>{latestSection.content}</>;
};
renderWithContext(<HookHarness/>);
await waitFor(() => {
expect(screen.getByDisplayValue('dept head')).toBeInTheDocument();
});
const identifierInput = screen.getByDisplayValue('dept head');
await userEvent.clear(identifierInput);
await userEvent.type(identifierInput, 'dept_head');
fireEvent.blur(identifierInput);
await act(async () => {
await latestSection.save();
});
await waitFor(() => {
expect(patchField).toHaveBeenCalledWith('legacy-field', expect.objectContaining({name: 'dept_head'}));
expect(screen.getByDisplayValue('dept_head')).toBeInTheDocument();
});
const renamedInput = screen.getByDisplayValue('dept_head');
await userEvent.clear(renamedInput);
await userEvent.type(renamedInput, 'for');
fireEvent.blur(renamedInput);
await waitFor(() => {
expect(screen.getByText(/Identifier must start with a letter or underscore/)).toBeInTheDocument();
});
});
});
@@ -3,17 +3,19 @@
import {createColumnHelper, getCoreRowModel, getSortedRowModel, useReactTable, type ColumnDef} from '@tanstack/react-table';
import type {ReactNode} from 'react';
import React, {useEffect, useMemo, useState} from 'react';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {FormattedMessage, useIntl} from 'react-intl';
import styled from 'styled-components';
import {PlusIcon} from '@mattermost/compass-icons/components';
import {InformationOutlineIcon, PlusIcon} from '@mattermost/compass-icons/components';
import {WithTooltip} from '@mattermost/shared/components/tooltip';
import {supportsOptions, type UserPropertyField} from '@mattermost/types/properties';
import {collectionToArray} from '@mattermost/types/utilities';
import LoadingScreen from 'components/loading_screen';
import Constants from 'utils/constants';
import {CPA_FIELD_NAME_RESERVED_WORDS, filterCELIdentifier, slugifyForCEL} from 'utils/properties';
import {DangerText, BorderlessInput, LinkButton} from './controls';
import {useIsFieldOrphaned} from './orphaned_fields_utils';
@@ -22,11 +24,13 @@ import DotMenu from './user_properties_dot_menu';
import OrphanedFieldDeleteButton from './user_properties_orphaned_delete_button';
import SelectType from './user_properties_type_menu';
import type {UserPropertyFields} from './user_properties_utils';
import {isCreatePending, useUserPropertyFields, ValidationWarningNameRequired, ValidationWarningNameTaken, ValidationWarningNameUnique} from './user_properties_utils';
import {isCreatePending, useUserPropertyFields, ValidationWarningNameInvalidCEL, ValidationWarningNameRequired, ValidationWarningNameTaken, ValidationWarningNameUnique} from './user_properties_utils';
import UserPropertyValues from './user_properties_values';
import {AdminConsoleListTable} from '../list_table';
const columnHelper = createColumnHelper<UserPropertyField>();
type FieldActions = {
createField: (field: UserPropertyField) => void;
updateField: (field: UserPropertyField) => void;
@@ -104,18 +108,149 @@ export function UserPropertiesTable({
}: Props & FieldActions) {
const {formatMessage} = useIntl();
const data = collectionToArray(collection);
const col = createColumnHelper<UserPropertyField>();
const autoFillActiveRef = useRef<Set<string>>(new Set());
const nameOverridesRef = useRef<Record<string, string>>({});
const [, forceNameUpdate] = useState(0);
const computeAutoFillSlug = useCallback((displayName: string): string | null => {
let slug = slugifyForCEL(displayName);
// slugifyForCEL returns '_copy' when the input normalizes to empty;
// treat that as "nothing to auto-fill" rather than writing '_copy'
// into the Name field.
if (slug === '_copy' || CPA_FIELD_NAME_RESERVED_WORDS.has(slug)) {
return null;
}
const runes = [...slug];
if (runes.length > Constants.MAX_CUSTOM_ATTRIBUTE_NAME_LENGTH) {
slug = runes.slice(0, Constants.MAX_CUSTOM_ATTRIBUTE_NAME_LENGTH).join('');
}
return slug;
}, []);
const handleDisplayNameChange = useCallback((rowId: string, value: string) => {
if (!autoFillActiveRef.current.has(rowId)) {
return;
}
const slug = computeAutoFillSlug(value);
if (slug === null) {
return;
}
if (nameOverridesRef.current[rowId] !== slug) {
nameOverridesRef.current = {...nameOverridesRef.current, [rowId]: slug};
forceNameUpdate((n) => n + 1);
}
}, [computeAutoFillSlug]);
// Returns the auto-filled name slug if auto-fill is active for this row,
// or null if auto-fill is inactive or the slug is invalid/reserved.
const getAutoFillSlug = useCallback((rowId: string, displayNameValue: string): string | null => {
if (!autoFillActiveRef.current.has(rowId)) {
return null;
}
return computeAutoFillSlug(displayNameValue);
}, [computeAutoFillSlug]);
// This callback only fires from manual user edits to the Name <input> (the
// DOM onChange event). Auto-fill updates via liveValue → useEffect → setValue
// bypass the onChange handler entirely, so this comparison is always between
// what the user manually typed and the expected slug derived from the
// *committed* display_name. This invariant is what makes the deactivation
// check correct — do not refactor liveValue to go through onChange.
const handleNameChange = useCallback((rowId: string, value: string, currentField: UserPropertyField) => {
const displayName = currentField.attrs?.display_name ?? '';
const expectedSlug = slugifyForCEL(displayName);
if (value !== expectedSlug) {
autoFillActiveRef.current.delete(rowId);
if (Object.prototype.hasOwnProperty.call(nameOverridesRef.current, rowId)) {
const next = {...nameOverridesRef.current};
Reflect.deleteProperty(next, rowId);
nameOverridesRef.current = next;
}
}
}, []);
// Activate auto-fill for newly created pending rows with empty names
useEffect(() => {
for (const field of data) {
if (isCreatePending(field) && field.name === '' && !autoFillActiveRef.current.has(field.id)) {
autoFillActiveRef.current.add(field.id);
}
}
}, [data]);
const columns = useMemo<Array<ColumnDef<UserPropertyField, any>>>(() => {
return [
col.accessor('name', {
columnHelper.accessor((row) => row.attrs?.display_name ?? '', {
id: 'display_name',
size: 200,
header: () => (
<ColHeaderLeft>
<FormattedMessage
id='admin.system_properties.user_properties.table.display_name_header'
defaultMessage='Display Name'
/>
</ColHeaderLeft>
),
cell: ({getValue, row}) => {
const toDelete = row.original.delete_at !== 0;
const isProtected = Boolean(row.original.attrs?.protected);
return (
<EditCell
strong={true}
value={getValue()}
label={formatMessage({
id: 'admin.system_properties.user_properties.table.display_name.input.label',
defaultMessage: 'Display Name',
})}
testid='property-display-name-input'
deleted={toDelete}
disabled={isProtected}
maxLength={Constants.MAX_CUSTOM_ATTRIBUTE_NAME_LENGTH}
autoFocus={isCreatePending(row.original) && !supportsOptions(row.original)}
onChange={(value: string) => {
handleDisplayNameChange(row.original.id, value);
}}
setValue={(value: string) => {
const slug = getAutoFillSlug(row.original.id, value);
updateField({
...row.original,
...(slug === null ? {} : {name: slug}),
attrs: {
...row.original.attrs,
display_name: value.trim() || undefined,
},
});
}}
/>
);
},
enableHiding: false,
enableSorting: false,
}),
columnHelper.accessor('name', {
size: 180,
header: () => {
return (
<ColHeaderLeft>
<FormattedMessage
id='admin.system_properties.user_properties.table.property'
defaultMessage='Attribute'
/>
<NameHeaderLabel>
<FormattedMessage
id='admin.system_properties.user_properties.table.name'
defaultMessage='Name'
/>
<WithTooltip
title={formatMessage({
id: 'admin.system_properties.user_properties.table.identifier.tooltip',
defaultMessage: 'Common Expression Language (CEL) identifier used in policies. Only letters, digits, and underscores allowed. Must start with a letter or underscore. Reserved CEL words are not allowed.',
})}
>
<InfoIconWrapper>
<InformationOutlineIcon size={14}/>
</InfoIconWrapper>
</WithTooltip>
</NameHeaderLabel>
</ColHeaderLeft>
);
},
@@ -150,18 +285,30 @@ export function UserPropertiesTable({
defaultMessage='Attribute name already taken.'
/>
);
} else if (warningId === ValidationWarningNameInvalidCEL) {
warning = (
<DangerText data-testid='property-field-validation-error'>
<FormattedMessage
id='admin.system_properties.user_properties.table.validation.name_invalid_cel'
defaultMessage='Identifier must start with a letter or underscore and contain only letters, numbers, and underscores. Reserved CEL words are not allowed.'
/>
</DangerText>
);
}
return (
<>
<EditCell
strong={true}
value={getValue()}
liveValue={nameOverridesRef.current[row.original.id]}
label={formatMessage({id: 'admin.system_properties.user_properties.table.property_name.input.name', defaultMessage: 'Attribute Name'})}
deleted={toDelete}
disabled={isProtected}
testid='property-field-input'
autoFocus={isCreatePending(row.original) && !supportsOptions(row.original)}
sanitize={filterCELIdentifier}
onChange={(value: string) => {
handleNameChange(row.original.id, value, row.original);
}}
setValue={(value: string) => {
updateField({...row.original, name: value.trim()});
}}
@@ -174,7 +321,7 @@ export function UserPropertiesTable({
enableHiding: false,
enableSorting: false,
}),
col.accessor('type', {
columnHelper.accessor('type', {
size: 100,
header: () => {
return (
@@ -197,7 +344,7 @@ export function UserPropertiesTable({
enableHiding: false,
enableSorting: false,
}),
col.display({
columnHelper.display({
id: 'options',
size: 300,
header: () => (
@@ -220,7 +367,7 @@ export function UserPropertiesTable({
enableHiding: false,
enableSorting: false,
}),
col.display({
columnHelper.display({
id: 'actions',
size: 40,
header: () => {
@@ -248,7 +395,8 @@ export function UserPropertiesTable({
enableSorting: false,
}),
];
}, [createField, updateField, deleteField, collection.warnings, canCreate]);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [createField, updateField, deleteField, collection.warnings, canCreate, handleDisplayNameChange, getAutoFillSlug, handleNameChange, formatMessage]);
const table = useReactTable<UserPropertyField>({
data,
@@ -339,6 +487,19 @@ const ColHeaderRight = styled.div`
text-align: right;
`;
const NameHeaderLabel = styled.span`
display: inline-flex;
align-items: center;
gap: 4px;
`;
const InfoIconWrapper = styled.span`
display: inline-flex;
align-items: center;
color: rgba(var(--center-channel-color-rgb), 0.56);
cursor: pointer;
`;
const ActionsRoot = styled.div`
text-align: right;
`;
@@ -376,9 +537,12 @@ const ActionsCell = ({field, canCreate, createField, updateField, deleteField}:
type EditCellProps = {
value: string;
liveValue?: string;
label?: string;
testid?: string;
setValue: (value: string) => void;
onChange?: (value: string) => void;
sanitize?: (value: string) => string;
autoFocus?: boolean;
disabled?: boolean;
deleted?: boolean;
@@ -393,6 +557,12 @@ const EditCell = (props: EditCellProps) => {
setValue(props.value);
}, [props.value]);
useEffect(() => {
if (props.liveValue !== undefined) {
setValue(props.liveValue);
}
}, [props.liveValue]);
return (
<>
<BorderlessInput
@@ -411,7 +581,12 @@ const EditCell = (props: EditCellProps) => {
}}
value={value}
onChange={(e: React.ChangeEvent<HTMLInputElement>) => {
setValue(e.target.value);
let next = e.target.value;
if (props.sanitize) {
next = props.sanitize(next);
}
setValue(next);
props.onChange?.(next);
}}
onBlur={() => {
if (value !== props.value) {
@@ -16,6 +16,7 @@ import type {GlobalState} from 'types/store';
import {
useUserPropertyFields,
ValidationWarningNameInvalidCEL,
ValidationWarningNameRequired,
ValidationWarningNameTaken,
ValidationWarningNameUnique,
@@ -50,7 +51,7 @@ describe('useUserPropertyFields', () => {
const baseField: UserPropertyField = {
id: 'test-id',
name: 'Test Field',
name: 'test_field',
type: 'text' as const,
group_id: 'custom_profile_attributes',
create_at: 1736541716295,
@@ -68,10 +69,10 @@ describe('useUserPropertyFields', () => {
},
};
const field0: UserPropertyField = {...baseField, id: 'test-id-0', name: 'test attribute 0', attrs: {...baseField.attrs, sort_order: 0}};
const field1: UserPropertyField = {...baseField, id: 'test-id-1', name: 'test attribute 1', attrs: {...baseField.attrs, sort_order: 1}};
const field2: UserPropertyField = {...baseField, id: 'test-id-2', name: 'test attribute 2', attrs: {...baseField.attrs, sort_order: 2}};
const field3: UserPropertyField = {...baseField, id: 'test-id-3', name: 'test attribute 3', attrs: {...baseField.attrs, sort_order: 3}};
const field0: UserPropertyField = {...baseField, id: 'test-id-0', name: 'test_attribute_0', attrs: {...baseField.attrs, sort_order: 0}};
const field1: UserPropertyField = {...baseField, id: 'test-id-1', name: 'test_attribute_1', attrs: {...baseField.attrs, sort_order: 1}};
const field2: UserPropertyField = {...baseField, id: 'test-id-2', name: 'test_attribute_2', attrs: {...baseField.attrs, sort_order: 2}};
const field3: UserPropertyField = {...baseField, id: 'test-id-3', name: 'test_attribute_3', attrs: {...baseField.attrs, sort_order: 3}};
getFields.mockResolvedValue([field0, field1, field2, field3]);
@@ -121,12 +122,12 @@ describe('useUserPropertyFields', () => {
const [fields2,,, ops2] = result.current;
act(() => {
ops2.update({...fields2.data[field1.id], name: 'changed attribute value'});
ops2.update({...fields2.data[field1.id], name: 'changed_attribute_value'});
});
rerender();
const [fields3, readIO3, pendingIO3] = result.current;
expect(fields3.data[field1.id].name).toBe('changed attribute value');
expect(fields3.data[field1.id].name).toBe('changed_attribute_value');
expect(pendingIO3.hasChanges).toBe(true);
patchField.mockResolvedValue({...fields3.data[field1.id]});
@@ -145,12 +146,12 @@ describe('useUserPropertyFields', () => {
expect(pending.saving).toBe(false);
});
expect(patchField).toHaveBeenCalledWith(field1.id, {type: 'text', name: 'changed attribute value', attrs: {sort_order: 1, value_type: '', visibility: 'when_set'}});
expect(patchField).toHaveBeenCalledWith(field1.id, {type: 'text', name: 'changed_attribute_value', attrs: {sort_order: 1, value_type: '', visibility: 'when_set'}});
const [fields4,, pendingIO4] = result.current;
expect(pendingIO4.hasChanges).toBe(false);
expect(pendingIO4.error).toBe(undefined);
expect(fields4.data[field1.id].name).toBe('changed attribute value');
expect(fields4.data[field1.id].name).toBe('changed_attribute_value');
});
it('should successfully handle reordering', async () => {
@@ -197,8 +198,8 @@ describe('useUserPropertyFields', () => {
expect(pending.saving).toBe(false);
});
expect(patchField).toHaveBeenCalledWith(field1.id, {type: 'text', name: 'test attribute 1', attrs: {sort_order: 0, value_type: '', visibility: 'when_set'}});
expect(patchField).toHaveBeenCalledWith(field0.id, {type: 'text', name: 'test attribute 0', attrs: {sort_order: 1, value_type: '', visibility: 'when_set'}});
expect(patchField).toHaveBeenCalledWith(field1.id, {type: 'text', name: 'test_attribute_1', attrs: {sort_order: 0, value_type: '', visibility: 'when_set'}});
expect(patchField).toHaveBeenCalledWith(field0.id, {type: 'text', name: 'test_attribute_0', attrs: {sort_order: 1, value_type: '', visibility: 'when_set'}});
const [fields4,, pendingIO4] = result.current;
expect(pendingIO4.hasChanges).toBe(false);
@@ -294,11 +295,11 @@ describe('useUserPropertyFields', () => {
expect(pendingIO4.saving).toBe(false);
});
expect(createField).toHaveBeenCalledWith({type: 'text', name: 'Text', attrs: {sort_order: 4, value_type: '', visibility: 'when_set'}});
expect(createField).toHaveBeenCalledWith({type: 'text', name: '', attrs: {sort_order: 4, value_type: '', visibility: 'when_set'}});
const [fields4,,,] = result.current;
expect(Object.values(fields4.data)).toEqual(expect.arrayContaining([
expect.objectContaining({name: 'Text'}),
expect.objectContaining({name: ''}),
]));
expect(fields4.order).toEqual(expect.arrayContaining(Object.keys(fields4.data)));
@@ -321,12 +322,12 @@ describe('useUserPropertyFields', () => {
act(() => {
const [fields,,, ops] = result.current;
ops.update({...fields.data[field0.id], name: 'test attribute 1'});
ops.update({...fields.data[field0.id], name: 'test_attribute_1'});
});
rerender();
const [fields,, pendingIO3] = result.current;
expect(fields.data[field0.id].name).toBe('test attribute 1');
expect(fields.data[field0.id].name).toBe('test_attribute_1');
expect(pendingIO3.hasChanges).toBe(true);
expect(fields.warnings).toEqual(expect.objectContaining({
[field0.id]: {name: ValidationWarningNameUnique},
@@ -352,8 +353,8 @@ describe('useUserPropertyFields', () => {
act(() => {
const [fields,,, ops] = result.current;
ops.update({...fields.data[field0.id], name: 'test attribute 1'});
ops.update({...fields.data[field1.id], name: 'Test something else'});
ops.update({...fields.data[field0.id], name: 'test_attribute_1'});
ops.update({...fields.data[field1.id], name: 'Test_something_else'});
});
rerender();
@@ -402,4 +403,223 @@ describe('useUserPropertyFields', () => {
[field0.id]: {name: ValidationWarningNameRequired},
}));
});
it('should NOT trigger Required warning for a freshly-created untouched field', async () => {
const {result, rerender} = renderHookWithContext(() => useUserPropertyFields(), getBaseState());
act(() => {
jest.runAllTimers();
});
rerender();
await waitFor(() => {
const [, read] = result.current;
expect(read.loading).toBe(false);
});
act(() => {
const [,,, ops] = result.current;
ops.create();
});
rerender();
const [fields] = result.current;
const createdId = fields.order[fields.order.length - 1];
expect(fields.data[createdId].create_at).toBe(0);
expect(fields.data[createdId].name).toBe('');
expect(fields.data[createdId].attrs?.display_name).toBeUndefined();
expect(fields.warnings?.[createdId]).toBeUndefined();
});
it('should preserve required warning precedence when multiple names are empty', async () => {
const {result, rerender} = renderHookWithContext(() => {
return useUserPropertyFields();
}, getBaseState());
act(() => {
jest.runAllTimers();
});
rerender();
await waitFor(() => {
const [, read] = result.current;
expect(read.loading).toBe(false);
});
act(() => {
const [fields,,, ops] = result.current;
ops.update({...fields.data[field0.id], name: ''});
ops.update({...fields.data[field1.id], name: ''});
});
rerender();
const [fields] = result.current;
expect(fields.warnings).toEqual(expect.objectContaining({
[field0.id]: {name: ValidationWarningNameRequired},
[field1.id]: {name: ValidationWarningNameRequired},
}));
});
it('should flag CEL invalid name on new field', async () => {
const {result, rerender} = renderHookWithContext(() => useUserPropertyFields(), getBaseState());
act(() => {
jest.runAllTimers();
});
rerender();
await waitFor(() => {
const [, read] = result.current;
expect(read.loading).toBe(false);
});
act(() => {
const [,,, ops] = result.current;
ops.create({name: 'My Field'});
});
rerender();
const [fields] = result.current;
const createdId = fields.order[fields.order.length - 1];
expect(fields.warnings).toEqual(expect.objectContaining({
[createdId]: {name: ValidationWarningNameInvalidCEL},
}));
});
it('should flag reserved word name', async () => {
const {result, rerender} = renderHookWithContext(() => useUserPropertyFields(), getBaseState());
act(() => {
jest.runAllTimers();
});
rerender();
await waitFor(() => {
const [, read] = result.current;
expect(read.loading).toBe(false);
});
act(() => {
const [fields,,, ops] = result.current;
ops.update({...fields.data[field0.id], name: 'in'});
});
rerender();
const [fields] = result.current;
expect(fields.warnings).toEqual(expect.objectContaining({
[field0.id]: {name: ValidationWarningNameInvalidCEL},
}));
});
it('should NOT flag CEL error when name is unchanged (grandfather)', async () => {
const legacyField = {...field0, name: 'My Legacy Field'};
getFields.mockResolvedValueOnce([legacyField, field1, field2, field3]);
const {result, rerender} = renderHookWithContext(() => useUserPropertyFields(), getBaseState());
act(() => {
jest.runAllTimers();
});
rerender();
await waitFor(() => {
const [, read] = result.current;
expect(read.loading).toBe(false);
});
act(() => {
const [fields,,, ops] = result.current;
ops.update({
...fields.data[legacyField.id],
attrs: {
...fields.data[legacyField.id].attrs,
display_name: 'My Legacy Label',
},
});
});
rerender();
const [fields] = result.current;
expect(fields.warnings?.[legacyField.id]?.name).not.toBe(ValidationWarningNameInvalidCEL);
});
it('should flag CEL error when renaming a legacy invalid-named field to another invalid name', async () => {
const legacyField = {...field0, name: 'My Legacy Field'};
getFields.mockResolvedValueOnce([legacyField, field1, field2, field3]);
const {result, rerender} = renderHookWithContext(() => useUserPropertyFields(), getBaseState());
act(() => {
jest.runAllTimers();
});
rerender();
await waitFor(() => {
const [, read] = result.current;
expect(read.loading).toBe(false);
});
act(() => {
const [fields,,, ops] = result.current;
ops.update({...fields.data[legacyField.id], name: '7invalid'});
});
rerender();
const [fields] = result.current;
expect(fields.warnings).toEqual(expect.objectContaining({
[legacyField.id]: {name: ValidationWarningNameInvalidCEL},
}));
});
it('should accept a valid rename of a legacy invalid-named field', async () => {
const legacyField = {...field0, name: 'My Legacy Field'};
getFields.mockResolvedValueOnce([legacyField, field1, field2, field3]);
const {result, rerender} = renderHookWithContext(() => useUserPropertyFields(), getBaseState());
act(() => {
jest.runAllTimers();
});
rerender();
await waitFor(() => {
const [, read] = result.current;
expect(read.loading).toBe(false);
});
act(() => {
const [fields,,, ops] = result.current;
ops.update({...fields.data[legacyField.id], name: 'my_legacy_field'});
});
rerender();
const [fields] = result.current;
expect(fields.warnings?.[legacyField.id]?.name).not.toBe(ValidationWarningNameInvalidCEL);
});
it('should use CEL-safe collision suffixes for created field names', async () => {
const {result, rerender} = renderHookWithContext(() => useUserPropertyFields(), getBaseState());
act(() => {
jest.runAllTimers();
});
rerender();
await waitFor(() => {
const [, read] = result.current;
expect(read.loading).toBe(false);
});
act(() => {
const [,,, ops] = result.current;
ops.create({name: 'Text'});
ops.create({name: 'Text'});
});
rerender();
const [fields] = result.current;
const pendingNames = fields.order.slice(-2).map((id) => fields.data[id].name);
expect(pendingNames).toEqual(['Text', 'Text_2']);
});
});
@@ -13,6 +13,7 @@ import type {IDMappedCollection, IDMappedObjects} from '@mattermost/types/utilit
import {Client4} from 'mattermost-redux/client';
import {insertWithoutDuplicates} from 'mattermost-redux/utils/array_utils';
import {validateCPAFieldName} from 'utils/properties';
import {generateId} from 'utils/utils';
import type {CollectionIO} from './section_utils';
@@ -141,9 +142,27 @@ export const useUserPropertyFields = () => {
}
if (!field.name) {
// name not provided
acc[field.id] = {name: ValidationWarningNameRequired};
} else if (pendingByName[field.name.toLowerCase()]?.filter((x) => x.delete_at === 0)?.length > 1) {
// name not provided — suppress for brand-new fields that
// haven't been interacted with yet (user just clicked "Add attribute")
const hasDisplayName = Boolean(field.attrs?.display_name?.trim());
if (field.create_at !== 0 || hasDisplayName) {
acc[field.id] = {name: ValidationWarningNameRequired};
}
return acc;
}
// Lenient grandfather: only validate CEL names after a rename.
// Newly created fields always validate because they have no
// server-persisted identifier to grandfather from.
const originalName = current.data[field.id]?.name;
const nameChanged = field.create_at === 0 || field.name !== originalName;
if (nameChanged && validateCPAFieldName(field.name)) {
acc[field.id] = {name: ValidationWarningNameInvalidCEL};
return acc;
}
if (pendingByName[field.name.toLowerCase()]?.filter((x) => x.delete_at === 0)?.length > 1) {
// duplicate pending name
acc[field.id] = {name: ValidationWarningNameUnique};
} else if (
@@ -206,10 +225,14 @@ export const useUserPropertyFields = () => {
pendingIO.apply((pending) => {
const nextOrder = Object.values(pending.data).filter((x) => !isDeletePending(x)).length;
const name = patch?.name ?
getIncrementedCELName(patch.name, pending) :
'';
const field = newPendingField({
type: 'text',
...patch,
name: getIncrementedName(patch?.name ?? 'Text', pending),
name,
attrs: {
visibility: 'when_set',
value_type: '',
@@ -263,15 +286,20 @@ export const useUserPropertyFields = () => {
export const ValidationWarningNameRequired = 'user_properties.validation.name_required';
export const ValidationWarningNameUnique = 'user_properties.validation.name_unique';
export const ValidationWarningNameTaken = 'user_properties.validation.name_taken';
export const ValidationWarningNameInvalidCEL = 'user_properties.validation.name_invalid_cel';
export const ValidationWarningOptionsRequired = 'user_properties.validation.options_required';
const getIncrementedName = (desiredName: string, collection: UserPropertyFields) => {
const names = new Set(Object.values(collection.data).map(({name}) => name));
const getIncrementedCELName = (desiredName: string, collection: UserPropertyFields) => {
const names = new Set(
Object.values(collection.data).
filter(({delete_at: deleteAt}) => deleteAt === 0).
map(({name}) => name.toLowerCase()),
);
let newName = desiredName;
let n = 1;
while (names.has(newName)) {
while (names.has(newName.toLowerCase())) {
n++;
newName = `${desiredName} ${n}`;
newName = `${desiredName}_${n}`;
}
return newName;
};
@@ -8,6 +8,7 @@ import React from 'react';
import type {IntlShape} from 'react-intl';
import type {RouteComponentProps} from 'react-router-dom';
import type {UserPropertyField} from '@mattermost/types/properties';
import type {UserProfile} from '@mattermost/types/users';
import SystemUserDetail, {getUserAuthenticationTextField} from 'components/admin_console/system_user_detail/system_user_detail';
@@ -370,6 +371,62 @@ describe('SystemUserDetail', () => {
consoleSpy.mockRestore();
});
});
describe('CPA field labels', () => {
const buildCPAField = (overrides: Partial<UserPropertyField['attrs']> = {}): UserPropertyField => ({
id: 'cpa-1',
name: 'department',
type: 'text',
group_id: 'custom_profile_attributes',
create_at: 0,
update_at: 0,
delete_at: 0,
created_by: '',
updated_by: '',
target_id: '',
target_type: '',
object_type: '',
attrs: {
sort_order: 0,
visibility: 'when_set',
value_type: '',
...overrides,
},
});
test('should render CPA label using display_name', async () => {
const cpaField = buildCPAField({display_name: 'Engineering Department'});
const props = {
...defaultProps,
customProfileAttributeFields: [cpaField],
getCustomProfileAttributeFields: jest.fn().mockResolvedValue({data: [cpaField]}),
};
renderWithContext(<SystemUserDetail {...props}/>);
await waitForLoadingToFinish();
const labelEl = await screen.findByTestId('user-detail-custom-attribute-label-cpa-1');
expect(labelEl).toHaveTextContent('Engineering Department');
expect(labelEl).not.toHaveTextContent('department');
});
test('should fall back to name when display_name is empty', async () => {
const cpaField = buildCPAField({display_name: ''});
const props = {
...defaultProps,
customProfileAttributeFields: [cpaField],
getCustomProfileAttributeFields: jest.fn().mockResolvedValue({data: [cpaField]}),
};
renderWithContext(<SystemUserDetail {...props}/>);
await waitForLoadingToFinish();
const labelEl = await screen.findByTestId('user-detail-custom-attribute-label-cpa-1');
expect(labelEl).toHaveTextContent('department');
});
});
});
describe('getUserAuthenticationTextField', () => {
@@ -43,6 +43,7 @@ import ShieldOutlineIcon from 'components/widgets/icons/shield_outline_icon';
import LoadingSpinner from 'components/widgets/loading/loading_spinner';
import {Constants, ModalIdentifiers} from 'utils/constants';
import {getUserPropertyFieldLabel} from 'utils/properties';
import {validHttpUrl} from 'utils/url';
import {toTitleCase} from 'utils/utils';
@@ -653,11 +654,12 @@ export class SystemUserDetail extends PureComponent<Props, State> {
<label
key={field.id}
className='cpa-field'
data-testid={`user-detail-custom-attribute-label-${field.id}`}
>
<FormattedMessage
id='admin.userManagement.userDetail.cpaField'
defaultMessage='{fieldName}'
values={{fieldName: field.name}}
values={{fieldName: getUserPropertyFieldLabel(field)}}
/>
{fieldContent}
{syncIndicator}
@@ -278,4 +278,83 @@ describe('components/ProfilePopoverCustomAttributes', () => {
// The attribute with empty value and 'when_set' visibility should not be rendered
expect(screen.queryByText('Text Attribute')).not.toBeInTheDocument();
});
test('should render display_name as the visible label when set', () => {
const state = {
...baseState,
entities: {
...baseState.entities,
general: {
...baseState.entities.general,
customProfileAttributes: {
...baseState.entities.general.customProfileAttributes,
text_attribute_id: {
...textAttribute,
attrs: {
...textAttribute.attrs,
display_name: 'Friendly Text Label',
},
},
},
},
},
};
const store = mockStore(state);
renderWithContext(
<Provider store={store}>
<ProfilePopoverCustomAttributes {...baseProps}/>
</Provider>,
);
const titleEl = screen.getByText('Friendly Text Label');
expect(titleEl).toHaveClass('user-popover__subtitle');
expect(titleEl.id).toBe('user-popover__custom_attributes-title-text_attribute_id');
expect(screen.queryByText('Text Attribute')).not.toBeInTheDocument();
});
test('should fall back to name when display_name is unset or whitespace-only', () => {
const buildState = (displayName: string | undefined) => ({
...baseState,
entities: {
...baseState.entities,
general: {
...baseState.entities.general,
customProfileAttributes: {
...baseState.entities.general.customProfileAttributes,
text_attribute_id: {
...textAttribute,
attrs: {
...textAttribute.attrs,
display_name: displayName,
},
},
},
},
},
});
const {unmount} = renderWithContext(
<Provider store={mockStore(buildState(undefined))}>
<ProfilePopoverCustomAttributes {...baseProps}/>
</Provider>,
);
const undefinedTitle = screen.getByText('Text Attribute');
expect(undefinedTitle.tagName).toBe('STRONG');
expect(undefinedTitle.id).toBe('user-popover__custom_attributes-title-text_attribute_id');
unmount();
renderWithContext(
<Provider store={mockStore(buildState(' '))}>
<ProfilePopoverCustomAttributes {...baseProps}/>
</Provider>,
);
const whitespaceTitle = screen.getByText('Text Attribute');
expect(whitespaceTitle.tagName).toBe('STRONG');
expect(whitespaceTitle.id).toBe('user-popover__custom_attributes-title-text_attribute_id');
});
});
@@ -10,6 +10,8 @@ import {getCustomProfileAttributeValues} from 'mattermost-redux/actions/users';
import {getCustomProfileAttributes} from 'mattermost-redux/selectors/entities/general';
import {getUser} from 'mattermost-redux/selectors/entities/users';
import {getUserPropertyFieldLabel} from 'utils/properties';
import type {GlobalState} from 'types/store';
import ProfilePopoverPhone from './profile_popover_phone';
@@ -83,7 +85,7 @@ const ProfilePopoverCustomAttributes = ({
id={`user-popover__custom_attributes-title-${attribute.id}`}
className='user-popover__subtitle'
>
{attribute.name}
{getUserPropertyFieldLabel(attribute)}
</strong>
{(attribute.type === 'multiselect' || attribute.type === 'select') && (
<ProfilePopoverSelectAttribute
@@ -941,4 +941,80 @@ describe('components/user_settings/general/UserSettingsGeneral', () => {
expect(screen.getByRole('textbox', {name: normalAttribute.name})).toBeInTheDocument();
expect(screen.queryByText('This field is managed by a plugin and cannot be edited.')).not.toBeInTheDocument();
});
test('should render section title using display_name when collapsed', async () => {
const attributeWithDisplayName: UserPropertyField = {
...customProfileAttribute,
attrs: {
...customProfileAttribute.attrs,
display_name: 'Friendly Display Name',
},
};
const props = {
...requiredProps,
enableCustomProfileAttributes: true,
customProfileAttributeFields: [attributeWithDisplayName],
user: {...user, custom_profile_attributes: {field1: 'FieldOneValue'}},
activeSection: '',
};
const {container} = renderWithContext(<UserSettingsGeneral {...props}/>);
expect(await screen.findByRole('button', {name: 'Friendly Display Name Edit'})).toBeInTheDocument();
const titleHeading = container.querySelector('#customAttribute_field1Title');
expect(titleHeading).not.toBeNull();
expect(titleHeading).toHaveTextContent('Friendly Display Name');
expect(screen.queryByRole('button', {name: `${attributeWithDisplayName.name} Edit`})).not.toBeInTheDocument();
});
test('should render SettingItemMax title using display_name when expanded', async () => {
const attributeWithDisplayName: UserPropertyField = {
...customProfileAttribute,
attrs: {
...customProfileAttribute.attrs,
display_name: 'Friendly Display Name',
},
};
const props = {
...requiredProps,
enableCustomProfileAttributes: true,
customProfileAttributeFields: [attributeWithDisplayName],
user: {...user},
activeSection: 'customAttribute_field1',
};
const {container} = renderWithContext(<UserSettingsGeneral {...props}/>);
const maxTitle = await screen.findByRole('heading', {name: 'Friendly Display Name'});
expect(maxTitle).toHaveAttribute('id', 'settingTitle');
const controlLabels = container.querySelectorAll('label.control-label');
expect(controlLabels.length).toBeGreaterThan(0);
expect(Array.from(controlLabels).some((el) => el.textContent === 'Friendly Display Name')).toBe(true);
});
test('should set input aria-label using display_name when expanded', async () => {
const attributeWithDisplayName: UserPropertyField = {
...customProfileAttribute,
attrs: {
...customProfileAttribute.attrs,
display_name: 'Friendly Display Name',
},
};
const props = {
...requiredProps,
enableCustomProfileAttributes: true,
customProfileAttributeFields: [attributeWithDisplayName],
user: {...user},
activeSection: 'customAttribute_field1',
};
renderWithContext(<UserSettingsGeneral {...props}/>);
expect(await screen.findByRole('textbox', {name: 'Friendly Display Name'})).toBeInTheDocument();
expect(screen.queryByRole('textbox', {name: attributeWithDisplayName.name})).not.toBeInTheDocument();
});
});
@@ -27,6 +27,7 @@ import Input from 'components/widgets/inputs/input/input';
import LoadingWrapper from 'components/widgets/loading/loading_wrapper';
import {AnnouncementBarMessages, AnnouncementBarTypes, AcceptedProfileImageTypes, Constants, ValidationErrors} from 'utils/constants';
import {getUserPropertyFieldLabel} from 'utils/properties';
import {validHttpUrl} from 'utils/url';
import * as Utils from 'utils/utils';
@@ -1547,7 +1548,7 @@ export class UserSettingsGeneralTab extends PureComponent<Props, State> {
// Only render inputs if the field is not synced, admin-managed, or protected
if (!isSynced && !isAdminManaged && !isProtected) {
let attributeLabel: JSX.Element | string = (
attribute.name
getUserPropertyFieldLabel(attribute)
);
if (this.props.isMobileView) {
attributeLabel = '';
@@ -1598,7 +1599,7 @@ export class UserSettingsGeneralTab extends PureComponent<Props, State> {
maxLength={Constants.MAX_CUSTOM_ATTRIBUTE_LENGTH}
autoCapitalize='off'
onFocus={Utils.moveCursorToEnd}
aria-label={attribute.name}
aria-label={getUserPropertyFieldLabel(attribute)}
validate={validate}
/>
</div>
@@ -1623,7 +1624,7 @@ export class UserSettingsGeneralTab extends PureComponent<Props, State> {
max = (
<SettingItemMax
key={'settingItemMax_' + attribute.id}
title={attribute.name}
title={getUserPropertyFieldLabel(attribute)}
inputs={inputs}
submit={submit}
saving={this.state.sectionIsSaving}
@@ -1670,7 +1671,7 @@ export class UserSettingsGeneralTab extends PureComponent<Props, State> {
key={'settingItem_' + attribute.id}
active={active}
areAllSectionsInactive={this.props.activeSection === ''}
title={attribute.name}
title={getUserPropertyFieldLabel(attribute)}
describe={describe}
section={sectionName}
updateSection={this.updateSection}
+5 -2
View File
@@ -3129,7 +3129,6 @@
"admin.system_properties.user_properties.dotmenu.ad_ldap.modal.helpText": "The attribute in the AD/LDAP server used to sync as a custom attribute in user's profile in Mattermost.",
"admin.system_properties.user_properties.dotmenu.delete.label": "Delete attribute",
"admin.system_properties.user_properties.dotmenu.duplicate.label": "Duplicate attribute",
"admin.system_properties.user_properties.dotmenu.duplicate.name_copy": "{fieldName} (copy)",
"admin.system_properties.user_properties.dotmenu.editable_by_users.label": "Editable by users",
"admin.system_properties.user_properties.dotmenu.saml.edit_link.label": "Edit SAML link",
"admin.system_properties.user_properties.dotmenu.saml.link_property.label": "Link attribute to SAML",
@@ -3141,8 +3140,11 @@
"admin.system_properties.user_properties.dotmenu.visibility.when_set.label": "Hide when empty",
"admin.system_properties.user_properties.subtitle": "Attributes will be shown in user profile and can be used in access control policies.",
"admin.system_properties.user_properties.table.actions": "Actions",
"admin.system_properties.user_properties.table.display_name_header": "Display Name",
"admin.system_properties.user_properties.table.display_name.input.label": "Display Name",
"admin.system_properties.user_properties.table.filter_type": "Attribute type",
"admin.system_properties.user_properties.table.property": "Attribute",
"admin.system_properties.user_properties.table.identifier.tooltip": "Common Expression Language (CEL) identifier used in policies. Only letters, digits, and underscores allowed. Must start with a letter or underscore. Reserved CEL words are not allowed.",
"admin.system_properties.user_properties.table.name": "Name",
"admin.system_properties.user_properties.table.property_name.input.name": "Attribute Name",
"admin.system_properties.user_properties.table.select_type.email": "Email",
"admin.system_properties.user_properties.table.select_type.multi_select": "Multi-select",
@@ -3151,6 +3153,7 @@
"admin.system_properties.user_properties.table.select_type.text": "Text",
"admin.system_properties.user_properties.table.select_type.url": "URL",
"admin.system_properties.user_properties.table.type": "Type",
"admin.system_properties.user_properties.table.validation.name_invalid_cel": "Identifier must start with a letter or underscore and contain only letters, numbers, and underscores. Reserved CEL words are not allowed.",
"admin.system_properties.user_properties.table.validation.name_required": "Please enter an attribute name.",
"admin.system_properties.user_properties.table.validation.name_taken": "Attribute name already taken.",
"admin.system_properties.user_properties.table.validation.name_unique": "Attribute names must be unique.",
@@ -0,0 +1,276 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {
CPA_FIELD_NAME_PATTERN,
CPA_FIELD_NAME_RESERVED_WORDS,
filterCELIdentifier,
getUserPropertyFieldLabel,
slugifyForCEL,
validateCPAFieldName,
} from './properties';
describe('utils/properties', () => {
describe('getUserPropertyFieldLabel', () => {
const base = {name: 'dept_head'};
test('returns display_name when set and non-empty', () => {
expect(getUserPropertyFieldLabel({
...base,
attrs: {sort_order: 0, visibility: 'always', value_type: '', display_name: 'Department Head'},
})).toBe('Department Head');
});
test('returns display_name trimmed when surrounded by whitespace', () => {
expect(getUserPropertyFieldLabel({
...base,
attrs: {sort_order: 0, visibility: 'always', value_type: '', display_name: ' Department Head '},
})).toBe('Department Head');
});
test('falls back to name when display_name is undefined', () => {
expect(getUserPropertyFieldLabel({
...base,
attrs: {sort_order: 0, visibility: 'always', value_type: ''},
})).toBe('dept_head');
});
test('falls back to name when display_name is the empty string', () => {
expect(getUserPropertyFieldLabel({
...base,
attrs: {sort_order: 0, visibility: 'always', value_type: '', display_name: ''},
})).toBe('dept_head');
});
test('falls back to name when display_name is whitespace-only', () => {
expect(getUserPropertyFieldLabel({
...base,
attrs: {sort_order: 0, visibility: 'always', value_type: '', display_name: ' '},
})).toBe('dept_head');
});
test('falls back to name when attrs is undefined (defensive)', () => {
// Type cast needed since UserPropertyField.attrs is normally required;
// this covers runtime API responses that may omit the field.
expect(getUserPropertyFieldLabel({
name: 'dept_head',
attrs: undefined as any,
})).toBe('dept_head');
});
test('returns non-ASCII display_name verbatim', () => {
expect(getUserPropertyFieldLabel({
name: 'employee_number',
attrs: {sort_order: 0, visibility: 'always', value_type: '', display_name: '员工编号'},
})).toBe('员工编号');
expect(getUserPropertyFieldLabel({
name: 'preferences',
attrs: {sort_order: 0, visibility: 'always', value_type: '', display_name: 'Préférences'},
})).toBe('Préférences');
});
});
});
describe('CPA field name constants - cross-stack drift guard', () => {
it('CPA_FIELD_NAME_PATTERN string matches the Go source exactly', () => {
if (CPA_FIELD_NAME_PATTERN.source !== '^[A-Za-z_][A-Za-z0-9_]*$') {
throw new Error('Update the TS constant to match the Go source at server/public/model/custom_profile_attributes.go');
}
expect(CPA_FIELD_NAME_PATTERN.source).toBe('^[A-Za-z_][A-Za-z0-9_]*$');
});
it('CPA_FIELD_NAME_RESERVED_WORDS contains exactly 21 words matching the Go source', () => {
const expected = new Set([
'true', 'false', 'null',
'in', 'as',
'break', 'const', 'continue', 'else',
'for', 'function', 'if', 'import',
'let', 'loop', 'package', 'namespace',
'return', 'var', 'void', 'while',
]);
if (CPA_FIELD_NAME_RESERVED_WORDS.size !== 21) {
throw new Error('Update the TS constant to match the Go source at server/public/model/custom_profile_attributes.go');
}
expect(CPA_FIELD_NAME_RESERVED_WORDS.size).toBe(21);
for (const word of expected) {
if (!CPA_FIELD_NAME_RESERVED_WORDS.has(word)) {
throw new Error('Update the TS constant to match the Go source at server/public/model/custom_profile_attributes.go');
}
expect(CPA_FIELD_NAME_RESERVED_WORDS.has(word)).toBe(true);
}
for (const word of CPA_FIELD_NAME_RESERVED_WORDS) {
if (!expected.has(word)) {
throw new Error('Update the TS constant to match the Go source at server/public/model/custom_profile_attributes.go');
}
expect(expected.has(word)).toBe(true);
}
});
});
describe('validateCPAFieldName', () => {
const validCases = [
['simple lowercase', 'department'],
['leading underscore', '_private'],
['uppercase start', 'Department'],
['single uppercase', 'A1'],
['underscore separator', 'a_b_c'],
['all uppercase', 'DEPT'],
['case-sensitive: IN is not reserved', 'IN'],
['case-sensitive: In is not reserved', 'In'],
['single lowercase letter', 'a'],
['single underscore', '_'],
['single uppercase letter', 'A'],
['reserved word as prefix', 'trueish'],
['reserved word as suffix', 'my_null'],
['255-rune name at exactly max length', 'a'.repeat(255)],
] as const;
test.each(validCases)('%s: %s -> null', (_label, input) => {
expect(validateCPAFieldName(input)).toBeNull();
});
const invalidCharsetCases = [
['space in name', 'My Field'],
['leading digit', '7department'],
['hyphen', 'foo-bar'],
['emoji', '🎯'],
['empty string', ''],
['trailing space', 'name '],
['non-ASCII letter', 'départment'],
['whitespace only', ' '],
['dot separator', 'foo.bar'],
['slash', 'foo/bar'],
] as const;
test.each(invalidCharsetCases)('%s: %s -> invalid_charset', (_label, input) => {
expect(validateCPAFieldName(input)).toEqual({kind: 'invalid_charset'});
});
const reservedWords = [
'true', 'false', 'null',
'in', 'as',
'break', 'const', 'continue', 'else',
'for', 'function', 'if', 'import',
'let', 'loop', 'package', 'namespace',
'return', 'var', 'void', 'while',
] as const;
test.each(reservedWords)('reserved word: %s -> reserved_word', (word) => {
expect(validateCPAFieldName(word)).toEqual({kind: 'reserved_word', word});
});
it('254-rune name -> null', () => {
expect(validateCPAFieldName('a'.repeat(254))).toBeNull();
});
it('255-rune name -> null (exactly at cap)', () => {
expect(validateCPAFieldName('a'.repeat(255))).toBeNull();
});
it('256-rune name -> too_long', () => {
expect(validateCPAFieldName('a'.repeat(256))).toEqual({kind: 'too_long', max: 255});
});
});
describe('slugifyForCEL', () => {
it('already snake_case identifier passes through unchanged', () => {
expect(slugifyForCEL('dept_head')).toBe('dept_head');
});
it('spaces are replaced with underscores and lowercased', () => {
expect(slugifyForCEL('My Field')).toBe('my_field');
});
it('hyphens are replaced with underscores and lowercased', () => {
expect(slugifyForCEL('foo-Bar')).toBe('foo_bar');
});
it('camelCase is converted to snake_case', () => {
expect(slugifyForCEL('myFieldName')).toBe('my_field_name');
});
it('PascalCase is converted to snake_case', () => {
expect(slugifyForCEL('MyField')).toBe('my_field');
});
it('consecutive uppercase acronyms split before final word', () => {
expect(slugifyForCEL('XMLParser')).toBe('xml_parser');
expect(slugifyForCEL('HTTPServerError')).toBe('http_server_error');
});
it('all-uppercase token is lowercased without inserting separators', () => {
expect(slugifyForCEL('DEPT')).toBe('dept');
});
it('digit-letter boundaries do not insert separators', () => {
expect(slugifyForCEL('field2name')).toBe('field2name');
expect(slugifyForCEL('Field2Name')).toBe('field2_name');
});
it('leading digit gets underscore prefix', () => {
expect(slugifyForCEL('7department')).toBe('_7department');
});
it('leading underscore is preserved', () => {
expect(slugifyForCEL('_Private')).toBe('_private');
});
it('empty string returns _copy', () => {
expect(slugifyForCEL('')).toBe('_copy');
});
it('all-punctuation string returns _copy', () => {
expect(slugifyForCEL('---')).toBe('_copy');
});
it('non-ASCII letters are replaced with underscores', () => {
expect(slugifyForCEL('Préférences')).toBe('pr_f_rences');
});
it('result always matches CPA_FIELD_NAME_PATTERN', () => {
const inputs = [
'My Field',
'foo-bar',
'7dept',
'',
'---',
'valid_name',
'DEPT',
'MyField',
'XMLParser',
'myFieldName',
'_Private',
'Préférences',
];
for (const input of inputs) {
const result = slugifyForCEL(input);
expect(CPA_FIELD_NAME_PATTERN.test(result)).toBe(true);
}
});
});
describe('filterCELIdentifier', () => {
const cases = [
['strips spaces', 'my field', 'myfield'],
['strips dashes', 'my-field', 'myfield'],
['prefixes leading digit', '7department', '_7department'],
['passes valid identifier through', 'my_field_2', 'my_field_2'],
['preserves case', 'MyField', 'MyField'],
['strips emoji', 'field🎉', 'field'],
['empty string stays empty', '', ''],
['all digits prefixed', '123', '_123'],
['all punctuation becomes empty', '!@#', ''],
['preserves multiple underscores', 'my__field', 'my__field'],
['leading underscore preserved', '_private', '_private'],
] as const;
test.each(cases)('%s: %s → %s', (_label, input, expected) => {
expect(filterCELIdentifier(input)).toBe(expected);
});
});
+129
View File
@@ -0,0 +1,129 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import type {UserPropertyField} from '@mattermost/types/properties';
/**
* Returns the user-facing label for a CPA field.
* Prefers attrs.display_name (trimmed); falls back to name for legacy
* fields that have not been backfilled yet.
*
* Use for ALL human-readable label rendering (visible text, aria-label,
* title, section headings, etc.).
*
* Do NOT use for:
* - CEL expression construction use field.name
* - React keys use field.id or field.name
* - HTML element ids use field.name or field.id
* - Comparison with currentAttribute in ABAC selectors use field.name
*/
export function getUserPropertyFieldLabel(
field: Pick<UserPropertyField, 'name' | 'attrs'>,
): string {
const displayName = field.attrs?.display_name?.trim();
return displayName || field.name;
}
// SOURCE OF TRUTH: server/public/model/custom_profile_attributes.go lines 81-97
// CPAFieldNamePattern and CPAFieldNameReservedWords are Go->TS transcriptions.
// If the Go source changes, update BOTH the regex and the Set below, then update
// the hard-coded assertions in properties.test.ts (describe 'CPA field name constants').
// DO NOT change these constants without a corresponding server-side change.
//
// Grandfather contract:
// CPA name validation only fires when `name` changes from its initial server-persisted value.
// After a successful rename, `current.data[field.id].name` is refreshed by the table's
// readIO.setData(newData) call, which moves the field into the strictly-validated regime
// for all subsequent edits. The regression guard for this contract is in
// user_properties_table.test.tsx - search for 'rename of legacy field clears grandfather'.
/**
* Mirrors server CPAFieldNamePattern (^[A-Za-z_][A-Za-z0-9_]*$).
* Source: server/public/model/custom_profile_attributes.go:81
*/
export const CPA_FIELD_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/;
/**
* Mirrors server CPAFieldNameReservedWords.
* Source: server/public/model/custom_profile_attributes.go:90-97
* 21 CEL keywords. Case-sensitive: only lowercase forms are reserved.
*/
export const CPA_FIELD_NAME_RESERVED_WORDS = new Set<string>([
'true', 'false', 'null',
'in', 'as',
'break', 'const', 'continue', 'else',
'for', 'function', 'if', 'import',
'let', 'loop', 'package', 'namespace',
'return', 'var', 'void', 'while',
]);
/** Max runes for a CPA field name. Mirrors server PropertyFieldNameMaxRunes. */
export const CPA_FIELD_NAME_MAX_RUNES = 255;
/**
* Strips characters that are not valid in a CEL identifier and
* prefixes a leading digit with underscore. Unlike slugifyForCEL,
* this does NOT collapse/trim underscores it is designed for
* live keystroke filtering where the user controls spacing.
*/
export function filterCELIdentifier(input: string): string {
let stripped = input.replace(/[^A-Za-z0-9_]/g, '');
if (stripped.length > 0 && (/^[0-9]/).test(stripped)) {
stripped = '_' + stripped;
}
return stripped;
}
export type CPAFieldNameValidationError =
| {kind: 'invalid_charset'}
| {kind: 'reserved_word'; word: string}
| {kind: 'too_long'; max: number};
/**
* Client-side mirror of server ValidateCPAFieldName.
* Returns null when the name is valid; returns an error descriptor otherwise.
*
* Length is checked here (against CPA_FIELD_NAME_MAX_RUNES = 255) even though
* the server's ValidateCPAFieldName does not - this provides an early guard
* matching the server's total rejection behavior.
*
* Lenient grandfather: callers must only invoke this when field.name has
* changed from its server-persisted value (mirrors App.PatchCPAField behavior).
*/
export function validateCPAFieldName(name: string): CPAFieldNameValidationError | null {
if ([...name].length > CPA_FIELD_NAME_MAX_RUNES) {
return {kind: 'too_long', max: CPA_FIELD_NAME_MAX_RUNES};
}
if (!CPA_FIELD_NAME_PATTERN.test(name)) {
return {kind: 'invalid_charset'};
}
if (CPA_FIELD_NAME_RESERVED_WORDS.has(name)) {
return {kind: 'reserved_word', word: name};
}
return null;
}
/**
* Converts an arbitrary string into a snake_case CEL-safe identifier for
* use as a duplicate-field base name. Camel/PascalCase boundaries are
* converted to underscore separators (e.g. 'MyField' -> 'my_field',
* 'XMLParser' -> 'xml_parser'), the result is lowercased, and any
* remaining non-identifier characters are replaced with underscores.
* A leading digit is prefixed with underscore. Consecutive underscores
* collapse to one and trailing underscores are trimmed (a leading
* underscore is preserved). Result is guaranteed to match
* CPA_FIELD_NAME_PATTERN if the input is non-empty; returns '_copy' if
* the entire input normalizes to empty.
*/
export function slugifyForCEL(name: string): string {
let slug = name.
replace(/([a-z0-9])([A-Z])/g, '$1_$2').
replace(/([A-Z]+)([A-Z][a-z])/g, '$1_$2').
toLowerCase().
replace(/[^a-z0-9_]/g, '_');
if ((/^[0-9]/).test(slug)) {
slug = '_' + slug;
}
slug = slug.replace(/_+/g, '_').replace(/_+$/, '');
return slug || '_copy';
}
+1
View File
@@ -84,6 +84,7 @@ export type UserPropertyField = PropertyField & {
protected?: boolean;
source_plugin_id?: string;
access_mode?: '' | 'source_only' | 'shared_only';
display_name?: string;
};
};