mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-01 15:00:08 +08:00
Guillermo Vayá
8cd48d4651
* [MM-67880] Add /mobile-logs slash command with E2E tests Add a new /mobile-logs slash command that allows users to manage the attach_app_logs preference for themselves or other users (admin-only). Includes unit tests for all code paths and Playwright E2E tests covering self-management, admin cross-user management, permission denial, and error handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix i18n error key suffixes and format E2E tests Rename error i18n keys to use .app_error suffix matching upstream convention (no_permission, update_error, user_not_found). Run prettier on the E2E test file to fix formatting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix E2E test type error: use getUserPreferences instead of getMyPreferences The getAttachLogsPreference helper was using getMyPreferences() which returns PreferenceType (not an array), causing TS2345 errors. Switch to getUserPreferences(userId) which returns the expected array type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [MM-67880] Move unreachable usage fallback into switch default case The return after the switch was unreachable because action is validated earlier to be "on", "off", or "status". Move it into an explicit default case with a defensive comment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * allow up to 2 arguments * Add audit logging for mobile logs slash command actions Implement a new function to log audit records when users enable or disable the attach_app_logs preference via the /mobile-logs command. This includes capturing relevant metadata such as user IDs, session information, and the action taken. The logging occurs in both the enable and disable command paths, enhancing traceability and accountability for user preference changes. * Enhance mobile logs command to handle cross-user permission checks Add a new response function for cases where a regular user attempts to access mobile log settings for another user, ensuring they receive a neutral error message instead of specific user information. Update the command logic to incorporate this response for both nonexistent users and deactivated accounts. Additionally, modify related tests and internationalization keys to reflect these changes, improving security and user experience. * Update E2E test for /mobile-logs command to verify permission denial for nonexistent users Enhance the existing E2E test for the /mobile-logs command by adding assertions to check that users receive a permission denial message when attempting to change mobile log settings for a nonexistent user. This improves test coverage and ensures proper error handling in the application. * update i18n strings --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Mattermost Build <build@mattermost.com>
Server Channels Review Guidelines
When reviewing or writing code in the server channels package, focus on SQL query performance and API layer efficiency.
SQL Store Layer
- Run
EXPLAIN ANALYZEon new or modified queries against a large dataset before merging. A query that performs well on a 12M-post database may degrade significantly at 100M+ posts. - Watch for sequential scans on large tables. Ensure appropriate indexes exist for new query patterns.
- When adding new queries to the store, check whether an existing query already fetches the needed data. Avoid duplicate round trips to the database.
API Layer
- Minimize database round trips. If an endpoint calls a
Getfollowed by aDeleteon the same row, consider usingDELETE ... RETURNINGto combine them into a single query. - Don't add queries that are unnecessary for the operation. The most efficient work is the work you don't do.
- When adding new API endpoints, add them to the load test tooling so performance can be validated under realistic concurrency.
Permissions and Security
- Verify that new endpoints enforce appropriate permissions. Rely on the dedicated security review for thorough coverage, but flag anything obviously missing (e.g., an endpoint that skips permission checks entirely).