[pull] master from mattermost:master#714
Merged
Merged
Conversation
…es (#36075) * Create config-change-checker.yml * Create check_config_changes_ci.py * Update config-change-checker.yml * Update check_config_changes_ci.py * Update check_config_changes_ci.py * Update check_config_changes_ci.py * Update check_config_changes_ci.py * Update config-change-checker.yml * Update check_config_changes_ci.py * Update config-change-checker.yml * Update config.go * Fix check_api to detect multi-line and multi-method endpoints The previous implementation matched the .Handle(...).Methods(...) regex line-by-line against diff lines. This silently missed two real and common patterns in api4/: 1. Multi-line .Handle(...) declarations — e.g. group.go has 18 of them, where the path lives on one line and the wrapper/handler on the next. The regex never matched, so PRs adding such endpoints produced empty release-note entries. 2. Multi-method declarations like .Methods(http.MethodGet, http.MethodHead) (4 instances in file.go) — the old regex required a closing paren immediately after the first method. The fix: - Add a file_at(ref, path) helper that snapshots a file at a git ref via 'git show', so checkers can compare full file states instead of pattern-matching diff text. - Add _scan_endpoints() that whitespace-collapses the file before matching, letting the regex span what were originally multiple lines. - Loosen _HANDLE_RE to capture the methods list as a substring and extract individual HTTP verbs with a known-method allowlist, so multi-method declarations produce one entry per verb. - Switch check_api to set-diff (after - before) / (before - after) on the parsed endpoint sets. This also cleanly handles routes that move within a file (no fragile add/remove dedup needed). - Anchor the new/deleted file detection to '^new file mode \d+' to avoid false positives from stray text in source files. Made-with: Cursor * Track enclosing struct in check_config to avoid dedup collisions The previous check_config keyed its add/remove dedup on the bare field name. The dedup intent was to ignore fields that were merely reordered within config.go (which appear in the diff as both '-Foo' and '+Foo'). But because the key was just the field name, an unrelated rename in one struct could silently cancel out a real new field with the same name in a different struct. For example, in a single PR: - EnableFoo *bool // removed from ServiceSettings + EnableFooV2 *bool - EnableBar *bool // removed from EmailSettings + EnableFoo *bool // newly added — but wrongly cancelled below The dedup would see 'EnableFoo' in both lists and drop both entries, hiding the brand-new EmailSettings.EnableFoo from the release-note output. The fix tracks each field's enclosing struct using a brace-depth stack that walks the file at BASE_SHA and HEAD_SHA. Fields are keyed as (struct_name, field_name) tuples, so identically-named fields in different structs are distinct, and the dedup only collapses true reorderings. As a side benefit the rendered output is now 'StructName.FieldName' which is much more useful to reviewers. Switching to file-at-revision scanning + set diff also removes the custom dedup logic entirely — set arithmetic handles "moved within file" naturally. Made-with: Cursor * Switch remaining checkers to file-at-revision style; drop lines_by_sign check_audit_events and check_go_version still parsed +/- diff lines directly, with the same brittle dedup-and-cancel logic that was used in the previous check_config. After the previous two commits the rest of the file uses the file_at(ref, path) helper to compare full file states between BASE_SHA and HEAD_SHA, which: - removes the entire moved-within-file dedup dance (set arithmetic handles it for free), - aligns all four checkers on a single, easy-to-reason-about pattern, - is robust to whitespace-only or reordering edits in the watched files. For Dockerfile.buildenv the helper also avoids a subtle case where the old code only inspected +/- lines: an edit to an unrelated RUN line that didn't touch the FROM line could in theory leave both old_ver and new_ver as None even though the version was effectively unchanged. Reading the file at each revision compares the actual current and previous FROM line directly. The lines_by_sign helper now has no callers, so remove it. Made-with: Cursor * Update config.go * Update config.go * Update check_config_changes_ci.py * Update check_config_changes_ci.py * Update check_config_changes_ci.py * Update check_config_changes_ci.py * Tighten check_config_changes_ci.py: regex coverage + idempotency - Restore tolerant `_HANDLE_RE` so 2-arg wrappers (e.g. `api.APISessionRequired(handler, handlerParamFileAPI)`) are not silently dropped from the api4 endpoint scan; broaden the `.Methods(...)` capture so string-literal variants (`Methods("GET")`) work too. Filtering moves back to the `_HTTP_METHODS` allowlist in `_parse_methods` to keep stray identifiers from being treated as HTTP verbs. - Make `strip_old_note` also remove auto-generated lines that landed outside the ```release-note fence (the inject_note fallback paths) so reruns no longer accumulate duplicates when a PR has no fence. - Skip the GitHub PATCH when the PR description is already up to date, so every commit no longer triggers an unconditional write. - Wire up `check_go_version`'s `additions` path in `_format_lines` and `_AUTO_LINE_RE` so a freshly-added Dockerfile.buildenv emits a note. - Remove the now-dead `CheckResult.to_markdown` method (replaced by `_format_lines`). Made-with: Cursor * Restore ExperimentalSettings.EnableWatermark The field was removed in f71527f but `server/config/client.go`, `server/config/client_test.go`, and `server/public/model/config_test.go` still reference it (added on master in #36025). Restoring the field makes the branch compile again so CI can go green. Made-with: Cursor * Replace placeholder release-note content (NONE / N/A) on injection The script previously appended its auto-detected lines INSIDE the ```release-note fence but never displaced template placeholders, so PRs that only had `NONE` ended up with output like: NONE Added `Foo.Bar` configuration setting. Go runtime updated from 1.25.8 to 1.25.9. When the existing fence content is empty or consists only of placeholder tokens (NONE, N/A, NA, dashes — case-insensitive), replace it entirely with the auto-detected entries. User-written human content is still preserved by appending instead. Idempotent: stripping followed by re-injection keeps the placeholder visible when there's nothing to inject, and replaces it again when there is. Made-with: Cursor * Update config-change-checker.yml * Update check_config_changes_ci.py --------- Co-authored-by: Your Name <eva.sarafianou@gmail.com> Co-authored-by: Mattermost Build <build@mattermost.com>
* Add "last used" field for incoming webhooks * Address feedback * Rename migrations * Fix web lint
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
* Hide Download Apps UI when running in Desktop app Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Fix ESLint import order for Desktop app visibility changes Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
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.
4 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 : )