fix: add SQS migrated project link back to source dashboard#484
Conversation
Adds a "SQS migrated project" link to every migrated SonarQube Cloud project, pointing back to the original SonarQube Server project's dashboard, so Support can identify migrated projects and trace them to their source (#418). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| func projectSourceLinkURL(baseURL, projectKey string) string { | ||
| if baseURL == "" || projectKey == "" { | ||
| return "" | ||
| } | ||
| base := strings.TrimRight(baseURL, "/") | ||
| return fmt.Sprintf("%s/dashboard?id=%s", base, url.QueryEscape(projectKey)) | ||
| } |
There was a problem hiding this comment.
💡 Edge Case: Idempotency may miss escaped project keys, creating duplicate links
In runSetProjectSourceLink, the built link URL uses url.QueryEscape(projectKey), so a project key containing special characters (e.g. the very common Maven-style com.company:project) becomes .../dashboard?id=com.company%3Aproject. The idempotency check compares the freshly-built linkURL against l.URL returned by e.Cloud.Projects.ListLinks with an exact string match (l.URL == linkURL). If SonarQube Cloud stores/returns the URL in a decoded/normalized form (e.g. %3A decoded back to :) the comparison fails and a re-run creates a duplicate "SQS migrated project" link.
Note the sibling issue-sync idempotency helper (TestSourceLinkIdempotencyHelpers) explicitly guards against HTML-escaped URL variants, indicating the team already knows cloud can return transformed URLs — but the project-link path does no such normalization. The two tests only exercise keys with no special chars (proj1), so this gap is untested.
Suggested fix: normalize both sides before comparison (e.g. compare on decoded URL or match on link name alone since the migration only ever creates one link with migratedProjectLinkName), or add a test case with a :-containing project key.
Was this helpful? React with 👍 / 👎
| err := forEachMigrateItem(ctx, e, "setProjectSourceLink", "createProjects", | ||
| func(ctx context.Context, item json.RawMessage, w *common.ChunkWriter) error { | ||
| cloudKey := extractField(item, "cloud_project_key") | ||
| if cloudKey == "" { | ||
| return nil | ||
| } | ||
| baseURL := resolveSourceBaseURL(e, extractField(item, "server_url")) | ||
| linkURL := projectSourceLinkURL(baseURL, extractField(item, "key")) | ||
| if linkURL == "" { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
💡 Performance: resolveSourceBaseURL re-reads getServerSettings per project
runSetProjectSourceLink calls resolveSourceBaseURL(e, ...) inside the forEachMigrateItem callback, which runs concurrently once per migrated project. resolveSourceBaseURL in turn calls readExtractItems(e, "getServerSettings") → structure.ReadExtractData, which reads and parses the entire getServerSettings extract from disk on every invocation. For a migration with N projects this re-reads/re-parses the same file N times (and concurrently). The server base URL is effectively per-server (usually a single source server), so this is redundant work.
This mirrors the existing issue/hotspot-sync pattern (also once per project), so it is not a regression, but it is worth resolving the base URL once (or memoizing per server_url) before the loop to avoid repeated disk I/O and JSON parsing on large migrations.
Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 0 resolved / 2 findingsAdds a link to source dashboards for migrated SQS projects, including new validation logic and integration tests. Consider refining the idempotency check to handle URL-encoded keys and caching server settings to avoid redundant lookups. 💡 Edge Case: Idempotency may miss escaped project keys, creating duplicate links📄 go/internal/migrate/tasks_associate.go:1086-1092 📄 go/internal/migrate/tasks_associate.go:1122-1128 In Note the sibling issue-sync idempotency helper ( Suggested fix: normalize both sides before comparison (e.g. compare on decoded URL or match on link name alone since the migration only ever creates one link with 💡 Performance: resolveSourceBaseURL re-reads getServerSettings per project📄 go/internal/migrate/tasks_associate.go:1105-1115 📄 go/internal/migrate/helpers.go:104-106
This mirrors the existing issue/hotspot-sync pattern (also once per project), so it is not a regression, but it is worth resolving the base URL once (or memoizing per server_url) before the loop to avoid repeated disk I/O and JSON parsing on large migrations. 🤖 Prompt for agentsOptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
2 participants
Summary
<originalServerURL>/dashboard?id=<originalProjectKey>), so Support (and anyone else) can identify migrated projects and trace them to their source.setProjectSourceLinktask, dependent oncreateProjects, registered alongsidesetProjectLinks; also added to the single-projecttransfercommand's task list.ci_name=sonar-migration-toolinanalytics.pb) was already implemented by fix: generate mandatory analytics.pb in fabricated scanner report #480.Fixes #418
Test plan
go build ./...go vet ./...go test ./...(full suite green)TestProjectSourceLinkURLcovering the URL builderTestSetProjectSourceLink(create path) andTestSetProjectSourceLinkSkipsExisting(idempotency)