[pull] master from mattermost:master - #920
Merged
Merged
Conversation
…tup) (#37966) * [MM-69646] Disallow MoveThreadsEnabled feature flag Reject the MoveThreadsEnabled feature flag during config validation so the server fails to start while it is enabled. The feature is being retired in favor of Wrangler and will be removed later. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69646] Cover nil FeatureFlags guard in config validation test Co-authored-by: mattermost-code <matty-code@mattermost.com> * Move MoveThreadsEnabled comment into isValid method body Keep isValid's doc comment generic since it will validate more flag combinations in the future, and place the MoveThreadsEnabled-specific rationale next to the actual flag check. * [MM-69646] Update TestMoveThread for retired MoveThreadsEnabled flag Config.IsValid now rejects enabling MoveThreadsEnabled, so the move-thread API stays disabled. Replace the enabled-path suite with assertions that the flag cannot be turned on and MoveThread returns 501. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69646] Stop forcing MoveThreadsEnabled in e2e environments E2E was setting MM_FEATUREFLAGS_MOVETHREADSENABLED=true, which now fails Config.IsValid and prevents the test server from starting. Remove the override and skip Cypress move-thread specs that require the retired flag. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69646] Skip TestMoveThread instead of asserting disabled flag Mirror the E2E describe.skip approach: retain the original TestMoveThread body and skip it at the top, since MoveThreadsEnabled is retired and rejected by Config.IsValid. * [MM-69646] Park cursor away from post dot menu in edit_file_attachment specs --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com> Co-authored-by: Mattermost Build <build@mattermost.com> Co-authored-by: Jesse Hallam <jesse@mattermost.com>
…turning -1 for other users (#37505) * Omit sanitized channel member timestamps from JSON The channel member sanitization introduced in #33835 replaced other users' LastViewedAt and LastUpdateAt with -1, which clients decode as Dec 31 1969. Serialize the sanitized sentinel as an absent field instead so the API no longer returns an invalid timestamp for other users. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Add tests and API docs for omitted sanitized member timestamps Verify at the JSON layer that last_viewed_at and last_update_at are omitted for other users' memberships (across the channel and user endpoints) while remaining present for the requester, including a legitimate zero timestamp. Document the omission in the API spec. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Strengthen sanitized-timestamp test coverage Cover the NDJSON streaming branch of getChannelMembersForUser and the getChannelMembersForTeamForUser endpoint, assert the requester's own timestamps are valid (not the sentinel), use the sanitizedTimestamp constant, and note the ChannelMemberForExport marshaling footgun. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Marshal team data via a typed struct in ChannelMemberWithTeamData Co-authored-by: mattermost-code <matty-code@mattermost.com> * Avoid shadowing err in ChannelMemberWithTeamData.MarshalJSON Co-authored-by: mattermost-code <matty-code@mattermost.com> * Use omitzero tags to omit sanitized member timestamps Replace the custom ChannelMember/ChannelMemberWithTeamData MarshalJSON round-trip with the Go 1.24 omitzero tag on LastViewedAt/LastUpdateAt. SanitizeForCurrentUser now zeroes another user's timestamps so they are omitted from API responses, per reviewer feedback. * Give current user a real last_viewed_at in sanitization test With omitzero, a zero last_viewed_at is legitimately omitted. Have user2 post an unread message and the current user view the channel so the current-user assertions verify a genuine timestamp survives sanitization. * Use -1 sentinel for sanitized member timestamps with single-pass marshal A last_viewed_at of 0 legitimately means "never viewed", so it cannot double as the sanitization sentinel. Restore the -1 sentinel and omit it during serialization via shadowing pointer fields, avoiding the previous marshal/unmarshal/marshal round-trip. * Clarify ChannelMember.MarshalJSON doc comment per review feedback * Address PR feedback: 0 answered, 4 resolved, 0 declined - Simplify sanitizedTimestamp and SanitizeForCurrentUser doc comments per review - Document that new ChannelMemberWithTeamData fields must be added to MarshalJSON - Add round-trip test guarding against fields dropped by MarshalJSON * Address PR feedback: remove round-trip MarshalJSON test The round-trip test did not guard against forgetting to add a new field to MarshalJSON, since the same field would also be missing from the test. * Address PR feedback: assert legitimate zero last_update_at is serialized * Mark sanitized channel member timestamp fields as nullable in OpenAPI spec --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com> Co-authored-by: Mattermost Build <build@mattermost.com>
* [MM-69865] Enable the Delete row action on Manage Attributes The kebab menu's Delete was stubbed as disabled + "Coming soon". Wire it to a confirmation modal and the existing DELETE property-field endpoint, dispatching PROPERTY_FIELD_DELETED so the row leaves via Redux rather than local component state. The modal closes immediately on confirm instead of freezing behind a spinner, so failures surface in an AlertBanner above the table rather than inside a modal that is already gone. A 409 (the server refuses while live linked dependents exist) gets its own message instead of the generic one -- the wording says "other attributes are still linked to it", matching what CountLinkedFields actually counts, not the AD/LDAP and SAML source links, which are plain attrs on the field itself. Delete stays disabled on plugin-owned rows, relabelled "Plugin-managed" so the reason is visible rather than the item silently doing nothing. E2E covers confirm, cancel, and the 409 branch against a real server response: the linked dependent is seeded through the API, which needs a non-template object type because IsValid rejects a template field carrying a linked_field_id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Resolve server-only plugin names instead of showing the raw plugin ID getPluginDisplayName only read state.plugins.plugins, which holds manifests for plugins that shipped a webapp bundle and registered themselves in the browser. A server-only plugin is never in that map, so every caller fell through to the fallback and rendered the bare ID -- "com.mattermost.gahelper" in the Manage Attributes Source column. Consult state.entities.admin.pluginStatuses as a second source before giving up on the ID. That map covers every installed plugin, server-only included, and is empty for non-admins, so the non-admin callers (user_settings_general, integrations/bots) are unaffected. Nothing on the Manage Attributes page loaded those statuses, so fetch them there -- once, and only when a plugin-owned row is actually present. The result is deliberately not awaited: a failure just leaves the column showing the ID it was already showing. Also fixes the same latent fallback on custom_profile_attributes, user_properties_values, and system_user_detail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Keep e2e attribute names under the 40-char Unique name cap Every test in the "create attribute" block built expectedName from a long prefix plus a 13-digit Date.now(), overshooting the Unique name input's Constants.MAX_CUSTOM_ATTRIBUTE_NAME_LENGTH (40). The derived slug truncated silently, so "playwright_created_attribute_<13 digits>" (42 chars) was asserted against the 40 chars actually rendered. The two linked-source tests only use expectedName for cleanup, so instead of failing they quietly leaked their seeded field onto the shared server on every run. Shorten the prefixes so the derived slug fits. The whole file is mode: 'serial', so these failures also skipped every test after them -- including the delete coverage at the end of the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Announce the delete error banner to assistive tech The banner was mounted together with its message, and an alert inserted into the DOM at the same moment as its text is not reliably announced -- so a screen-reader user got silence when a delete they had just confirmed failed. Keep the region mounted and swap only its content, matching the reason attribute_external_source.tsx already keeps its own status region mounted rather than rendering it alongside the announcement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [MM-69865] Allow deleting a plugin-owned attribute once its plugin is uninstalled Delete was permanently disabled on plugin-owned rows, but the server only protects such a field while its source plugin is still installed: checkFieldDeleteAccess allows the delete once the plugin is gone, which is how an admin cleans up what an uninstalled plugin left behind. Reuse the User Attributes page's orphan check rather than growing a second one. isFieldOrphaned moves out of system_properties/orphaned_fields_utils.ts into utils/properties.ts, widened to PropertyField so both pages can use it, with the redux-aware hook in components/common/hooks/use_field_orphaned.ts. The hook unions admin.plugins and admin.pluginStatuses because the two attribute pages populate different slices, and a field should not read as orphaned merely because the page rendering it loaded only one of them. The delete confirmation now names the uninstalled plugin, since an admin has no other way to tell where the leftover attribute came from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix the delete-error scroll and the premature orphan check The error banner sits above the table, so a delete confirmed from a row further down left the failure off-screen. The existing scroll fired on the error state, which lands during the modal's fade-out: GenericModal passes restoreFocus, so react-bootstrap returns focus to the row's actions button on close, and focusing an off-screen element scrolls it back into view -- undoing the scroll every time. GenericModal also starts closing before it invokes handleConfirm, so the delete response can land either side of the fade and the error and the exit arrive in either order. Scroll only once both have landed, take focus on the banner without its own scroll, then scroll the console wrapper to the top. Focusing the banner also leaves keyboard and screen reader users at the error rather than back on a row button. Separately, gate the orphan check on the plugin inventory having settled. Both admin.plugins and admin.pluginStatuses start empty, and the fetch is dispatched from an effect, so an inventory that has not loaded is indistinguishable from one where nothing is installed -- which isFieldOrphaned reads as "every plugin-owned field is orphaned". A protected row briefly offered Delete behind a dialog wrongly claiming its plugin was uninstalled, which the server would then refuse anyway. Settled rather than resolved: a failed fetch still leaves the inventory as good as it will get, and staying false forever would strand genuine leftovers as undeletable. The same gap exists for the system_properties consumers of the hook, which never fetch the inventory at all; left alone here and noted on the hook. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…7759) * Drop top-level min_date/max_date/time_interval and allow_manual_time_entry; require datetime_config (and manual_time_entry). Update docs, tests, and e2e fixtures accordingly. * update important-upgrade-notes.rst per Doc Impact Analysis
* [MM-57814][MM-57815] Graduate user typing settings to Site Configuration > Posts Move ServiceSettings.EnableUserTypingMessages and ServiceSettings.TimeBetweenUserTypingUpdatesMilliseconds out of System Console > Experimental > Features into the Performance & Limits section of System Console > Site Configuration > Posts, and reclassify their access tags from experimental_features to site_posts (preserving write_restrictable and cloud_restrictable). The two settings stay adjacent, and the timeout remains disabled while typing messages are off. The timeout label now states its unit, since "User Typing Timeout" alone did not convey milliseconds. The i18n ids move from admin.experimental.* to the Posts page's admin.posts.* convention; the "E.g.: 5000" placeholder previously shared with the experimental user status and profile fetching poll interval is now defined once per setting. No config keys, defaults, or runtime behavior change. * [MM-57814][MM-57815] Assert user typing settings are searchable under Posts Searching the System Console for "typing" now also matches Site Configuration > Posts, guarding the new location of the user typing settings. Experimental Features still matches on unrelated help text about typing a tilde to trigger channel autocomplete. * [MM-57814][MM-57815] Move user typing settings docs out of Experimental Document "Enable user typing messages" and "User typing timeout" in the Posts section of the site configuration settings guide, and drop them from the experimental configuration settings guide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )