feat: implement fetchThread for BacklogAdapter#18
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 59 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughBacklogAdapterにBacklogClientを統合し、未実装だった Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Adapter as BacklogAdapter
participant Client as BacklogClient
participant API as Backlog API
Caller->>Adapter: fetchThread(threadId)
Adapter->>Client: getIssue(issueKey)
Client->>API: HTTP GET /issues/{issueKey}
API-->>Client: issue payload
Client-->>Adapter: issue payload
Adapter-->>Caller: ThreadInfo { id, channelId, channelName, metadata }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/adapter.ts (2)
24-32:BacklogClientは注入可能にしておくと保守しやすいです。Line 31 の直接
newはテスト時に prototype モックへ依存しやすいため、任意注入にしておくとユニットテストの分離性が上がります。♻️ リファクタ案
export class BacklogAdapter implements Adapter<BacklogThreadId, unknown> { @@ - constructor(config: BacklogConfig) { + constructor(config: BacklogConfig, client?: BacklogClient) { this.config = config; this.userName = config.userName ?? "Backlog Bot"; - this.client = new BacklogClient(config.host, config.apiKey); + this.client = client ?? new BacklogClient(config.host, config.apiKey); this.formatConverter = new BacklogFormatConverter(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adapter.ts` around lines 24 - 32, The constructor currently instantiates BacklogClient directly (new BacklogClient(...)) which hinders testability; change the Adapter to accept an optional BacklogClient via dependency injection (e.g., add an optional parameter client?: BacklogClient to the constructor or a setter) and fall back to new BacklogClient(config.host, config.apiKey) only when none is provided; update the class field declaration for client and the constructor logic in adapter.ts (symbols: BacklogClient, constructor, this.client) so unit tests can inject a mock client.
90-95:decodeThreadIdの再実行は 1 回にまとめられます。Line 90 で既に分解済みなので、Line 94 の
channelIdFromThreadId(threadId)は組み立てに置き換え可能です。小さな最適化ですが可読性も上がります。🔧 軽微な改善案
async fetchThread(threadId: string): Promise<ThreadInfo> { - const { issueKey, projectKey } = this.decodeThreadId(threadId); + const { issueKey, projectKey, spaceKey } = this.decodeThreadId(threadId); const issue = await this.client.getIssue(issueKey); return { id: threadId, - channelId: this.channelIdFromThreadId(threadId), + channelId: `backlog:${spaceKey}:${projectKey}`, channelName: projectKey,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adapter.ts` around lines 90 - 95, You already decode threadId into issueKey and projectKey via decodeThreadId(threadId); avoid re-running that parsing by replacing the call this.channelIdFromThreadId(threadId) with a direct construction that uses the decoded values (projectKey and/or issueKey) to produce the same channelId format that channelIdFromThreadId emits—i.e., reuse issueKey/projectKey to assemble the channelId inline or via a helper that accepts those parts, keeping decodeThreadId only once and improving readability.
🤖 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/adapter.test.ts`:
- Around line 55-87: Clear the mock call history before each test and assert
exact call counts to avoid false positives: add a beforeEach that calls
vi.clearAllMocks() (or vi.resetAllMocks()) so BacklogClient.prototype.getIssue
is cleared, and in each test that uses adapter.fetchThread assert both that
BacklogClient.prototype.getIssue was calledWith the expected issue key and that
it was called exactly once (e.g., toHaveBeenCalledTimes(1)); refer to the
existing tests that call adapter.fetchThread and
BacklogClient.prototype.getIssue to locate where to add the beforeEach and the
extra toHaveBeenCalledTimes assertions.
---
Nitpick comments:
In `@src/adapter.ts`:
- Around line 24-32: The constructor currently instantiates BacklogClient
directly (new BacklogClient(...)) which hinders testability; change the Adapter
to accept an optional BacklogClient via dependency injection (e.g., add an
optional parameter client?: BacklogClient to the constructor or a setter) and
fall back to new BacklogClient(config.host, config.apiKey) only when none is
provided; update the class field declaration for client and the constructor
logic in adapter.ts (symbols: BacklogClient, constructor, this.client) so unit
tests can inject a mock client.
- Around line 90-95: You already decode threadId into issueKey and projectKey
via decodeThreadId(threadId); avoid re-running that parsing by replacing the
call this.channelIdFromThreadId(threadId) with a direct construction that uses
the decoded values (projectKey and/or issueKey) to produce the same channelId
format that channelIdFromThreadId emits—i.e., reuse issueKey/projectKey to
assemble the channelId inline or via a helper that accepts those parts, keeping
decodeThreadId only once and improving readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21d706c0-fc41-4dfe-b82e-6d8db63f7557
📒 Files selected for processing (2)
src/adapter.test.tssrc/adapter.ts
| it("returns ThreadInfo for an issue thread", async () => { | ||
| vi.mocked(BacklogClient.prototype.getIssue).mockResolvedValue(mockIssue); | ||
|
|
||
| const result = await adapter.fetchThread("backlog:myspace:PROJ-123"); | ||
| expect(result).toEqual({ | ||
| id: "backlog:myspace:PROJ-123", | ||
| channelId: "backlog:myspace:PROJ", | ||
| channelName: "PROJ", | ||
| metadata: { | ||
| issueKey: "PROJ-123", | ||
| summary: "Test Issue", | ||
| issue: mockIssue, | ||
| }, | ||
| }); | ||
| expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123"); | ||
| }); | ||
|
|
||
| it("returns ThreadInfo for a comment thread", async () => { | ||
| vi.mocked(BacklogClient.prototype.getIssue).mockResolvedValue(mockIssue); | ||
|
|
||
| const result = await adapter.fetchThread("backlog:myspace:PROJ-123:456"); | ||
| expect(result).toEqual({ | ||
| id: "backlog:myspace:PROJ-123:456", | ||
| channelId: "backlog:myspace:PROJ", | ||
| channelName: "PROJ", | ||
| metadata: { | ||
| issueKey: "PROJ-123", | ||
| summary: "Test Issue", | ||
| issue: mockIssue, | ||
| }, | ||
| }); | ||
| expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123"); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 目的: テスト分離ガードの有無を静的に確認する(read-only)
sed -n '1,180p' src/adapter.test.ts
rg -n 'beforeEach|clearAllMocks|resetAllMocks|restoreAllMocks' src/adapter.test.ts
rg -n 'toHaveBeenCalledWith|toHaveBeenCalledTimes' src/adapter.test.tsRepository: plainbrew/chat-adapter-backlog
Length of output: 3264
モック呼び出し検証が偽陽性になる余地があります。
Line 69 / Line 86 は toHaveBeenCalledWith のみなので、前テストの呼び出し履歴が残っていても通る可能性があります。beforeEach でモックをクリアし、各テストで呼び出し回数も固定してください。
💡 修正案
describe("fetchThread", () => {
+ beforeEach(() => {
+ vi.clearAllMocks();
+ });
+
const mockIssue = {
@@
it("returns ThreadInfo for an issue thread", async () => {
@@
- expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
+ expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1);
+ expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
});
@@
it("returns ThreadInfo for a comment thread", async () => {
@@
- expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
+ expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1);
+ expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
});
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/adapter.test.ts` around lines 55 - 87, Clear the mock call history before
each test and assert exact call counts to avoid false positives: add a
beforeEach that calls vi.clearAllMocks() (or vi.resetAllMocks()) so
BacklogClient.prototype.getIssue is cleared, and in each test that uses
adapter.fetchThread assert both that BacklogClient.prototype.getIssue was
calledWith the expected issue key and that it was called exactly once (e.g.,
toHaveBeenCalledTimes(1)); refer to the existing tests that call
adapter.fetchThread and BacklogClient.prototype.getIssue to locate where to add
the beforeEach and the extra toHaveBeenCalledTimes assertions.
…fetchthread-for-backlogadapter
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/chat-adapter-backlog/src/adapter.test.ts (1)
55-87:⚠️ Potential issue | 🟡 Minorモック履歴のリセットと呼び出し回数の固定を追加してください。
toHaveBeenCalledWithのみだと前テスト由来の呼び出し履歴で通る余地があります。beforeEachで履歴をクリアし、各テストでtoHaveBeenCalledTimes(1)も入れてください。💡 修正案
describe("fetchThread", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + const mockIssue = { @@ it("returns ThreadInfo for an issue thread", async () => { @@ - expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123"); + expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1); + expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123"); }); @@ it("returns ThreadInfo for a comment thread", async () => { @@ - expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123"); + expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1); + expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123"); }); });#!/bin/bash # 目的: fetchThreadテストの分離ガード有無をread-onlyで確認 sed -n '1,220p' packages/chat-adapter-backlog/src/adapter.test.ts rg -n 'describe\("fetchThread"|beforeEach|clearAllMocks|toHaveBeenCalledTimes|toHaveBeenCalledWith' packages/chat-adapter-backlog/src/adapter.test.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat-adapter-backlog/src/adapter.test.ts` around lines 55 - 87, Add mock reset and strict call count checks: in the test suite's beforeEach add vi.clearAllMocks() (or vi.resetAllMocks()) so BacklogClient.prototype.getIssue history is cleared before each test, and update both tests that call adapter.fetchThread to assert BacklogClient.prototype.getIssue was called exactly once by adding expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1) in addition to the existing toHaveBeenCalledWith("PROJ-123"); this ensures calls to BacklogClient.prototype.getIssue are isolated per test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/chat-adapter-backlog/src/adapter.test.ts`:
- Around line 55-87: Add mock reset and strict call count checks: in the test
suite's beforeEach add vi.clearAllMocks() (or vi.resetAllMocks()) so
BacklogClient.prototype.getIssue history is cleared before each test, and update
both tests that call adapter.fetchThread to assert
BacklogClient.prototype.getIssue was called exactly once by adding
expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1) in addition to
the existing toHaveBeenCalledWith("PROJ-123"); this ensures calls to
BacklogClient.prototype.getIssue are isolated per test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d91f5c0c-4578-435b-82a4-a449f4af9674
📒 Files selected for processing (2)
packages/chat-adapter-backlog/src/adapter.test.tspackages/chat-adapter-backlog/src/adapter.ts
…fetchthread-for-backlogadapter
8937874 to
8d40fae
Compare
Summary
BacklogAdapterにBacklogClientを追加し、コンストラクタで初期化fetchThreadを実装: Backlog API で issue を取得しThreadInfoに変換して返すfetchThreadのテストを追加(issue スレッド・コメントスレッド両パターン)Test plan
pnpm testが全て通ることcloses https://linear.app/plainbrew/issue/PC-31
Summary by CodeRabbit
リリースノート
新機能
テスト
Chores