fix: auth login 時に tenantId を service として config に保存する#89
Merged
Conversation
ログイン後に NGSILD-Tenant ヘッダーが付与されない問題を修正。 レスポンスの tenantId を config.service に保存するようにした。
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughマルチテナントの対話型 email/password ログインで、選択された最終テナントIDを追跡し、ログイン結果を設定に保存する際に Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/auth.test.ts (1)
338-349: 既存serviceがある場合の消去ケースもテストしてください。このケースは「最初から
serviceが無い設定」しか検証していないため、既存serviceが残留する不具合を取りこぼします。loadConfigを{ service: "old-tenant" }にしたテストを追加するのが安全です。🧪 追加テスト案
+ it("removes existing service when tenantId is absent from login response", async () => { + vi.mocked(isInteractive).mockReturnValue(true); + vi.mocked(promptEmail).mockResolvedValue("user@example.com"); + vi.mocked(promptPassword).mockResolvedValue("pass123"); + vi.mocked(loadConfig).mockReturnValue({ service: "old-tenant" } as never); + client.rawRequest.mockResolvedValue( + mockResponse({ accessToken: "tok" }), + ); + const program = makeProgram(); + await runCommand(program, ["auth", "login"]); + const savedConfig = vi.mocked(saveConfig).mock.calls[0][0] as Record<string, unknown>; + expect(savedConfig).not.toHaveProperty("service"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth.test.ts` around lines 338 - 349, Add a second test that covers the case where an existing service value should be removed: mock loadConfig to return { service: "old-tenant" } before calling runCommand in the "does not set service when tenantId is absent from login response" scenario (use the same setup as the existing test: vi.mocked(isInteractive).., promptEmail, promptPassword, client.rawRequest returning mockResponse({ accessToken: "tok" })), then call runCommand(program, ["auth", "login"]) and assert that the saved config passed to saveConfig (vi.mocked(saveConfig).mock.calls[0][0]) does not have the "service" property; reference functions: loadConfig, saveConfig, runCommand, makeProgram, client.rawRequest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/auth.ts`:
- Around line 147-149: The current branch only sets config.service when
finalTenantId is present, leaving an old tenant value when finalTenantId is
absent; update the logic around finalTenantId (the if (finalTenantId) {
config.service = finalTenantId; } block) to explicitly remove or clear
config.service when finalTenantId is falsy (e.g., delete config.service or set
it to undefined/null) so stale NGSILD-Tenant headers are not sent by subsequent
API calls.
---
Nitpick comments:
In `@tests/auth.test.ts`:
- Around line 338-349: Add a second test that covers the case where an existing
service value should be removed: mock loadConfig to return { service:
"old-tenant" } before calling runCommand in the "does not set service when
tenantId is absent from login response" scenario (use the same setup as the
existing test: vi.mocked(isInteractive).., promptEmail, promptPassword,
client.rawRequest returning mockResponse({ accessToken: "tok" })), then call
runCommand(program, ["auth", "login"]) and assert that the saved config passed
to saveConfig (vi.mocked(saveConfig).mock.calls[0][0]) does not have the
"service" property; reference functions: loadConfig, saveConfig, runCommand,
makeProgram, client.rawRequest.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 65541604-e640-4179-b457-1f4fe2f2e1ff
📒 Files selected for processing (3)
CHANGELOG.mdsrc/commands/auth.tstests/auth.test.ts
CodeRabbit レビュー対応。finalTenantId が falsy の場合に config.service を明示的に delete してステール値を防ぐ。 テストケースも追加。
Member
Author
|
ご指摘ありがとうございます。2点対応しました。 1.
2.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
auth login後にリクエストへNGSILD-Tenantヘッダーが付与されない問題を修正tenantIdをconfig.serviceとして保存するようにしたselectedTenantIdを保存tenantIdがないレスポンス（スーパー管理者等）は従来通りservice未設定のままRoot Cause
GdbClient.buildHeaders()はthis.serviceが設定されている場合のみNGSILD-Tenantヘッダーを付与する。しかしauth loginはトークンと refreshToken しか config に保存しておらず、tenantId→serviceの変換が行われていなかった。Test plan
auth login後にgeonic entities list --dry-runでNGSILD-Tenantヘッダーが含まれることを確認npm run lint && npm run typecheck && npm testがすべて通ること（CI）serviceが設定されないことを確認Summary by CodeRabbit
バグ修正
テスト
ドキュメント