Skip to content

config: map mattermost-plugin-docs to its plugin ID - #98

Closed
calebroseland wants to merge 1 commit into
masterfrom
docs-plugin-id-mapping
Closed

config: map mattermost-plugin-docs to its plugin ID#98
calebroseland wants to merge 1 commit into
masterfrom
docs-plugin-id-mapping

Conversation

@calebroseland

@calebroseland calebroseland commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

Adds mattermost-plugin-docscom.mattermost.docs to PluginRepoToIDMapping, using the mechanism #78 introduced.

Without an entry, waitForAndInstallPlugin falls back to trimming the mattermost-plugin- prefix:

pluginID, exists := s.Config.PluginRepoToIDMapping[pr.RepoName]
if !exists {
    pluginID = strings.TrimPrefix(pr.RepoName, pluginRepoPrefix)  // → "docs"
}

That yields docs, but the Docs manifest declares "id": "com.mattermost.docs". install-url succeeds either way because it takes a URL, so the Setup Cloud Test Server label produces a test server with the plugin installed but not enabled — surfacing as the "Plugin Installation Issue / Enable Error" warning on the PR comment.

Same shape as the mattermost-plugin-boardsfocalboard entry: a repo whose plugin ID isn't just its name minus the prefix.

Context

The producer side is now in place on the Docs repo — mattermost-plugin-docs#20 adds the dist job that uploads mattermost-plugin-docs-<sha7>.tar.gz to the bucket matterwick polls, and the upload is confirmed working. Nothing else in matterwick needs changing: isPluginRepository already matches by mattermost-plugin- prefix, the bucket is hardcoded to the one being written to, and the Setup Cloud Test Server label already exists on that repo.

Note for whoever deploys

This updates the in-repo default config only. If the deployed config-matterwick.json secret does not inherit from this file, it needs the same entry to take effect.

Test plan

Apply Setup Cloud Test Server to a Docs PR and confirm the resulting comment reports the plugin as enabled rather than an Enable Error.

Release Note

NONE

Without an entry the ID falls back to trimming the mattermost-plugin-
prefix, giving "docs", but the Docs manifest declares
"com.mattermost.docs". install-url succeeds either way since it takes a
URL, so the Setup Cloud Test Server label produces a server with the
plugin installed but not enabled.
@mm-cloud-bot

Copy link
Copy Markdown

@calebroseland: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

I understand the commands that are listed here

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The default configuration now maps mattermost-plugin-docs to com.mattermost.docs and retains the existing boards mapping.

Changes

Plugin repository mapping

Layer / File(s) Summary
Add documentation plugin mapping
config/config-matterwick.default.json
The default PluginRepoToIDMapping includes mattermost-plugin-docs mapped to com.mattermost.docs. The existing boards mapping remains unchanged.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Mergeability Score: 🔵 Low · up to 54b14

The PR fixes Docs plugin ID resolution in the default configuration. It is mergeable with explicit owner follow-up to ensure any deployed configuration override includes the same mapping; otherwise Docs may be installed but remain disabled.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the configuration mapping added by this pull request.
Description check ✅ Passed The description explains the mapping, its purpose, deployment consideration, and test plan.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs-plugin-id-mapping

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔇 Additional comments (1)
config/config-matterwick.default.json (1)

73-73: 🗄️ Data Integrity & Integration

⚠️ Unverified finding
Sandbox verification was unavailable.

Confirm that deployed configuration overrides include this mapping.

The value matches the mmctl plugin enable lookup in server/spinwick_plugin.go. If a deployed configuration overrides PluginRepoToIDMapping instead of inheriting config/config-matterwick.default.json, add "mattermost-plugin-docs": "com.mattermost.docs" there as well. Otherwise, the fallback still derives docs.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 25a19266-1181-4324-81e2-78fd0c0329c8

📥 Commits

Reviewing files that changed from the base of the PR and between da38ec0 and 54b14c1.

📒 Files selected for processing (1)
  • config/config-matterwick.default.json

@mm-cloud-bot mm-cloud-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed labels Aug 12, 2026
@calebroseland

calebroseland commented Aug 25, 2026

Copy link
Copy Markdown
Member Author

Closing — wrong target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note-none Denotes a PR that doesn't merit a release note.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants