Add Xquik social posting service#18
Open
kriptoburak wants to merge 2 commits into
Open
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Reviewer's GuideIntroduces a new Xquik-based social posting service wired into the existing social-posting module, including request/response mapping to the AgentOS social platform result model and focused tests using stubbed fetch calls. Sequence diagram for XquikSocialPostingService.publishPost flowsequenceDiagram
participant Caller
participant XquikSocialPostingService
participant SocialAbstractService
participant XquikAPI
Caller->>XquikSocialPostingService: publishPost(post, options)
XquikSocialPostingService->>XquikSocialPostingService: publish(input, options)
XquikSocialPostingService->>XquikSocialPostingService: createRequestBody(input)
XquikSocialPostingService->>SocialAbstractService: fetchJson(url, requestInit, options)
SocialAbstractService->>XquikAPI: POST /api/v1/x/tweets
XquikAPI-->>SocialAbstractService: XquikCreateTweetResponse
SocialAbstractService-->>XquikSocialPostingService: XquikCreateTweetResponse
alt [response.success]
XquikSocialPostingService-->>Caller: SocialPostPlatformResult(status=success, postId, url)
else [pending_confirmation]
XquikSocialPostingService-->>Caller: SocialPostPlatformResult(status=pending)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThis PR adds a new Xquik social posting service, exports its types from the social-posting barrel, and expands tests around request shaping, response handling, and platform-content fallback. ChangesXquik Social Posting Service
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant XquikSocialPostingService
participant XquikAPI
Caller->>XquikSocialPostingService: publishPost(post, options)
XquikSocialPostingService->>XquikSocialPostingService: createRequestBody(input)
XquikSocialPostingService->>XquikAPI: POST /api/v1/x/tweets (x-api-key)
XquikAPI-->>XquikSocialPostingService: tweet response or pending_confirmation
XquikSocialPostingService-->>Caller: SocialPostPlatformResult (success or pending)
Related PRs: None identified. Suggested labels: enhancement, social-posting Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoAdd Xquik social posting service with success/pending result mapping
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
![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/social-posting/XquikSocialPostingService.spec.ts (1)
44-79: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for a genuine (non-pending-confirmation) error response.
Current tests only exercise the
successand exactpending_confirmationshapes. Sincepublish()currently maps any other response to"pending"(see comment onXquikSocialPostingService.tsLines 82-95), a test asserting that a real error response yieldsstatus: "error"would have caught that gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/social-posting/XquikSocialPostingService.spec.ts` around lines 44 - 79, Add a test in XquikSocialPostingService.spec.ts that covers a non-pending-confirmation error response from XquikSocialPostingService.publish, using the existing fetchMock/service setup. Verify that when the API returns a real error shape (not the pending_confirmation case), publish() maps it to a platform result with status "error" instead of "pending".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/io/channels/social-posting/XquikSocialPostingService.ts`:
- Around line 82-95: The fallback in XquikSocialPostingService should not treat
every non-success response as pending, because that hides real failures and
drops pending metadata. Update the response handling in the posting method to
explicitly discriminate between the documented success and pending_confirmation
shapes, return pending only for a genuine pending_confirmation response while
preserving its writeActionId, and map any other unexpected or failed payload to
SocialPostPlatformResult.status = "error" with the available error details so
SocialPostManager can surface it correctly.
---
Nitpick comments:
In `@tests/social-posting/XquikSocialPostingService.spec.ts`:
- Around line 44-79: Add a test in XquikSocialPostingService.spec.ts that covers
a non-pending-confirmation error response from
XquikSocialPostingService.publish, using the existing fetchMock/service setup.
Verify that when the API returns a real error shape (not the
pending_confirmation case), publish() maps it to a platform result with status
"error" instead of "pending".
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 03be8d6c-e142-4240-a829-7ff04f3a8781
📒 Files selected for processing (3)
src/io/channels/social-posting/XquikSocialPostingService.tssrc/io/channels/social-posting/index.tstests/social-posting/XquikSocialPostingService.spec.ts
Comment on lines
+82
to
+95
| if ("success" in response && response.success) { | ||
| return { | ||
| platform: this.platform, | ||
| postId: response.tweetId, | ||
| publishedAt: new Date().toISOString(), | ||
| status: "success", | ||
| url: `https://x.com/i/web/status/${response.tweetId}`, | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| platform: this.platform, | ||
| status: "pending", | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Non-success responses are unconditionally treated as "pending", masking real errors.
The only two documented response shapes are success and pending_confirmation (Line 41-45), but the runtime response isn't validated against that discriminant before falling through. Any other error response (auth failure, rate limiting, validation error, unexpected payload) will also hit this branch and be reported as status: "pending" instead of status: "error", silently hiding failures from the rest of the posting pipeline (SocialPostPlatformResult.status per SocialPostManager.ts:58-71 supports an 'error' state specifically for this).
Additionally, the writeActionId from a genuine pending response is discarded — without it, there's no reference the caller can use later to reconcile/poll the tweet's final status.
🐛 Proposed fix to validate the discriminant and preserve error info
if ("success" in response && response.success) {
return {
platform: this.platform,
postId: response.tweetId,
publishedAt: new Date().toISOString(),
status: "success",
url: `https://x.com/i/web/status/${response.tweetId}`,
};
}
- return {
- platform: this.platform,
- status: "pending",
- };
+ if (
+ "status" in response &&
+ response.status === "pending_confirmation"
+ ) {
+ return {
+ platform: this.platform,
+ status: "pending",
+ };
+ }
+
+ return {
+ error: "Unexpected response from Xquik create-tweet endpoint",
+ platform: this.platform,
+ status: "error",
+ };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/io/channels/social-posting/XquikSocialPostingService.ts` around lines 82
- 95, The fallback in XquikSocialPostingService should not treat every
non-success response as pending, because that hides real failures and drops
pending metadata. Update the response handling in the posting method to
explicitly discriminate between the documented success and pending_confirmation
shapes, return pending only for a genuine pending_confirmation response while
preserving its writeActionId, and map any other unexpected or failed payload to
SocialPostPlatformResult.status = "error" with the available error details so
SocialPostManager can surface it correctly.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
publish, any non-successresponse is treated aspending; consider explicitly checking for thex_write_unconfirmed/pending_confirmationshape and surfacing unexpected response formats as errors to avoid silently misclassifying failures. - The returned
urlis alwayshttps://x.com/...even whenplatformand/orbaseUrlare overridden; if these configurations are meant to support alternative environments or platforms, consider deriving the URL from config to keep the result consistent with the target platform.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `publish`, any non-`success` response is treated as `pending`; consider explicitly checking for the `x_write_unconfirmed`/`pending_confirmation` shape and surfacing unexpected response formats as errors to avoid silently misclassifying failures.
- The returned `url` is always `https://x.com/...` even when `platform` and/or `baseUrl` are overridden; if these configurations are meant to support alternative environments or platforms, consider deriving the URL from config to keep the result consistent with the target platform.
## Individual Comments
### Comment 1
<location path="tests/social-posting/XquikSocialPostingService.spec.ts" line_range="6" />
<code_context>
+import { XquikSocialPostingService } from "../../src/io/channels/social-posting/XquikSocialPostingService.js";
+import type { SocialPost } from "../../src/io/channels/social-posting/SocialPostManager.js";
+
+describe("XquikSocialPostingService", () => {
+ afterEach(() => {
+ vi.restoreAllMocks();
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for all optional Xquik request fields (replyToTweetId, attachmentUrl, communityId, isNoteTweet, mediaUrls, account override) to lock in the camel→snake mapping behavior.
Right now the tests only cover `text` and the `mediaUrls`-when-empty behavior. Please add one or two focused tests that build an `XquikPublishInput` with these optional properties set and assert that the resulting `XquikCreateTweetBody` matches the expected shape. That will lock in the mapping behavior and guard against regressions if the request-building logic changes.
Suggested implementation:
```typescript
describe("XquikSocialPostingService", () => {
afterEach(() => {
vi.restoreAllMocks();
});
it("maps optional Xquik publish input fields to snake_case request body", async () => {
const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ success: true, tweetId: "12345" }), {
headers: { "content-type": "application/json" },
status: 200,
}),
);
vi.stubGlobal("fetch", fetchMock);
const service = new XquikSocialPostingService({
apiBaseUrl: "https://xquik.example.com",
apiKey: "test-api-key",
});
const post: SocialPost = {
platform: "x",
text: "Optional fields test",
mediaUrls: ["https://example.com/image.png"],
// Optional Xquik-specific fields we want to lock in
replyToTweetId: "9876543210",
attachmentUrl: "https://example.com/attachment.pdf",
communityId: "community-123",
isNoteTweet: true,
accountOverride: "override-account-id",
} as SocialPost;
await service.publish(post);
expect(fetchMock).toHaveBeenCalledTimes(1);
const [, requestInit] = fetchMock.mock.calls[0];
expect(requestInit).toBeDefined();
const body = JSON.parse((requestInit as RequestInit).body as string);
expect(body).toEqual({
text: post.text,
media_urls: post.mediaUrls,
reply_to_tweet_id: post.replyToTweetId,
attachment_url: post.attachmentUrl,
community_id: post.communityId,
is_note_tweet: post.isNoteTweet,
account_override: post.accountOverride,
});
});
it("publishes a tweet through the Xquik create tweet endpoint", async () => {
```
You may need to adjust:
1. The construction of `XquikSocialPostingService` (its constructor options) to match the actual implementation.
2. The `SocialPost` shape and property names (`replyToTweetId`, `attachmentUrl`, `communityId`, `isNoteTweet`, `accountOverride`) if your `SocialPost`/`XquikPublishInput` type uses different names.
3. The expected snake_case keys (`reply_to_tweet_id`, `attachment_url`, `community_id`, `is_note_tweet`, `account_override`) if the actual `XquikCreateTweetBody` uses slightly different field names.
4. The method name `publish` if the service exposes a different method for posting (e.g. `publishToX`, `publishTweet`, etc.).
</issue_to_address>
### Comment 2
<location path="tests/social-posting/XquikSocialPostingService.spec.ts" line_range="81" />
<code_context>
+ );
+ });
+
+ it("publishes the adapted platform content from a social post", async () => {
+ const fetchMock = vi.fn().mockResolvedValue(
+ new Response(JSON.stringify({ success: true, tweetId: "67890" }), {
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for the fallback order of `publishPost` content (platform adaptation → twitter adaptation → baseContent).
This only covers the happy path where `adaptations.x` exists. To validate the documented fallback logic, please add tests for: (1) `adaptations.x` missing but `adaptations.twitter` present, and (2) both missing so `baseContent` is used. Each should assert the exact body passed to `fetch`, as in the current test.
Suggested implementation:
```typescript
it("publishes the adapted platform content from a social post", async () => {
const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ success: true, tweetId: "67890" }), {
headers: { "content-type": "application/json" },
status: 200,
}),
);
vi.stubGlobal("fetch", fetchMock);
const service = new XquikSocialPostingService({
account: "@agent",
apiKey: "test-key",
platform: "x",
});
const socialPost = {
baseContent: "Base content",
adaptations: {
x: "X adaptation",
twitter: "Twitter adaptation",
},
media: ["https://example.com/image.png"],
};
await service.publishPost(socialPost);
const init = fetchMock.mock.calls[0]?.[1] as RequestInit;
expect(init.body).toBe(
JSON.stringify({
account: "@agent",
media: ["https://example.com/image.png"],
content: "X adaptation",
}),
);
});
it("falls back to twitter adaptation when platform-specific adaptation is missing", async () => {
const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ success: true, tweetId: "67890" }), {
headers: { "content-type": "application/json" },
status: 200,
}),
);
vi.stubGlobal("fetch", fetchMock);
const service = new XquikSocialPostingService({
account: "@agent",
apiKey: "test-key",
platform: "x",
});
const socialPost = {
baseContent: "Base content",
adaptations: {
twitter: "Twitter adaptation",
},
media: ["https://example.com/image.png"],
};
await service.publishPost(socialPost);
const init = fetchMock.mock.calls[0]?.[1] as RequestInit;
expect(init.body).toBe(
JSON.stringify({
account: "@agent",
media: ["https://example.com/image.png"],
content: "Twitter adaptation",
}),
);
});
it("falls back to baseContent when no adaptations are present", async () => {
const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ success: true, tweetId: "67890" }), {
headers: { "content-type": "application/json" },
status: 200,
}),
);
vi.stubGlobal("fetch", fetchMock);
const service = new XquikSocialPostingService({
account: "@agent",
apiKey: "test-key",
platform: "x",
});
const socialPost = {
baseContent: "Base content",
media: ["https://example.com/image.png"],
};
await service.publishPost(socialPost);
const init = fetchMock.mock.calls[0]?.[1] as RequestInit;
expect(init.body).toBe(
JSON.stringify({
account: "@agent",
media: ["https://example.com/image.png"],
content: "Base content",
}),
);
});
```
- Align the `socialPost` shape with the actual type used in this file (e.g., if there is a `SocialPost` type or helper factory, use it instead of a raw object).
- If the existing happy-path test already creates a `socialPost` fixture differently (e.g., via a factory or with additional fields), reuse that pattern to avoid duplication and keep the tests consistent.
- If `publishPost` requires additional options/parameters in this file, ensure those are passed in these new tests in the same way as in the existing happy-path test.
</issue_to_address>
### Comment 3
<location path="tests/social-posting/XquikSocialPostingService.spec.ts" line_range="24-33" />
<code_context>
+ account: "@agent",
+ apiKey: "test-key",
+ });
+ const result = await service.publish({ text: "hello" });
+
+ expect(result).toMatchObject({
</code_context>
<issue_to_address>
**suggestion (testing):** Assert that `publishedAt` is set on successful platform results to fully validate the mapping contract.
Since this test is validating the Xquik → AgentOS mapping, it should also cover the `publishedAt` field that the implementation sets. Please add an assertion that `result.publishedAt` is defined (and ideally a valid ISO timestamp) so this part of the mapping can’t be accidentally removed or changed without test failures.
```suggestion
const result = await service.publish({ text: "hello" });
expect(result).toMatchObject({
platform: "twitter",
postId: "12345",
status: "success",
url: "https://x.com/i/web/status/12345",
});
expect(result.publishedAt).toBeDefined();
expect(typeof result.publishedAt).toBe("string");
expect(Number.isNaN(Date.parse(result.publishedAt as string))).toBe(false);
const call = fetchMock.mock.calls[0];
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| import { XquikSocialPostingService } from "../../src/io/channels/social-posting/XquikSocialPostingService.js"; | ||
| import type { SocialPost } from "../../src/io/channels/social-posting/SocialPostManager.js"; | ||
|
|
||
| describe("XquikSocialPostingService", () => { |
There was a problem hiding this comment.
suggestion (testing): Add tests for all optional Xquik request fields (replyToTweetId, attachmentUrl, communityId, isNoteTweet, mediaUrls, account override) to lock in the camel→snake mapping behavior.
Right now the tests only cover text and the mediaUrls-when-empty behavior. Please add one or two focused tests that build an XquikPublishInput with these optional properties set and assert that the resulting XquikCreateTweetBody matches the expected shape. That will lock in the mapping behavior and guard against regressions if the request-building logic changes.
Suggested implementation:
describe("XquikSocialPostingService", () => {
afterEach(() => {
vi.restoreAllMocks();
});
it("maps optional Xquik publish input fields to snake_case request body", async () => {
const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ success: true, tweetId: "12345" }), {
headers: { "content-type": "application/json" },
status: 200,
}),
);
vi.stubGlobal("fetch", fetchMock);
const service = new XquikSocialPostingService({
apiBaseUrl: "https://xquik.example.com",
apiKey: "test-api-key",
});
const post: SocialPost = {
platform: "x",
text: "Optional fields test",
mediaUrls: ["https://example.com/image.png"],
// Optional Xquik-specific fields we want to lock in
replyToTweetId: "9876543210",
attachmentUrl: "https://example.com/attachment.pdf",
communityId: "community-123",
isNoteTweet: true,
accountOverride: "override-account-id",
} as SocialPost;
await service.publish(post);
expect(fetchMock).toHaveBeenCalledTimes(1);
const [, requestInit] = fetchMock.mock.calls[0];
expect(requestInit).toBeDefined();
const body = JSON.parse((requestInit as RequestInit).body as string);
expect(body).toEqual({
text: post.text,
media_urls: post.mediaUrls,
reply_to_tweet_id: post.replyToTweetId,
attachment_url: post.attachmentUrl,
community_id: post.communityId,
is_note_tweet: post.isNoteTweet,
account_override: post.accountOverride,
});
});
it("publishes a tweet through the Xquik create tweet endpoint", async () => {You may need to adjust:
- The construction of
XquikSocialPostingService(its constructor options) to match the actual implementation. - The
SocialPostshape and property names (replyToTweetId,attachmentUrl,communityId,isNoteTweet,accountOverride) if yourSocialPost/XquikPublishInputtype uses different names. - The expected snake_case keys (
reply_to_tweet_id,attachment_url,community_id,is_note_tweet,account_override) if the actualXquikCreateTweetBodyuses slightly different field names. - The method name
publishif the service exposes a different method for posting (e.g.publishToX,publishTweet, etc.).
| ); | ||
| }); | ||
|
|
||
| it("publishes the adapted platform content from a social post", async () => { |
There was a problem hiding this comment.
suggestion (testing): Add tests for the fallback order of publishPost content (platform adaptation → twitter adaptation → baseContent).
This only covers the happy path where adaptations.x exists. To validate the documented fallback logic, please add tests for: (1) adaptations.x missing but adaptations.twitter present, and (2) both missing so baseContent is used. Each should assert the exact body passed to fetch, as in the current test.
Suggested implementation:
it("publishes the adapted platform content from a social post", async () => {
const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ success: true, tweetId: "67890" }), {
headers: { "content-type": "application/json" },
status: 200,
}),
);
vi.stubGlobal("fetch", fetchMock);
const service = new XquikSocialPostingService({
account: "@agent",
apiKey: "test-key",
platform: "x",
});
const socialPost = {
baseContent: "Base content",
adaptations: {
x: "X adaptation",
twitter: "Twitter adaptation",
},
media: ["https://example.com/image.png"],
};
await service.publishPost(socialPost);
const init = fetchMock.mock.calls[0]?.[1] as RequestInit;
expect(init.body).toBe(
JSON.stringify({
account: "@agent",
media: ["https://example.com/image.png"],
content: "X adaptation",
}),
);
});
it("falls back to twitter adaptation when platform-specific adaptation is missing", async () => {
const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ success: true, tweetId: "67890" }), {
headers: { "content-type": "application/json" },
status: 200,
}),
);
vi.stubGlobal("fetch", fetchMock);
const service = new XquikSocialPostingService({
account: "@agent",
apiKey: "test-key",
platform: "x",
});
const socialPost = {
baseContent: "Base content",
adaptations: {
twitter: "Twitter adaptation",
},
media: ["https://example.com/image.png"],
};
await service.publishPost(socialPost);
const init = fetchMock.mock.calls[0]?.[1] as RequestInit;
expect(init.body).toBe(
JSON.stringify({
account: "@agent",
media: ["https://example.com/image.png"],
content: "Twitter adaptation",
}),
);
});
it("falls back to baseContent when no adaptations are present", async () => {
const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ success: true, tweetId: "67890" }), {
headers: { "content-type": "application/json" },
status: 200,
}),
);
vi.stubGlobal("fetch", fetchMock);
const service = new XquikSocialPostingService({
account: "@agent",
apiKey: "test-key",
platform: "x",
});
const socialPost = {
baseContent: "Base content",
media: ["https://example.com/image.png"],
};
await service.publishPost(socialPost);
const init = fetchMock.mock.calls[0]?.[1] as RequestInit;
expect(init.body).toBe(
JSON.stringify({
account: "@agent",
media: ["https://example.com/image.png"],
content: "Base content",
}),
);
});- Align the
socialPostshape with the actual type used in this file (e.g., if there is aSocialPosttype or helper factory, use it instead of a raw object). - If the existing happy-path test already creates a
socialPostfixture differently (e.g., via a factory or with additional fields), reuse that pattern to avoid duplication and keep the tests consistent. - If
publishPostrequires additional options/parameters in this file, ensure those are passed in these new tests in the same way as in the existing happy-path test.
Comment on lines
+24
to
+33
| const result = await service.publish({ text: "hello" }); | ||
|
|
||
| expect(result).toMatchObject({ | ||
| platform: "twitter", | ||
| postId: "12345", | ||
| status: "success", | ||
| url: "https://x.com/i/web/status/12345", | ||
| }); | ||
|
|
||
| const call = fetchMock.mock.calls[0]; |
There was a problem hiding this comment.
suggestion (testing): Assert that publishedAt is set on successful platform results to fully validate the mapping contract.
Since this test is validating the Xquik → AgentOS mapping, it should also cover the publishedAt field that the implementation sets. Please add an assertion that result.publishedAt is defined (and ideally a valid ISO timestamp) so this part of the mapping can’t be accidentally removed or changed without test failures.
Suggested change
| const result = await service.publish({ text: "hello" }); | |
| expect(result).toMatchObject({ | |
| platform: "twitter", | |
| postId: "12345", | |
| status: "success", | |
| url: "https://x.com/i/web/status/12345", | |
| }); | |
| const call = fetchMock.mock.calls[0]; | |
| const result = await service.publish({ text: "hello" }); | |
| expect(result).toMatchObject({ | |
| platform: "twitter", | |
| postId: "12345", | |
| status: "success", | |
| url: "https://x.com/i/web/status/12345", | |
| }); | |
| expect(result.publishedAt).toBeDefined(); | |
| expect(typeof result.publishedAt).toBe("string"); | |
| expect(Number.isNaN(Date.parse(result.publishedAt as string))).toBe(false); | |
| const call = fetchMock.mock.calls[0]; |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/social-posting/XquikSocialPostingService.spec.ts (2)
41-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBody assertions are coupled to exact key insertion order.
Comparing
init.bodyagainstJSON.stringify({...})string literals passes only because the literal's key order in each test happens to matchcreateRequestBody's field-insertion order. Any harmless reordering of fields increateRequestBody(e.g. movingtextaftermedia) would break these tests despite producing an equivalent JSON object.♻️ Suggested pattern: parse then compare structurally
- expect(init.body).toBe( - JSON.stringify({ account: "`@agent`", text: "hello" }), - ); + expect(JSON.parse(init.body as string)).toEqual({ + account: "`@agent`", + text: "hello", + });Also applies to: 75-80, 108-118, 171-173, 216-221
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/social-posting/XquikSocialPostingService.spec.ts` around lines 41 - 43, The body checks in XquikSocialPostingService.spec.ts are relying on exact JSON string order instead of the actual payload shape. Update the assertions around createRequestBody/init.body to parse the body and compare the resulting object structurally, so the tests stay stable if field insertion order changes. Apply the same approach in all affected XquikSocialPostingService.spec.ts cases using init.body and JSON.stringify literals.
154-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
SocialPostfixture construction across tests.The post literals in these two tests differ by only a few fields (
adaptations,baseContent,id,seedId). Consider extracting a small factory (e.g.buildTestPost(overrides)) to reduce repetition.Also applies to: 190-209
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/social-posting/XquikSocialPostingService.spec.ts` around lines 154 - 166, The SocialPost fixture is duplicated across multiple tests with only small field differences, so extract a shared factory in XquikSocialPostingService.spec.ts such as buildTestPost(overrides) and use it in the affected test cases. Keep the default SocialPost shape in one place and pass per-test overrides for fields like adaptations, baseContent, id, and seedId to reduce repetition and make future changes easier.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/social-posting/XquikSocialPostingService.spec.ts`:
- Around line 41-43: The body checks in XquikSocialPostingService.spec.ts are
relying on exact JSON string order instead of the actual payload shape. Update
the assertions around createRequestBody/init.body to parse the body and compare
the resulting object structurally, so the tests stay stable if field insertion
order changes. Apply the same approach in all affected
XquikSocialPostingService.spec.ts cases using init.body and JSON.stringify
literals.
- Around line 154-166: The SocialPost fixture is duplicated across multiple
tests with only small field differences, so extract a shared factory in
XquikSocialPostingService.spec.ts such as buildTestPost(overrides) and use it in
the affected test cases. Keep the default SocialPost shape in one place and pass
per-test overrides for fields like adaptations, baseContent, id, and seedId to
reduce repetition and make future changes easier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 525b7592-8644-4b1a-b60d-845353115d96
📒 Files selected for processing (2)
src/io/channels/social-posting/XquikSocialPostingService.tstests/social-posting/XquikSocialPostingService.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/io/channels/social-posting/XquikSocialPostingService.ts
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
Validation
Summary by Sourcery
Add a new Xquik-based social posting service and wire it into the existing social-posting module.
New Features:
Tests:
Summary by CodeRabbit