Skip to content

Add: client_id をアンインストールフォームURLに付与 - #451

Merged
ujiro99 merged 3 commits into
dev-1.1.1from
feat/uninstall-url-client-id
Aug 14, 2026
Merged

Add: client_id をアンインストールフォームURLに付与#451
ujiro99 merged 3 commits into
dev-1.1.1from
feat/uninstall-url-client-id

Conversation

@ujiro99

@ujiro99 ujiro99 commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • アンインストールフォーム表示時のURLに、アナリティクスで使用している client_id(GA4イベント送信と同一のUUID、getOrCreateClientId())をクエリパラメータとして付与
  • これにより、アンインストール理由の回答とGA4のイベントデータを紐づけて分析できるようにする

Closes #449

Test plan

  • yarn lint でエラーがないことを確認(既存の警告のみ)
  • yarn vitest run src/background_script.test.ts で既存テストが全て通ることを確認

🤖 Generated with Claude Code

Closes #449

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown

コードレビュー

getOrCreateClientId() を使ってアンインストールアンケートURLに client_id を付与する変更です。目的(GA4イベントとの紐付け)は明確で、実装自体もシンプルですが、以下の点が気になりました。

1. 既存の同一 try ブロック内の処理を巻き込むリスク(要検討)

packages/extension/src/background_script.ts:456-465

if (
  details.reason === chrome.runtime.OnInstalledReason.INSTALL ||
  details.reason === chrome.runtime.OnInstalledReason.UPDATE
) {
  // Set uninstall survey URL with client_id for analysis
  const clientId = await getOrCreateClientId()
  chrome.runtime.setUninstallURL(
    `${NEW_HUB_URL}/uninstall?client_id=${clientId}`,
  )
}

getOrCreateClientId() は内部で Storage.get/Storage.setpackages/extension/src/services/storage/index.ts)を呼んでおり、これらは「拡張機能コンテキスト無効化」以外のエラー(例: QUOTA_BYTES_PER_ITEM 超過など)は re-throw する実装です(packages/extension/src/services/storage/index.test.ts:233-239, 262-269CLIENT_ID キーに対する re-throw が既にテストされています)。

このコードは onInstalled リスナー全体を包む try/catchbackground_script.ts:443-474)の中にあるため、getOrCreateClientId() が例外を投げると、後続の checkAndPerformDailyBackup() / checkAndPerformWeeklyBackup() の呼び出し(background_script.ts:468, 471)までスキップされてしまいます。install/update 時のバックアップチェックが client_id 取得失敗に巻き込まれて実行されなくなる可能性があるので、setUninstallURL の設定は個別に try/catch で囲むか、失敗時は client_id なしのURLにフォールバックする形にした方が安全だと思います。

2. 既存テストがこの変更を実質カバーできていない

packages/extension/src/background_script.test.ts:576-621(IN-01, IN-02)

vi.doMock("@/services/analytics", ...)ANALYTICS_EVENTSsendEvent のみをモックしており、今回importが追加された getOrCreateClientId はモックされていません。

vi.doMock("@/services/analytics", () => ({
  ANALYTICS_EVENTS: { INSTALLED: "installed" },
  sendEvent: mockSendEvent,
}))

そのため onInstalledListener 内で getOrCreateClientId() を呼んだ瞬間に TypeError: getOrCreateClientId is not a function が発生し、外側の try/catchbackground_script.ts:472-474)で握りつぶされます。IN-01 では sendEvent の呼び出しが例外発生より前(INSTALL 時、background_script.ts:447)にあるためテスト自体は緑になりますが、結果として新規追加した setUninstallURL(...client_id=...) のロジックはテスト実行時に一度も正常完了しておらず、実質未検証の状態です。今後この部分にバグ(URL組み立てミスなど)が混入してもテストでは検知できません。

getOrCreateClientId をモックに追加し、chrome.runtime.setUninstallURL が期待したURL(client_id 付き)で呼ばれることをアサーションするテストを追加することをおすすめします。

3. (軽微)encodeURIComponent の不使用

packages/extension/src/background_script.ts:463

clientIdcrypto.randomUUID() 由来の値(services/analytics.ts:171-178)なので実害はありませんが、他の箇所(例: packages/extension/src/components/option/ShareButton.tsx:39encodeURIComponent(command.id))ではクエリパラメータをエンコードする慣習があるようです。一貫性の観点で encodeURIComponent(clientId) としておくと安全側に倒せます。

その他

  • Lint/型に問題は見当たりません。
  • 変更差分自体は小さく、意図も明確でした。

🤖 Generated with Claude Code によるレビューです。

ujiro99 and others added 2 commits August 14, 2026 11:54
…kip backup checks

Wraps the client_id fetch/setUninstallURL call in its own try/catch,
falling back to the plain uninstall URL on failure, and adds tests
that exercise the onInstalled listener end to end.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts:
#	packages/extension/src/background_script.test.ts
#	packages/extension/src/test/setup.ts
@ujiro99
ujiro99 merged commit 308fab5 into dev-1.1.1 Aug 14, 2026
1 of 2 checks passed
@ujiro99
ujiro99 deleted the feat/uninstall-url-client-id branch August 14, 2026 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant