diff --git a/.github/scripts/check_config_changes_ci.py b/.github/scripts/check_config_changes_ci.py new file mode 100644 index 00000000000..be00bc7fef8 --- /dev/null +++ b/.github/scripts/check_config_changes_ci.py @@ -0,0 +1,590 @@ +#!/usr/bin/env python3 +""" +.github/scripts/check_config_changes_ci.py + +CI script that detects notable changes across several Mattermost source files +and appends structured release-note entries to the PR description. + +Checkers +──────── +1. config.go — exported struct field additions/removals +2. api4/ — API endpoint additions/removals (Handle() calls) +3. audit_events.go — AuditEvent* constant additions/removals +4. Dockerfile.buildenv — Go (base-image) version changes + +All inputs come from environment variables set by the GitHub Actions workflow: + GITHUB_TOKEN — built-in Actions token (pull-requests: write scope) + PR_NUMBER — pull request number + BASE_SHA — base commit SHA + HEAD_SHA — head commit SHA + REPO — owner/repo (e.g. mattermost/mattermost) +""" + +import os +import re +import sys +import subprocess +import requests +from dataclasses import dataclass, field +from typing import Optional + +# ── Environment ──────────────────────────────────────────────────────────────── + +GITHUB_TOKEN = os.environ["GITHUB_TOKEN"] +PR_NUMBER = int(os.environ["PR_NUMBER"]) +BASE_SHA = os.environ["BASE_SHA"] +HEAD_SHA = os.environ["HEAD_SHA"] +REPO = os.environ.get("REPO", "mattermost/mattermost") + +BASE_URL = "https://api.github.com" +HEADERS = { + "Authorization": f"token {GITHUB_TOKEN}", + "Accept": "application/vnd.github.v3+json", +} + +# Timeout for all GitHub API requests: (connect seconds, read seconds). +# Prevents the workflow from hanging indefinitely on a slow/unresponsive API. +_TIMEOUT = (5, 30) + +# Paths watched by this script (must align with `paths:` in the workflow YAML) +WATCHED_PATHS = [ + "server/public/model/config.go", + "server/channels/api4/", + "server/public/model/audit_events.go", + "server/build/Dockerfile.buildenv", +] + + +# ── Data types ───────────────────────────────────────────────────────────────── + +@dataclass +class CheckResult: + """Holds the findings from one checker.""" + label: str # Section heading, e.g. "`config.json` Changes" + additions: list = field(default_factory=list) + removals: list = field(default_factory=list) + changes: list = field(default_factory=list) # for free-form entries (version bumps) + + def has_findings(self) -> bool: + return bool(self.additions or self.removals or self.changes) + + def to_markdown(self) -> str: + lines = [f"### {self.label}"] + if self.additions: + lines.append("**Added:** " + ", ".join(self.additions)) + if self.removals: + lines.append("**Removed:** " + ", ".join(self.removals)) + for change in self.changes: + lines.append(change) + return "\n".join(lines) + + +# ── Diff helpers ─────────────────────────────────────────────────────────────── + +def get_full_patch() -> str: + """Return unified diff for all watched paths between base and head.""" + result = subprocess.run( + ["git", "diff", f"{BASE_SHA}...{HEAD_SHA}", "--"] + WATCHED_PATHS, + capture_output=True, + text=True, + check=True, + ) + return result.stdout + + +def split_patch_by_file(full_patch: str) -> dict[str, str]: + """ + Split a multi-file unified diff into {filename: patch} mapping. + Filenames are the b-side (new) path, stripped of the 'b/' prefix. + """ + patches: dict[str, str] = {} + current_file: Optional[str] = None + current_lines: list[str] = [] + + for line in full_patch.splitlines(keepends=True): + if line.startswith("diff --git "): + if current_file: + patches[current_file] = "".join(current_lines) + current_lines = [line] + # Extract filename from "diff --git a/foo b/foo" + m = re.search(r" b/(.+)$", line.rstrip()) + current_file = m.group(1) if m else None + else: + current_lines.append(line) + + if current_file: + patches[current_file] = "".join(current_lines) + + return patches + + +def file_at(ref: str, path: str) -> str: + """Return the full contents of `path` at git ref `ref`, or '' if absent.""" + try: + return subprocess.run( + ["git", "show", f"{ref}:{path}"], + capture_output=True, text=True, check=True, + ).stdout + except subprocess.CalledProcessError: + return "" + + +# ── Checker 1 — config.go ────────────────────────────────────────────────────── + +_CONFIG_PATH = "server/public/model/config.go" +_STRUCT_DECL_RE = re.compile(r"^type\s+(\w+)\s+struct\s*\{") +_FIELD_LINE_RE = re.compile(r"^\t([A-Z][A-Za-z0-9_]*)\s+\S") + + +def _scan_struct_fields(src: str) -> set[tuple[str, str]]: + """ + Walk Go source and return {(StructName, FieldName)} for every exported + field in every struct. + + Uses a brace-depth stack so nested anonymous structs, interface bodies, + and function literals don't corrupt the enclosing struct context. + Named type declarations cannot be nested in Go, so the struct_stack + never grows beyond one entry for named structs. + """ + fields: set[tuple[str, str]] = set() + # Each entry: (struct_name, brace_depth_when_opened) + struct_stack: list[tuple[str, int]] = [] + depth = 0 + + for line in src.splitlines(): + sm = _STRUCT_DECL_RE.match(line) + if sm: + # Record depth *before* counting this line's braces + struct_stack.append((sm.group(1), depth)) + + depth += line.count("{") - line.count("}") + + # Pop any structs whose closing brace has been passed + while struct_stack and depth <= struct_stack[-1][1]: + struct_stack.pop() + + # Record fields only when we're directly inside exactly one named struct + if len(struct_stack) == 1: + fm = _FIELD_LINE_RE.match(line) + if fm: + fields.add((struct_stack[0][0], fm.group(1))) + + return fields + + +def check_config(patches: dict[str, str]) -> CheckResult: + """ + Detect exported Go struct field additions/removals in config.go. + + Compares full-file snapshots at BASE_SHA and HEAD_SHA so that fields + are always attributed to the correct struct regardless of which diff + hunks are present. + """ + result = CheckResult(label="`config.json` Field Changes") + if _CONFIG_PATH not in patches: + return result + + base_fields = _scan_struct_fields(file_at(BASE_SHA, _CONFIG_PATH)) + head_fields = _scan_struct_fields(file_at(HEAD_SHA, _CONFIG_PATH)) + + added = head_fields - base_fields + removed = base_fields - head_fields + + result.additions = sorted(f"`{s}.{f}`" for s, f in added) + result.removals = sorted(f"`{s}.{f}`" for s, f in removed) + return result + + +# ── Checker 2 — api4/ ───────────────────────────────────────────────────────── + +# Matches Handle() route registrations after whitespace-collapsing the source. +# Whitespace collapse makes multi-line declarations single-searchable. +# Group 1: path Group 2: handler func Group 3: raw Methods(...) content +# +# The wrapper pattern uses [^)]* so it tolerates any middleware arguments +# (e.g. r.APIHandler(...), r.ApiSessionRequired(..., isLocal=true), etc.) +# without having to enumerate every possible wrapper signature. +_HANDLE_RE = re.compile( + r'\.Handle\("([^"]*)"' # path + r',\s*[^)]*\((\w+)\)\)' # wrapper(...handlerFunc)) + r'\.Methods\(([^)]+)\)', # .Methods(one or more methods) +) + +_METHOD_RE = re.compile(r'(?:http\.Method)?(\w+)') +_HTTP_METHODS = {"GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS"} + + +def _parse_methods(raw: str) -> list[str]: + """Split raw Methods(...) content into individual uppercase HTTP verbs. + + Filters against the set of known HTTP methods so that incidental + identifiers (handler names, constants, etc.) that happen to appear + inside Methods(...) don't produce spurious results. + """ + return [ + verb + for token in raw.split(",") + if (m := _METHOD_RE.search(token.strip())) + and (verb := m.group(1).upper()) in _HTTP_METHODS + ] + + +def _format_endpoint(path: str, handler: str, method: str) -> str: + return f"`{method.upper()} {path or '/'}` (`{handler}`)" + + +def _parse_endpoints(src: str) -> set[tuple[str, str, str]]: + """ + Parse Handle() registrations from a Go source file. + + Whitespace-collapses the entire file first so multi-line declarations + (e.g. the 18 in group.go) are matched as a single token sequence. + Returns {(path, handler, method)} tuples. + """ + blob = " ".join(src.split()) + endpoints: set[tuple[str, str, str]] = set() + for m in _HANDLE_RE.finditer(blob): + path, handler, methods_raw = m.group(1), m.group(2), m.group(3) + for method in _parse_methods(methods_raw): + endpoints.add((path or "/", handler, method)) + return endpoints + + +def check_api(patches: dict[str, str]) -> CheckResult: + """ + Detect API endpoint additions/removals in the api4/ directory. + + Compares full-file snapshots at BASE_SHA and HEAD_SHA via set arithmetic, + so multi-line and multi-method registrations are handled correctly. + """ + result = CheckResult(label="API Changes (`api4`)") + + api4_patches = { + fname: patch + for fname, patch in patches.items() + if fname.startswith("server/channels/api4/") and fname.endswith(".go") + } + if not api4_patches: + return result + + added_eps: set[tuple[str, str, str]] = set() + removed_eps: set[tuple[str, str, str]] = set() + + for fname, patch in api4_patches.items(): + base_eps = _parse_endpoints(file_at(BASE_SHA, fname)) + head_eps = _parse_endpoints(file_at(HEAD_SHA, fname)) + added_eps |= head_eps - base_eps + removed_eps |= base_eps - head_eps + + # Anchor the check to avoid false positives from unrelated source text + if re.search(r"^new file mode \d+", patch, re.MULTILINE): + result.changes.append(f"🆕 New API file: `{fname.split('/')[-1]}`") + if re.search(r"^deleted file mode \d+", patch, re.MULTILINE): + result.changes.append(f"🗑️ Removed API file: `{fname.split('/')[-1]}`") + + result.additions = sorted(_format_endpoint(p, h, m) for p, h, m in added_eps) + result.removals = sorted(_format_endpoint(p, h, m) for p, h, m in removed_eps) + return result + + +# ── Checker 3 — audit_events.go ─────────────────────────────────────────────── + +_AUDIT_EVENT_PATH = "server/public/model/audit_events.go" +_AUDIT_CONST_RE = re.compile(r"^\t(AuditEvent\w+)\s*=") + + +def _parse_audit_events(src: str) -> set[str]: + return {m.group(1) for line in src.splitlines() if (m := _AUDIT_CONST_RE.match(line))} + + +def check_audit_events(patches: dict[str, str]) -> CheckResult: + """ + Detect AuditEvent* constant additions/removals. + + Uses full-file snapshots at BASE_SHA/HEAD_SHA so reorderings and + cross-constant name collisions don't produce false results. + """ + result = CheckResult(label="Audit Log Event Changes") + if _AUDIT_EVENT_PATH not in patches: + return result + + base_events = _parse_audit_events(file_at(BASE_SHA, _AUDIT_EVENT_PATH)) + head_events = _parse_audit_events(file_at(HEAD_SHA, _AUDIT_EVENT_PATH)) + + result.additions = sorted(f"`{e}`" for e in head_events - base_events) + result.removals = sorted(f"`{e}`" for e in base_events - head_events) + return result + + +# ── Checker 4 — Dockerfile.buildenv (Go version) ────────────────────────────── + +# The Go version lives in the base image tag, e.g.: +# FROM mattermost/golang-bullseye:1.25.8@sha256:... +_DOCKERFILE_PATH = "server/build/Dockerfile.buildenv" +_IMAGE_VER_RE = re.compile(r"^FROM \S+:([0-9]+\.[0-9]+(?:\.[0-9]+)?)") + + +def _parse_go_version(src: str) -> Optional[str]: + for line in src.splitlines(): + m = _IMAGE_VER_RE.match(line.strip()) + if m: + return m.group(1) + return None + + +def check_go_version(patches: dict[str, str]) -> CheckResult: + """ + Detect Go runtime version changes via the base image tag. + + Uses full-file snapshots so the version is read from the actual file + state at each ref rather than reconstructed from patch lines. + """ + result = CheckResult(label="Go Runtime Version") + if _DOCKERFILE_PATH not in patches: + return result + + old_ver = _parse_go_version(file_at(BASE_SHA, _DOCKERFILE_PATH)) + new_ver = _parse_go_version(file_at(HEAD_SHA, _DOCKERFILE_PATH)) + + if old_ver and new_ver and old_ver != new_ver: + result.changes.append(f"Go updated: `{old_ver}` → `{new_ver}`") + elif new_ver and not old_ver: + result.additions.append(f"`{new_ver}`") + return result + + +# ── PR description helpers ───────────────────────────────────────────────────── + +# Matches lines that were auto-generated by this script so they can be stripped +# before re-injecting a fresh set on subsequent commits. +_AUTO_LINE_RE = re.compile( + r"^(Added|Removed) `[^`]+`.*(configuration setting|API endpoint|audit log event)\." + r"|^Go runtime updated from \S+ to \S+\." + r"|^Go runtime set to `[^`]+`\." + r"|^🆕 New API file:" + r"|^🗑️ Removed API file:" +) + +# Matches placeholder content inside a release-note fence that means "nothing +# to report yet" (e.g. NONE, N/A, ---). When detected, we replace the +# placeholder rather than appending alongside it. +_PLACEHOLDER_RE = re.compile(r"^\s*(?:NONE|N/?A|-+)\s*$", re.IGNORECASE) + + +def _format_lines(result: CheckResult) -> list[str]: + """Produce natural-language lines for one checker result.""" + lines = [] + + if "`config.json`" in result.label: + for item in result.additions: + lines.append(f"Added {item} configuration setting.") + for item in result.removals: + lines.append(f"Removed {item} configuration setting.") + + elif "API Changes" in result.label: + for item in result.additions: + lines.append(f"Added {item} API endpoint.") + for item in result.removals: + lines.append(f"Removed {item} API endpoint.") + lines.extend(result.changes) # new/deleted file entries + + elif "Audit" in result.label: + for item in result.additions: + lines.append(f"Added {item} audit log event.") + for item in result.removals: + lines.append(f"Removed {item} audit log event.") + + elif "Go Runtime" in result.label: + for item in result.additions: + # item is e.g. "`1.22`" — strip backticks for the prose form + lines.append(f"Go runtime set to {item}.") + for c in result.changes: + # c arrives as "Go updated: `1.21` → `1.22`" — rewrite it + m = re.search(r"`([^`]+)`\s*→\s*`([^`]+)`", c) + if m: + lines.append(f"Go runtime updated from {m.group(1)} to {m.group(2)}.") + else: + lines.append(c) + + return lines + + +def build_pr_note(results: list[CheckResult]) -> str: + """Assemble all findings into a clean plain-text block.""" + lines = [] + for r in results: + if r.has_findings(): + lines.extend(_format_lines(r)) + return "\n".join(lines) + + +def strip_old_note(body: str) -> str: + """ + Remove previously auto-generated lines from the PR description. + + Primary path — lines inside the ```release-note ... ``` fence. + Fallback path — auto-generated lines that were appended outside any fence + (e.g. via the ## Release Notes section on earlier runs). + + Lines are identified by pattern rather than visible markers, so the PR + description stays clean for human readers. + """ + def _clean_fence(m: re.Match) -> str: + open_tag, content, close_tag = m.group(1), m.group(2), m.group(3) + cleaned_lines = [ + line for line in content.split("\n") + if not _AUTO_LINE_RE.match(line.strip()) + ] + return open_tag + "\n".join(cleaned_lines) + close_tag + + cleaned = re.sub( + r"(```release-note)(.*?)(```)", + _clean_fence, + body or "", + flags=re.DOTALL | re.IGNORECASE, + ) + + # Fallback: strip any auto-generated lines that appear outside a fence + # (written by an older version of this script or via the header-inject path). + cleaned_lines = [ + line for line in cleaned.splitlines() + if not _AUTO_LINE_RE.match(line.strip()) + ] + return "\n".join(cleaned_lines).rstrip() + + +def inject_note(body: str, note: str) -> str: + """ + Insert `note` using this priority order: + + 1. INSIDE the ```release-note block, before its closing ``` + (Mattermost convention — keeps everything in one place for reviewers) + 2. After a recognised release-notes section header (## Release Notes, etc.) + 3. Fallback: append a new ## Release Notes section at the end + """ + body = strip_old_note(body) + if not note: + return body + + # 1. Mattermost-style ```release-note ... ``` block — inject INSIDE the fence. + # If the fence currently contains only a placeholder (NONE / N/A / ---), + # replace the placeholder rather than appending alongside it. + release_note_block = re.search( + r"(```release-note)(.*?)(```)", + body, + flags=re.DOTALL | re.IGNORECASE, + ) + if release_note_block: + open_tag = release_note_block.group(1) + content = release_note_block.group(2) # everything between the fences + close_tag = release_note_block.group(3) + block_start = release_note_block.start() + block_end = release_note_block.end() + + # Strip leading/trailing newlines inside the fence for comparison + inner = content.strip() + if _PLACEHOLDER_RE.match(inner): + # Replace the entire fence with a fresh one + new_block = f"{open_tag}\n{note}\n{close_tag}" + else: + # Append before the closing fence + new_block = open_tag + content + note + "\n" + close_tag + + return body[:block_start] + new_block + body[block_end:] + + # 2. Markdown section headers + for header in ["## Release Notes", "## Changelog", "## What Changed", "## What's Changed"]: + if header.lower() in body.lower(): + idx = body.lower().index(header.lower()) + len(header) + return body[:idx] + "\n\n" + note + body[idx:] + + # 3. Fallback — append + return body + "\n\n## Release Notes\n\n" + note + + +# ── GitHub API ───────────────────────────────────────────────────────────────── + +def get_pr_body() -> str: + r = requests.get( + f"{BASE_URL}/repos/{REPO}/pulls/{PR_NUMBER}", + headers=HEADERS, + timeout=_TIMEOUT, + ) + r.raise_for_status() + return r.json().get("body") or "" + + +def update_pr_body(new_body: str) -> None: + r = requests.patch( + f"{BASE_URL}/repos/{REPO}/pulls/{PR_NUMBER}", + headers=HEADERS, + json={"body": new_body}, + timeout=_TIMEOUT, + ) + r.raise_for_status() + + +# ── Main ─────────────────────────────────────────────────────────────────────── + +def main(): + print(f"📋 PR #{PR_NUMBER} | base {BASE_SHA[:8]} → head {HEAD_SHA[:8]}") + print("🔍 Collecting diffs …") + + full_patch = get_full_patch() + if not full_patch.strip(): + print("ℹ️ No changes in watched paths. Nothing to do.") + return + + patches = split_patch_by_file(full_patch) + print(f" {len(patches)} file(s) changed in watched paths.\n") + + # Run all checkers + checkers = [ + check_config, + check_api, + check_audit_events, + check_go_version, + ] + results: list[CheckResult] = [fn(patches) for fn in checkers] + + for r in results: + if r.has_findings(): + print(f" ✅ {r.label}") + if r.additions: + print(f" Added: {', '.join(r.additions)}") + if r.removals: + print(f" Removed: {', '.join(r.removals)}") + for c in r.changes: + print(f" {c}") + else: + print(f" – {r.label}: no changes") + + note = build_pr_note(results) + if not note: + print("\nℹ️ No notable changes found across all checkers.") + return + + print("\n🔄 Fetching PR description …") + body = get_pr_body() + new_body = inject_note(body, note) + + if new_body == body: + print("ℹ️ PR description already up to date — no changes needed.") + return + + update_pr_body(new_body) + print(f"✅ PR #{PR_NUMBER} description updated.") + + +if __name__ == "__main__": + try: + main() + except subprocess.CalledProcessError as e: + print(f"❌ git diff failed:\n{e.stderr}", file=sys.stderr) + sys.exit(1) + except requests.HTTPError as e: + # Avoid dumping the full response body (can be large / noisy). + # status + reason gives enough context for debugging (e.g. "403 Forbidden"). + reason = e.response.reason or "unknown" + print(f"❌ GitHub API error: {e.response.status_code} {reason}", file=sys.stderr) + sys.exit(1) diff --git a/.github/workflows/config-change-checker.yml b/.github/workflows/config-change-checker.yml new file mode 100644 index 00000000000..0772baaf0e6 --- /dev/null +++ b/.github/workflows/config-change-checker.yml @@ -0,0 +1,66 @@ +# .github/workflows/config-change-checker.yml +# +# Automatically detects notable additions/removals across four source files +# and appends structured release-note entries to the PR description under +# the "## Release Notes" section. +# +# Tracked files / directories: +# • server/public/model/config.go — config struct field changes +# • server/channels/api4/ — API endpoint additions/removals +# • server/public/model/audit_events.go — audit log event constant changes +# • server/build/Dockerfile.buildenv — Go runtime version changes +# +# No secrets needed — uses the built-in GITHUB_TOKEN. + +name: Config Change Checker + +on: + pull_request: + types: [opened, synchronize, reopened] + paths: + - 'server/public/model/config.go' + - 'server/channels/api4/**' + - 'server/public/model/audit_events.go' + - 'server/build/Dockerfile.buildenv' + +# Cancel any in-progress run for the same PR when a new commit is pushed. +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + check-release-notes: + name: Detect release-note-worthy changes + runs-on: ubuntu-latest + # Skip bot-authored PRs (Dependabot, mattermost-bot, etc.) — they will + # not touch these paths intentionally and cannot receive description updates + # via GITHUB_TOKEN anyway (fork-like restrictions apply to most bots). + if: github.event.pull_request.user.type != 'Bot' + + permissions: + pull-requests: write # needed to update the PR description + contents: read + + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + # Fetch enough history to diff against the base branch + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: '3.11' + + - name: Install dependencies + run: pip install requests==2.32.3 --quiet + + - name: Detect changes and update PR description + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + REPO: ${{ github.repository }} + run: python3 .github/scripts/check_config_changes_ci.py diff --git a/api/v4/source/definitions.yaml b/api/v4/source/definitions.yaml index 0968f660efe..ba31ba5325f 100644 --- a/api/v4/source/definitions.yaml +++ b/api/v4/source/definitions.yaml @@ -1151,6 +1151,10 @@ components: description: The time in milliseconds a incoming webhook was deleted type: integer format: int64 + last_used: + description: The time in milliseconds this incoming webhook was last used to post a message + type: integer + format: int64 channel_id: description: The ID of a public channel or private group that receives the webhook payloads diff --git a/server/channels/app/webhook.go b/server/channels/app/webhook.go index 8ee15553f59..92775e70d50 100644 --- a/server/channels/app/webhook.go +++ b/server/channels/app/webhook.go @@ -445,6 +445,7 @@ func (a *App) UpdateIncomingWebhook(oldHook, updatedHook *model.IncomingWebhook) updatedHook.UpdateAt = model.GetMillis() updatedHook.TeamId = oldHook.TeamId updatedHook.DeleteAt = oldHook.DeleteAt + updatedHook.LastUsed = oldHook.LastUsed newWebhook, err := a.Srv().Store().Webhook().UpdateIncoming(updatedHook) if err != nil { @@ -903,7 +904,16 @@ func (a *App) HandleIncomingWebhook(rctx request.CTX, hookID string, req *model. } _, err := a.CreateWebhookPost(rctx, hook.UserId, channel, text, overrideUsername, overrideIconURL, req.IconEmoji, req.Props, webhookType, threadRootID, req.Priority) - return err + if err != nil { + return err + } + + now := model.GetMillis() + if nErr := a.Srv().Store().Webhook().UpdateIncomingLastUsed(hook.Id, now); nErr != nil { + rctx.Logger().Warn("Failed to update incoming webhook LastUsed", mlog.String("hook_id", hook.Id), mlog.Err(nErr)) + } + + return nil } func (a *App) CreateCommandWebhook(commandID string, args *model.CommandArgs) (*model.CommandWebhook, *model.AppError) { diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 04fc52052f5..73a7ed10fd1 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -363,3 +363,5 @@ channels/db/migrations/postgres/000182_create_channel_join_requests_channel_stat channels/db/migrations/postgres/000182_create_channel_join_requests_channel_status_index.up.sql channels/db/migrations/postgres/000183_create_channel_join_requests_user_status_index.down.sql channels/db/migrations/postgres/000183_create_channel_join_requests_user_status_index.up.sql +channels/db/migrations/postgres/000184_add_lastused_to_incoming_webhooks.down.sql +channels/db/migrations/postgres/000184_add_lastused_to_incoming_webhooks.up.sql diff --git a/server/channels/db/migrations/postgres/000184_add_lastused_to_incoming_webhooks.down.sql b/server/channels/db/migrations/postgres/000184_add_lastused_to_incoming_webhooks.down.sql new file mode 100644 index 00000000000..a0dc89dc227 --- /dev/null +++ b/server/channels/db/migrations/postgres/000184_add_lastused_to_incoming_webhooks.down.sql @@ -0,0 +1 @@ +ALTER TABLE incomingwebhooks DROP COLUMN IF EXISTS lastused; diff --git a/server/channels/db/migrations/postgres/000184_add_lastused_to_incoming_webhooks.up.sql b/server/channels/db/migrations/postgres/000184_add_lastused_to_incoming_webhooks.up.sql new file mode 100644 index 00000000000..8031f7eeee4 --- /dev/null +++ b/server/channels/db/migrations/postgres/000184_add_lastused_to_incoming_webhooks.up.sql @@ -0,0 +1 @@ +ALTER TABLE incomingwebhooks ADD COLUMN IF NOT EXISTS lastused bigint NOT NULL DEFAULT 0; diff --git a/server/channels/store/localcachelayer/webhook_layer.go b/server/channels/store/localcachelayer/webhook_layer.go index 9a753b4b4ff..75d11d89671 100644 --- a/server/channels/store/localcachelayer/webhook_layer.go +++ b/server/channels/store/localcachelayer/webhook_layer.go @@ -87,3 +87,13 @@ func (s LocalCacheWebhookStore) PermanentDeleteIncomingByChannel(channelId strin s.ClearCaches() return nil } + +func (s LocalCacheWebhookStore) UpdateIncomingLastUsed(webhookID string, lastUsed int64) error { + err := s.WebhookStore.UpdateIncomingLastUsed(webhookID, lastUsed) + if err != nil { + return err + } + + s.InvalidateWebhookCache(webhookID) + return nil +} diff --git a/server/channels/store/sqlstore/webhook_store.go b/server/channels/store/sqlstore/webhook_store.go index 4a7857f878f..5b3f4feca41 100644 --- a/server/channels/store/sqlstore/webhook_store.go +++ b/server/channels/store/sqlstore/webhook_store.go @@ -46,6 +46,7 @@ func newSqlWebhookStore(sqlStore *SqlStore, metrics einterfaces.MetricsInterface "Username", "IconURL", "ChannelLocked", + "LastUsed", ). From("IncomingWebhooks") @@ -88,9 +89,9 @@ func (s SqlWebhookStore) SaveIncoming(webhook *model.IncomingWebhook) (*model.In } if _, err := s.GetMaster().NamedExec(`INSERT INTO IncomingWebhooks - (Id, CreateAt, UpdateAt, DeleteAt, UserId, ChannelId, TeamId, DisplayName, Description, Username, IconURL, ChannelLocked) + (Id, CreateAt, UpdateAt, DeleteAt, UserId, ChannelId, TeamId, DisplayName, Description, Username, IconURL, ChannelLocked, LastUsed) VALUES - (:Id, :CreateAt, :UpdateAt, :DeleteAt, :UserId, :ChannelId, :TeamId, :DisplayName, :Description, :Username, :IconURL, :ChannelLocked)`, webhook); err != nil { + (:Id, :CreateAt, :UpdateAt, :DeleteAt, :UserId, :ChannelId, :TeamId, :DisplayName, :Description, :Username, :IconURL, :ChannelLocked, :LastUsed)`, webhook); err != nil { return nil, errors.Wrapf(err, "failed to save IncomingWebhook with id=%s", webhook.Id) } @@ -111,6 +112,19 @@ func (s SqlWebhookStore) UpdateIncoming(hook *model.IncomingWebhook) (*model.Inc return hook, nil } +func (s SqlWebhookStore) UpdateIncomingLastUsed(webhookID string, lastUsed int64) error { + _, err := s.GetMaster().Exec( + `UPDATE IncomingWebhooks SET LastUsed = ? WHERE Id = ? AND DeleteAt = 0`, + lastUsed, + webhookID, + ) + if err != nil { + return errors.Wrapf(err, "failed to update LastUsed for IncomingWebhook id=%s", webhookID) + } + + return nil +} + func (s SqlWebhookStore) GetIncoming(id string, allowFromCache bool) (*model.IncomingWebhook, error) { var webhook model.IncomingWebhook diff --git a/server/channels/store/store.go b/server/channels/store/store.go index c93eb8a2725..04715debd3d 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -655,6 +655,7 @@ type WebhookStore interface { GetIncomingByTeam(teamID string, offset, limit int) ([]*model.IncomingWebhook, error) GetIncomingByTeamByUser(teamID string, userID string, offset, limit int) ([]*model.IncomingWebhook, error) UpdateIncoming(webhook *model.IncomingWebhook) (*model.IncomingWebhook, error) + UpdateIncomingLastUsed(webhookID string, lastUsed int64) error GetIncomingByChannel(channelID string) ([]*model.IncomingWebhook, error) DeleteIncoming(webhookID string, timestamp int64) error PermanentDeleteIncomingByChannel(channelID string) error diff --git a/server/channels/store/storetest/mocks/WebhookStore.go b/server/channels/store/storetest/mocks/WebhookStore.go index 77dd12fd254..16f563681f7 100644 --- a/server/channels/store/storetest/mocks/WebhookStore.go +++ b/server/channels/store/storetest/mocks/WebhookStore.go @@ -668,6 +668,24 @@ func (_m *WebhookStore) UpdateIncoming(webhook *model.IncomingWebhook) (*model.I return r0, r1 } +// UpdateIncomingLastUsed provides a mock function with given fields: webhookID, lastUsed +func (_m *WebhookStore) UpdateIncomingLastUsed(webhookID string, lastUsed int64) error { + ret := _m.Called(webhookID, lastUsed) + + if len(ret) == 0 { + panic("no return value specified for UpdateIncomingLastUsed") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string, int64) error); ok { + r0 = rf(webhookID, lastUsed) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // UpdateOutgoing provides a mock function with given fields: hook func (_m *WebhookStore) UpdateOutgoing(hook *model.OutgoingWebhook) (*model.OutgoingWebhook, error) { ret := _m.Called(hook) diff --git a/server/channels/store/storetest/webhook_store.go b/server/channels/store/storetest/webhook_store.go index a0884698bce..a026cbf1d04 100644 --- a/server/channels/store/storetest/webhook_store.go +++ b/server/channels/store/storetest/webhook_store.go @@ -18,6 +18,8 @@ import ( func TestWebhookStore(t *testing.T, rctx request.CTX, ss store.Store) { t.Run("SaveIncoming", func(t *testing.T) { testWebhookStoreSaveIncoming(t, rctx, ss) }) t.Run("UpdateIncoming", func(t *testing.T) { testWebhookStoreUpdateIncoming(t, rctx, ss) }) + t.Run("UpdateIncomingPreservesLastUsed", func(t *testing.T) { testWebhookStoreUpdateIncomingPreservesLastUsed(t, rctx, ss) }) + t.Run("UpdateIncomingLastUsed", func(t *testing.T) { testWebhookStoreUpdateIncomingLastUsed(t, rctx, ss) }) t.Run("GetIncoming", func(t *testing.T) { testWebhookStoreGetIncoming(t, rctx, ss) }) t.Run("GetIncomingList", func(t *testing.T) { testWebhookStoreGetIncomingList(t, rctx, ss) }) t.Run("GetIncomingListByUser", func(t *testing.T) { testWebhookStoreGetIncomingListByUser(t, rctx, ss) }) @@ -73,6 +75,45 @@ func testWebhookStoreUpdateIncoming(t *testing.T, rctx request.CTX, ss store.Sto require.Equal(t, "TestHook", webhook.DisplayName, "display name is not updated") } +// testWebhookStoreUpdateIncomingPreservesLastUsed ensures the generic UpdateIncoming path does not +// overwrite LastUsed; only UpdateIncomingLastUsed should change that column. +func testWebhookStoreUpdateIncomingPreservesLastUsed(t *testing.T, rctx request.CTX, ss store.Store) { + o1 := buildIncomingWebhook() + saved, err := ss.Webhook().SaveIncoming(o1) + require.NoError(t, err) + require.Zero(t, saved.LastUsed, "new webhook should have LastUsed 0") + + lastUsed := model.GetMillis() + err = ss.Webhook().UpdateIncomingLastUsed(saved.Id, lastUsed) + require.NoError(t, err) + + withStaleLastUsed := *saved + withStaleLastUsed.DisplayName = "RenamedHook" + withStaleLastUsed.LastUsed = 0 + + _, err = ss.Webhook().UpdateIncoming(&withStaleLastUsed) + require.NoError(t, err) + + fromDB, err := ss.Webhook().GetIncoming(saved.Id, false) + require.NoError(t, err) + require.Equal(t, lastUsed, fromDB.LastUsed, "UpdateIncoming must not clear LastUsed when struct has LastUsed 0") + require.Equal(t, "RenamedHook", fromDB.DisplayName) +} + +func testWebhookStoreUpdateIncomingLastUsed(t *testing.T, rctx request.CTX, ss store.Store) { + o1 := buildIncomingWebhook() + o1, err := ss.Webhook().SaveIncoming(o1) + require.NoError(t, err) + + lastUsed := model.GetMillis() + err = ss.Webhook().UpdateIncomingLastUsed(o1.Id, lastUsed) + require.NoError(t, err) + + updated, err := ss.Webhook().GetIncoming(o1.Id, false) + require.NoError(t, err) + require.Equal(t, lastUsed, updated.LastUsed) +} + func testWebhookStoreGetIncoming(t *testing.T, rctx request.CTX, ss store.Store) { var err error diff --git a/server/channels/web/webhook_test.go b/server/channels/web/webhook_test.go index aab7cf79010..b7e33eb12e6 100644 --- a/server/channels/web/webhook_test.go +++ b/server/channels/web/webhook_test.go @@ -42,6 +42,10 @@ func TestIncomingWebhook(t *testing.T) { require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) + refreshed, appErr := th.App.GetIncomingWebhook(hook.Id) + require.Nil(t, appErr) + require.NotZero(t, refreshed.LastUsed) + payload = "payload={\"text\": \"\"}" resp, err = http.Post(url, "application/x-www-form-urlencoded", strings.NewReader(payload)) require.NoError(t, err) diff --git a/server/public/model/incoming_webhook.go b/server/public/model/incoming_webhook.go index ac28a3260e2..2e2b3ded574 100644 --- a/server/public/model/incoming_webhook.go +++ b/server/public/model/incoming_webhook.go @@ -28,6 +28,7 @@ type IncomingWebhook struct { Username string `json:"username"` IconURL string `json:"icon_url"` ChannelLocked bool `json:"channel_locked"` + LastUsed int64 `json:"last_used"` } func (o *IncomingWebhook) Auditable() map[string]any { @@ -44,6 +45,7 @@ func (o *IncomingWebhook) Auditable() map[string]any { "username": o.Username, "icon_url:": o.IconURL, "channel_locked": o.ChannelLocked, + "last_used": o.LastUsed, } } diff --git a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx index ddb52be5bc4..86bbc3b23ac 100644 --- a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx +++ b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx @@ -181,6 +181,7 @@ const AdvancedTextEditor = ({ const teammateDisplayName = useSelector((state: GlobalState) => (teammateId ? getDisplayName(state, teammateId) : '')); const showDndWarning = useSelector((state: GlobalState) => (teammateId ? getStatusForUserId(state, teammateId) === UserStatuses.DND : false)); const selectedPostFocussedAt = useSelector((state: GlobalState) => getSelectedPostFocussedAt(state)); + const aiActionMenuItems = useSelector((state: GlobalState) => state.plugins.components.AIActionMenuItem); const {available: aiRewriteEnabled} = useGetAgentsBridgeEnabled(); const canPost = useSelector((state: GlobalState) => { @@ -317,6 +318,7 @@ const AdvancedTextEditor = ({ rewriteMenuProps, isProcessing: rewriteIsProcessing, } = useRewrite(draft, handleDraftChange, textboxRef, focusTextbox, setServerError); + const hasAIActionsMenu = (aiActionMenuItems?.length ?? 0) > 0 || (aiRewriteEnabled && Boolean(rewriteMenuProps)); const isDisabled = Boolean(readOnlyChannel || (!enableSharedChannelsDMs && isDMOrGMRemote) || rewriteIsProcessing); const [attachmentPreview, fileUploadJSX] = useUploadFiles( @@ -707,6 +709,10 @@ const AdvancedTextEditor = ({ }, [handleDraftChange, draft]); const aiActionsMenu = useMemo(() => { + if (!hasAIActionsMenu) { + return null; + } + return ( ); - }, [draft, getSelectedText, updateText, channelId, location, rewriteMenuProps, aiRewriteEnabled]); + }, [draft, getSelectedText, updateText, channelId, location, rewriteMenuProps, aiRewriteEnabled, hasAIActionsMenu]); const formattingBar = ( { expect(fireEvent.mouseDown(screen.getByLabelText('code'))).toBe(false); }); + + test('should only render separator before bold when AI actions menu is present', () => { + jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({layoutMode: LayoutModes.Wide, ...splitFormattingBarControls('wide')}); + + const {container, rerender} = renderWithContext( + {'AI Actions'}} + />, + ); + + expect(container.querySelectorAll('[data-testid="formatting-bar-separator"]')).toHaveLength(2); + + rerender( + , + ); + + expect(container.querySelectorAll('[data-testid="formatting-bar-separator"]')).toHaveLength(1); + }); }); diff --git a/webapp/channels/src/components/advanced_text_editor/formatting_bar/formatting_bar.tsx b/webapp/channels/src/components/advanced_text_editor/formatting_bar/formatting_bar.tsx index aa006e19cca..2e389136bb8 100644 --- a/webapp/channels/src/components/advanced_text_editor/formatting_bar/formatting_bar.tsx +++ b/webapp/channels/src/components/advanced_text_editor/formatting_bar/formatting_bar.tsx @@ -16,7 +16,7 @@ import type {ApplyMarkdownOptions, MarkdownMode} from 'utils/markdown/apply_mark import FormattingIcon, {IconContainer} from './formatting_icon'; import {LayoutModes, useFormattingBarControls} from './hooks'; -export const Separator = styled.div` +export const Separator = styled.div.attrs({'data-testid': 'formatting-bar-separator'})` display: block; position: relative; width: 1px; diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu_list/product_menu_list.test.tsx b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu_list/product_menu_list.test.tsx index 2ddcec4280c..17a0b6cc39d 100644 --- a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu_list/product_menu_list.test.tsx +++ b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu_list/product_menu_list.test.tsx @@ -3,6 +3,7 @@ import React from 'react'; +import * as UserAgent from '@mattermost/shared/utils/user_agent'; import type {UserProfile} from '@mattermost/types/users'; import type {DeepPartial} from '@mattermost/types/utilities'; @@ -14,6 +15,11 @@ import type {GlobalState} from 'types/store'; import ProductMenuList from './product_menu_list'; import type {Props as ProductMenuListProps} from './product_menu_list'; +const isDesktopAppMock = jest.mocked(UserAgent.isDesktopApp); + +jest.mock('@mattermost/shared/utils/user_agent', () => ({ + isDesktopApp: jest.fn(() => false), +})); jest.mock('components/widgets/menu/menu_items/menu_cloud_trial', () => () => null); jest.mock('components/widgets/menu/menu_items/menu_item_cloud_limit', () => () => null); jest.mock('components/permissions_gates/system_permission_gate', () => ({children}: {children: React.ReactNode}) => <>{children}); @@ -238,4 +244,17 @@ describe('components/global/product_switcher_menu', () => { expect(container).toMatchSnapshot(); }); }); + + test('shows Download Apps link when appDownloadLink configured and not in desktop app', () => { + isDesktopAppMock.mockReturnValue(false); + const {container} = renderWithContext(, adminState); + expect(container.querySelector('#nativeAppLink')).not.toBeNull(); + }); + + test('hides Download Apps link when in desktop app', () => { + isDesktopAppMock.mockReturnValue(true); + const {container} = renderWithContext(, adminState); + expect(container.querySelector('#nativeAppLink')).toBeNull(); + isDesktopAppMock.mockReturnValue(false); + }); }); diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu_list/product_menu_list.tsx b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu_list/product_menu_list.tsx index 71b78712ac1..9df7b1cc883 100644 --- a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu_list/product_menu_list.tsx +++ b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu_list/product_menu_list.tsx @@ -13,6 +13,7 @@ import { ViewGridPlusOutlineIcon, WebhookIncomingIcon, } from '@mattermost/compass-icons/components'; +import {isDesktopApp} from '@mattermost/shared/utils/user_agent'; import type {UserProfile} from '@mattermost/types/users'; import {Permissions} from 'mattermost-redux/constants'; @@ -208,7 +209,7 @@ const ProductMenuList = (props: Props): JSX.Element | null => { } diff --git a/webapp/channels/src/components/integrations/__snapshots__/installed_incoming_webhook.test.tsx.snap b/webapp/channels/src/components/integrations/__snapshots__/installed_incoming_webhook.test.tsx.snap index e300c6f1cef..9989a4c5336 100644 --- a/webapp/channels/src/components/integrations/__snapshots__/installed_incoming_webhook.test.tsx.snap +++ b/webapp/channels/src/components/integrations/__snapshots__/installed_incoming_webhook.test.tsx.snap @@ -69,6 +69,15 @@ exports[`components/integrations/InstalledIncomingWebhook should match snapshot Created by creator on Friday, August 11, 2017 +
+ + Never used + +
diff --git a/webapp/channels/src/components/integrations/abstract_incoming_hook.test.tsx b/webapp/channels/src/components/integrations/abstract_incoming_hook.test.tsx index e871af7bc57..b71264acc59 100644 --- a/webapp/channels/src/components/integrations/abstract_incoming_hook.test.tsx +++ b/webapp/channels/src/components/integrations/abstract_incoming_hook.test.tsx @@ -50,6 +50,7 @@ describe('components/integrations/AbstractIncomingWebhook', () => { username: '', icon_url: '', channel_locked: false, + last_used: 0, }; const enablePostUsernameOverride = true; const enablePostIconOverride = true; diff --git a/webapp/channels/src/components/integrations/abstract_incoming_webhook.tsx b/webapp/channels/src/components/integrations/abstract_incoming_webhook.tsx index 9f76a5a8f63..8f1d35ca5f4 100644 --- a/webapp/channels/src/components/integrations/abstract_incoming_webhook.tsx +++ b/webapp/channels/src/components/integrations/abstract_incoming_webhook.tsx @@ -142,6 +142,7 @@ export default class AbstractIncomingWebhook extends PureComponent delete_at: this.props.initialHook?.delete_at || 0, team_id: this.props.initialHook?.team_id || '', user_id: this.props.initialHook?.user_id || '', + last_used: this.props.initialHook?.last_used || 0, }; this.props.action(hook).then(() => this.setState({saving: false})); diff --git a/webapp/channels/src/components/integrations/edit_incoming_webhook/edit_incoming_webhook.test.tsx b/webapp/channels/src/components/integrations/edit_incoming_webhook/edit_incoming_webhook.test.tsx index c33d7457a70..e8209839a39 100644 --- a/webapp/channels/src/components/integrations/edit_incoming_webhook/edit_incoming_webhook.test.tsx +++ b/webapp/channels/src/components/integrations/edit_incoming_webhook/edit_incoming_webhook.test.tsx @@ -63,6 +63,7 @@ describe('components/integrations/EditIncomingWebhook', () => { username: 'username', icon_url: 'http://test/icon.png', channel_locked: false, + last_used: 0, }; const updateIncomingHook = jest.fn(); diff --git a/webapp/channels/src/components/integrations/installed_incoming_webhook.test.tsx b/webapp/channels/src/components/integrations/installed_incoming_webhook.test.tsx index ebe36f5abc4..898dd6b299d 100644 --- a/webapp/channels/src/components/integrations/installed_incoming_webhook.test.tsx +++ b/webapp/channels/src/components/integrations/installed_incoming_webhook.test.tsx @@ -15,6 +15,7 @@ describe('components/integrations/InstalledIncomingWebhook', () => { id: '9w96t4nhbfdiij64wfqors4i1r', channel_id: '1jiw9kphbjrntfyrm7xpdcya4o', create_at: 1502455422406, + last_used: 0, delete_at: 0, description: 'build status', display_name: 'build', @@ -163,4 +164,24 @@ describe('components/integrations/InstalledIncomingWebhook', () => { ); expect(container.querySelector('.item-details')).not.toBeNull(); }); + + test('should show Last used on when last_used is non-zero', () => { + const lastUsedMs = 1704067200000; + const hookWithLastUsed: IncomingWebhook = { + ...incomingWebhook, + last_used: lastUsedMs, + }; + + renderWithContext( + , + initialState, + ); + + expect(screen.getByText(/Last used on/i)).toBeInTheDocument(); + expect(screen.queryByText('Never used')).not.toBeInTheDocument(); + }); }); diff --git a/webapp/channels/src/components/integrations/installed_incoming_webhook.tsx b/webapp/channels/src/components/integrations/installed_incoming_webhook.tsx index d3e8a680ba6..970924bba29 100644 --- a/webapp/channels/src/components/integrations/installed_incoming_webhook.tsx +++ b/webapp/channels/src/components/integrations/installed_incoming_webhook.tsx @@ -181,6 +181,24 @@ export default class InstalledIncomingWebhook extends React.PureComponent /> +
+ + {incomingWebhook.last_used > 0 ? ( + + ) : ( + + )} + +
); diff --git a/webapp/channels/src/components/mobile_sidebar_right/mobile_sidebar_right_items/mobile_sidebar_right_items.test.tsx b/webapp/channels/src/components/mobile_sidebar_right/mobile_sidebar_right_items/mobile_sidebar_right_items.test.tsx index ccd287426f0..54fb1f5991e 100644 --- a/webapp/channels/src/components/mobile_sidebar_right/mobile_sidebar_right_items/mobile_sidebar_right_items.test.tsx +++ b/webapp/channels/src/components/mobile_sidebar_right/mobile_sidebar_right_items/mobile_sidebar_right_items.test.tsx @@ -3,6 +3,8 @@ import React from 'react'; +import * as UserAgent from '@mattermost/shared/utils/user_agent'; + import {Permissions} from 'mattermost-redux/constants'; import type {MockIntl} from 'tests/helpers/intl-test-helper'; @@ -11,6 +13,12 @@ import {renderWithContext, screen} from 'tests/react_testing_utils'; import {MobileSidebarRightItems} from './mobile_sidebar_right_items'; import type {Props} from './mobile_sidebar_right_items'; +const isDesktopAppMock = jest.mocked(UserAgent.isDesktopApp); + +jest.mock('@mattermost/shared/utils/user_agent', () => ({ + isDesktopApp: jest.fn(() => false), +})); + describe('MobileSidebarRightItems', () => { const defaultProps: Props = { teamId: 'team-id', @@ -160,14 +168,28 @@ describe('MobileSidebarRightItems', () => { expect(screen.getByText('Help')).toBeInTheDocument(); }); - test('should show report link when provided', () => { + test('should show Download Apps link when appDownloadLink is set and not in desktop app', () => { + isDesktopAppMock.mockReturnValue(false); + renderWithContext( + , + defaultState, + ); + expect(screen.getByText('Download Apps')).toBeInTheDocument(); + }); + + test('should hide Download Apps link when in desktop app', () => { + isDesktopAppMock.mockReturnValue(true); renderWithContext( , defaultState, ); - expect(screen.getByText('Report a Problem')).toBeInTheDocument(); + expect(screen.queryByText('Download Apps')).not.toBeInTheDocument(); + isDesktopAppMock.mockReturnValue(false); }); }); diff --git a/webapp/channels/src/components/mobile_sidebar_right/mobile_sidebar_right_items/mobile_sidebar_right_items.tsx b/webapp/channels/src/components/mobile_sidebar_right/mobile_sidebar_right_items/mobile_sidebar_right_items.tsx index 48cf7eb6b64..f6e6aafb3be 100644 --- a/webapp/channels/src/components/mobile_sidebar_right/mobile_sidebar_right_items/mobile_sidebar_right_items.tsx +++ b/webapp/channels/src/components/mobile_sidebar_right/mobile_sidebar_right_items/mobile_sidebar_right_items.tsx @@ -5,6 +5,8 @@ import React from 'react'; import {injectIntl} from 'react-intl'; import type {WrappedComponentProps} from 'react-intl'; +import {isDesktopApp} from '@mattermost/shared/utils/user_agent'; + import {Permissions} from 'mattermost-redux/constants'; import {emitUserLoggedOutEvent} from 'actions/global_actions'; @@ -407,7 +409,7 @@ export class MobileSidebarRightItems extends React.PureComponent { /> ({ + isDesktopApp: jest.fn(() => false), +})); + jest.mock('mattermost-redux/actions/admin', () => ({ ...jest.requireActual('mattermost-redux/actions/admin'), getPrevTrialLicense: () => ({type: 'MOCK_GET_PREV_TRIAL_LICENSE'}), @@ -67,4 +75,17 @@ describe('components/onboarding_tasklist/onboarding_tasklist_completed.tsx', () await userEvent.click(noThanksLink[0]); expect(dismissMockFn).toHaveBeenCalledTimes(1); }); + + test('displays download apps link when not in desktop app', () => { + isDesktopAppMock.mockReturnValue(false); + const {container} = renderWithContext(, initialState); + expect(container.querySelectorAll('.download-apps')).toHaveLength(1); + }); + + test('hides download apps link when in desktop app', () => { + isDesktopAppMock.mockReturnValue(true); + const {container} = renderWithContext(, initialState); + expect(container.querySelectorAll('.download-apps')).toHaveLength(0); + isDesktopAppMock.mockReturnValue(false); + }); }); diff --git a/webapp/channels/src/components/onboarding_tasklist/onboarding_tasklist_completed.tsx b/webapp/channels/src/components/onboarding_tasklist/onboarding_tasklist_completed.tsx index 7d3c9c37365..f3bcae1ce6d 100644 --- a/webapp/channels/src/components/onboarding_tasklist/onboarding_tasklist_completed.tsx +++ b/webapp/channels/src/components/onboarding_tasklist/onboarding_tasklist_completed.tsx @@ -7,6 +7,7 @@ import {useSelector, useDispatch} from 'react-redux'; import {CSSTransition} from 'react-transition-group'; import styled from 'styled-components'; +import {isDesktopApp} from '@mattermost/shared/utils/user_agent'; import type {GlobalState} from '@mattermost/types/store'; import {getPrevTrialLicense} from 'mattermost-redux/actions/admin'; @@ -212,24 +213,26 @@ const Completed = (props: Props): JSX.Element => { /> )} -
- - ( - - {msg} - - ), - }} - /> - -
+ {!isDesktopApp() && ( +
+ + ( + + {msg} + + ), + }} + /> + +
+ )} {showStartTrialBtn &&
({ + isDesktopApp: jest.fn(() => false), +})); + const WrapperComponent = (): JSX.Element => { const taskList = useTasksList(); return ( @@ -87,4 +95,16 @@ describe('onboarding tasks manager', () => { // verify visit_system_console and start_trial were removed expect(screen.queryByText('invite_people')).not.toBeInTheDocument(); }); + + it('Removes download_app task when running in desktop app', () => { + isDesktopAppMock.mockReturnValue(true); + renderWithContext( + , + initialState, + ); + + expect(screen.getAllByRole('listitem')).toHaveLength(5); + expect(screen.queryByText('download_app')).not.toBeInTheDocument(); + isDesktopAppMock.mockReturnValue(false); + }); }); diff --git a/webapp/channels/src/components/onboarding_tasks/onboarding_tasks_manager.tsx b/webapp/channels/src/components/onboarding_tasks/onboarding_tasks_manager.tsx index 2ef52001d99..1483cd77503 100644 --- a/webapp/channels/src/components/onboarding_tasks/onboarding_tasks_manager.tsx +++ b/webapp/channels/src/components/onboarding_tasks/onboarding_tasks_manager.tsx @@ -6,6 +6,8 @@ import {useIntl} from 'react-intl'; import {useDispatch, useSelector} from 'react-redux'; import {matchPath, useLocation} from 'react-router-dom'; +import {isDesktopApp} from '@mattermost/shared/utils/user_agent'; + import {savePreferences} from 'mattermost-redux/actions/preferences'; import {getCurrentUserId} from 'mattermost-redux/selectors/entities/common'; import {getLicense} from 'mattermost-redux/selectors/entities/general'; @@ -138,6 +140,10 @@ export const useTasksList = () => { delete list.INVITE_PEOPLE; } + if (isDesktopApp()) { + delete list.DOWNLOAD_APP; + } + return Object.values(list); }; diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index a8450a2e1f4..ba270f32a04 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -5109,6 +5109,8 @@ "installed_integrations.edit": "Edit", "installed_integrations.fromApp": "Managed by Apps Framework", "installed_integrations.hideSecret": "Hide Secret", + "installed_integrations.last_used": "Last used on {lastUsed, date, full}", + "installed_integrations.never_used": "Never used", "installed_integrations.regenSecret": "Regenerate Secret", "installed_integrations.regenToken": "Regenerate Token", "installed_integrations.showSecret": "Show Secret", diff --git a/webapp/channels/src/utils/test_helper.ts b/webapp/channels/src/utils/test_helper.ts index 8dacc70d202..cf3b21d2587 100644 --- a/webapp/channels/src/utils/test_helper.ts +++ b/webapp/channels/src/utils/test_helper.ts @@ -285,6 +285,7 @@ export class TestHelper { create_at: 1, update_at: 1, delete_at: 1, + last_used: 0, user_id: '', channel_id: '', team_id: '', diff --git a/webapp/platform/types/src/integrations.ts b/webapp/platform/types/src/integrations.ts index 3cdc5e592a7..b78cff05e8c 100644 --- a/webapp/platform/types/src/integrations.ts +++ b/webapp/platform/types/src/integrations.ts @@ -9,6 +9,7 @@ export type IncomingWebhook = { create_at: number; update_at: number; delete_at: number; + last_used: number; user_id: string; channel_id: string; team_id: string;