[hanzei]

Summary

Problem

• While the OAuth app isn't set up, the checkConfigured middleware logs This plugin is not configured. at error level on every API request. The webapp calls these endpoints (for example /api/v1/connected) on every page load for every user, so the error count grows with users times page loads. Being unconfigured is the normal state right after the plugin is enabled, for example right after the onboarding wizard turns it on.
• getSiteURL only rejects a nil SiteURL. Today an empty SiteURL is caught only by accident: plugin RPC uses gob, which skips zero values, so "" arrives as nil. If "" ever arrived as is, OnActivate would start the plugin and build relative OAuth callback and webhook URLs.

Change

• checkConfigured now logs at debug level. The 501 response is unchanged. System admins still see the specific validation error when they run any /github command.
• getSiteURL now rejects "" as well as nil, with the same error message as before.

Testing

• New TestGetSiteURL: errors for nil and ""; returns the URL for a valid value, with any trailing slash removed.
• go test ./server/..., go vet, golangci-lint run (no new issues) and mattermost-govet pass locally.

Ticket Link

https://mattermost.atlassian.net/browse/MM-71865

🤖 Generated with Claude Code

Change Impact: 🟡 Medium

Changes Breakdown:

+2 -2 Production Changes (11%)
+32 -0 Test Changes (89%)

Regression Risk: The shared helper getSiteURL now rejects empty values, not only nil. This can block plugin activation for deployments with an empty SiteURL, and the diff does not show a test for the activation path. The new TestGetSiteURL covers the nil, empty, and trailing-slash cases.

Generated by CodeRabbitAI

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The changes adjust logging for invalid plugin configuration and make getSiteURL return a configuration error for empty values. Tests cover nil, empty, valid, and trailing-slash site URLs.

Changes: Configuration logging

Layer / File(s)	Summary
Configuration check server/plugin/api.go	checkConfigured logs invalid plugin configuration at Debug level. Its existing 501 API error response remains unchanged.

Site URL handling

Layer / File(s)	Summary
Site URL validation and tests server/plugin/utils.go, server/plugin/utils_test.go	getSiteURL returns an error for nil or empty values. It removes a trailing slash from non-empty URLs. The tests cover these cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: Minimal · up to af6da

This change lowers the log level for an unconfigured plugin and rejects an empty site URL, so the plugin no longer activates with relative callback URLs. The 501 response is unchanged. No actionable merge-blocking risk remains.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names two changes that match the changeset: the unconfigured-plugin log level moves from Error to Debug in checkConfigured, and getSiteURL now rejects an empty SiteURL as well as nil. The ti…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names two changes that match the changeset: the unconfigured-plugin log level moves from Error to Debug in checkConfigured, and getSiteURL now rejects an empty SiteURL as well as nil. The title is specific and accurate. It lists both changes, so it covers the full scope of the diff. The ticket prefix MM-71865 is an identifier and does not reduce clarity.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Essentials
• Run ID: 8c37ec0b-2bd7-46a3-a5e9-ffeb302ea32f

Commits

Reviewing files that changed from the base of the PR and between bcbfe13 and af6da95.

Files selected for processing: 3

• server/plugin/api.go
• server/plugin/utils.go
• server/plugin/utils_test.go

Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.