Skip to content

Conversation

@szuperaz
Copy link
Contributor

@szuperaz szuperaz commented Dec 17, 2025

🎫 Ticket: https://linear.app/stream/issue/XYZ-123

📑 Docs: https://github.com/GetStream/docs-content/pull/

💡 Overview

📝 Implementation notes

Summary by CodeRabbit

  • Tests
    • Added integration tests for cross-user feed invitation flows, including invite, rejection, and querying members (with moderator role coverage).
    • Expanded reaction tests to include querying activity and comment reactions using filtering (e.g., by reaction type).

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 17, 2025

Walkthrough

Adds integration tests: cross-user feed invitation flows (invite, reject, member queries) and reaction-query tests that exercise activity/comment reaction filtering. No production code changes.

Changes

Cohort / File(s) Summary
Feed invitation flow tests
packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
Adds setup for an inviting user and a separate client (john), upserts member invitations, tests rejection via rejectFeedMemberInvite, and adds member-query tests including a moderator-role query; duplicates a pattern for extra coverage.
Reaction query tests
packages/feeds-client/__integration-tests__/docs-snippets/reactions.test.ts
Adds a "Query Reactions" test exercising queryActivityReactions and queryCommentReactions with a { reaction_type: 'like' } filter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify test setup and teardown for multiple clients/users follow project conventions
  • Confirm assertions around invite upsert, rejection, and member-query roles are correct
  • Validate reaction query filters match API contract and test IDs used are valid

Poem

🐰 I hopped into tests with a twitch and a grin,

Invites sent, then politely declined—what a spin!
I sniffed for reactions, a like here, a cheer,
Members counted, roles found — all nicely clear.
🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description uses the required template structure but all content sections (Overview and Implementation notes) are empty, providing no meaningful information about the changes. Fill in the Overview section with a summary of what was added (cross-user feed invitation flow tests and reaction query tests), and add Implementation notes explaining the test coverage additions.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'test: more docs snippet tests' directly relates to the main changes, which add new test cases to documentation snippet test files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch docs-review-december

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • XYZ-123: Entity not found: Issue - Could not find referenced Issue.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@szuperaz szuperaz changed the title test: mode docs snippet tests test: more docs snippet tests Dec 17, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 83abe04 and 289682c.

📒 Files selected for processing (2)
  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts (2 hunks)
  • packages/feeds-client/__integration-tests__/docs-snippets/reactions.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript for all code in this repository

Files:

  • packages/feeds-client/__integration-tests__/docs-snippets/reactions.test.ts
  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
**/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in integration-tests/ directories

Files:

  • packages/feeds-client/__integration-tests__/docs-snippets/reactions.test.ts
  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to **/*.test.ts : Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in __integration-tests__/ directories
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to **/*.test.ts : Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in __integration-tests__/ directories

Applied to files:

  • packages/feeds-client/__integration-tests__/docs-snippets/reactions.test.ts
  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Prioritize backwards compatibility, API stability, and high test coverage when changing code in this TypeScript Feeds SDK repository

Applied to files:

  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
🧬 Code graph analysis (1)
packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts (1)
packages/feeds-client/src/feed/feed.ts (1)
  • feed (269-271)
🪛 GitHub Actions: Lint and test
packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts

[error] 238-238: ESLint: @typescript-eslint/no-shadow - 'feed' is already declared in the upper scope (line 15, column 7).

🪛 GitHub Check: lint-and-test (22.x)
packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts

[failure] 238-238:
'feed' is already declared in the upper scope on line 15 column 7

🔇 Additional comments (3)
packages/feeds-client/__integration-tests__/docs-snippets/reactions.test.ts (1)

92-109: LGTM! Test addition demonstrates query API with optional filters.

The new test case appropriately demonstrates querying reactions with optional filters. The implementation correctly shows both queryActivityReactions and queryCommentReactions usage patterns with filter parameters.

packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts (2)

16-26: LGTM! Cross-user test setup is well-structured.

The setup correctly initializes a separate client and user (John) for testing cross-user feed invitation workflows, which appropriately expands integration test coverage.


224-251: Test logic looks good once shadowing is fixed.

The new tests appropriately demonstrate:

  • Cross-user feed member invitation flow with upsert and rejection
  • Querying feed members with role-based filtering

The commented accept call on line 242 helpfully shows the alternative to rejection.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts (2)

23-23: Consider whether aliasing is necessary.

Both feed and invitingFeed point to the same Feed instance. If the intent is just to have a more descriptive name in the invitation context, this works, but it might be clearer to either use invitingFeed consistently or stick with feed throughout.


302-305: Consider disconnecting johnClient in cleanup.

The afterAll hook disconnects client but not johnClient, which was connected in beforeAll. While not critical for test functionality, cleaning up all resources improves test hygiene.

Apply this diff to add the cleanup:

 afterAll(async () => {
   await feed.delete({ hard_delete: true });
   await client.disconnectUser();
+  await johnClient.disconnectUser();
 });
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 289682c and 621045a.

📒 Files selected for processing (1)
  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript for all code in this repository

Files:

  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
**/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in integration-tests/ directories

Files:

  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to **/*.test.ts : Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in __integration-tests__/ directories
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to **/*.test.ts : Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in __integration-tests__/ directories

Applied to files:

  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Prioritize backwards compatibility, API stability, and high test coverage when changing code in this TypeScript Feeds SDK repository

Applied to files:

  • packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts
🧬 Code graph analysis (1)
packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts (2)
packages/feeds-client/src/feed/feed.ts (2)
  • Feed (161-1023)
  • feed (269-271)
packages/feeds-client/src/feeds-client/feeds-client.ts (1)
  • FeedsClient (111-1019)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint-and-test (22.x)
  • GitHub Check: lint-and-test (22.x)
🔇 Additional comments (3)
packages/feeds-client/__integration-tests__/docs-snippets/feed.test.ts (3)

16-18: LGTM! Clear test setup for cross-user scenarios.

The variable declarations properly set up the infrastructure for testing feed invitations across multiple users.


224-243: LGTM! Invitation flow correctly demonstrates cross-user interaction.

The test properly:

  • Creates an invitation from one user's feed
  • Switches to the invitee's client perspective
  • Demonstrates rejecting the invitation

The variable shadowing issue from the previous review has been resolved by using feedWithInvite.


245-251: LGTM! Demonstrates the feed members query API.

The test correctly shows how to query feed members with role-based filtering.

@szuperaz szuperaz merged commit 98412af into main Dec 17, 2025
7 of 9 checks passed
@szuperaz szuperaz deleted the docs-review-december branch December 17, 2025 14:59
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.

2 participants