Skip to content

[CHA-2563] Add retention policy integration tests#227

Open
hifaizsk wants to merge 4 commits intomainfrom
feature/cha-2563-retention-policy-endpoints
Open

[CHA-2563] Add retention policy integration tests#227
hifaizsk wants to merge 4 commits intomainfrom
feature/cha-2563-retention-policy-endpoints

Conversation

@hifaizsk
Copy link

@hifaizsk hifaizsk commented Mar 23, 2026

Ticket

Summary

  • Add integration tests for the 4 retention policy endpoints: SetRetentionPolicy, GetRetentionPolicy, GetRetentionPolicyRuns, DeleteRetentionPolicy
  • Tests cover the full CRUD lifecycle: create policy → get policy → get runs → delete policy
  • Graceful skip when retention feature is not enabled on the test app

Checklist

  • Integration tests for retention policy endpoints

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added chat retention policy management: view, create/update (with max-age), and delete policies.
    • Added ability to list retention policy cleanup runs with pagination and detailed run stats (counts, timestamps, error info).
  • Tests
    • Added tests covering retention policy operations and retrieval of cleanup runs.

@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

Warning

Rate limit exceeded

@hifaizsk has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 20 minutes and 41 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bfd3260-081e-4a89-9c0f-12576835c38c

📥 Commits

Reviewing files that changed from the base of the PR and between ca69edc and 9e76a30.

📒 Files selected for processing (1)
  • tests/test_chat_misc.py
📝 Walkthrough

Walkthrough

Added four ChatRestClient methods for retention policy management (get, set, delete, list runs), new dataclass models for retention policies and cleanup runs, and two tests that exercise setting/getting/deleting policies and fetching retention policy runs.

Changes

Cohort / File(s) Summary
Retention Policy Client Methods
getstream/chat/rest_client.py
Added four methods: get_retention_policy(), set_retention_policy(policy, max_age_hours), delete_retention_policy(policy), and get_retention_policy_runs(limit=None, offset=None) with telemetry decorators and existing request/response patterns.
Retention Policy Models
getstream/models/__init__.py
Added dataclasses: RetentionPolicyConfig, RetentionPolicy, RetentionCleanupRunStats, RetentionCleanupRun, and request/response types (SetRetentionPolicyRequest/Response, DeleteRetentionPolicyRequest/Response, GetRetentionPolicyResponse, GetRetentionPolicyRunsResponse) including datetime encoders/decoders.
Retention Policy Tests
tests/test_chat_misc.py
Added two tests: test_get_retention_policy and test_get_retention_policy_runs, each creating a policy via set_retention_policy, validating retrieval, and attempting cleanup in finally blocks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped to code a tidy rule,

set, get, delete—no messy spool.
Models snug, tests take a trot,
Cleanup runs check every spot,
Policies safe in our burrowed pool 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title focuses on adding integration tests, but the changeset includes significant implementation of four retention policy client methods and supporting models. The implementation is the primary change, with tests being secondary. Revise the title to reflect the main implementation work, such as: 'Add retention policy endpoints' or 'Implement retention policy client methods and integration tests'. The current title misrepresents the changeset scope.
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/cha-2563-retention-policy-endpoints

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.

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.

🧹 Nitpick comments (1)
getstream/models/__init__.py (1)

1825-1828: Note: Default type value applies to only one of four handled event types.

AsyncExportErrorEvent is used for export.bulk_image_moderation.error, export.channels.error, export.moderation_logs.error, and export.users.error (per getstream/webhook.py mapping). The default now specifically matches bulk_image_moderation.error.

This is fine for deserialization (the type comes from the payload), but if code instantiates this class directly without specifying type, it will default to bulk image moderation regardless of the actual error context.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@getstream/models/__init__.py` around lines 1825 - 1828, The
AsyncExportErrorEvent class currently sets type: str =
dc_field(default="export.bulk_image_moderation.error",
metadata=dc_config(field_name="type")), which biases direct instantiation to the
bulk_image_moderation event; remove the hardcoded default so the field is
required (e.g., change to type: str =
dc_field(metadata=dc_config(field_name="type")) or use an explicit
empty/default-less dc_field) and add optional runtime validation if needed;
reference AsyncExportErrorEvent, the type field declaration, dc_field/dc_config
usage, and the getstream/webhook.py mapping to ensure callers supply the correct
specific event type when constructing instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@getstream/models/__init__.py`:
- Around line 1825-1828: The AsyncExportErrorEvent class currently sets type:
str = dc_field(default="export.bulk_image_moderation.error",
metadata=dc_config(field_name="type")), which biases direct instantiation to the
bulk_image_moderation event; remove the hardcoded default so the field is
required (e.g., change to type: str =
dc_field(metadata=dc_config(field_name="type")) or use an explicit
empty/default-less dc_field) and add optional runtime validation if needed;
reference AsyncExportErrorEvent, the type field declaration, dc_field/dc_config
usage, and the getstream/webhook.py mapping to ensure callers supply the correct
specific event type when constructing instances.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 34692f6d-7d29-403d-9747-43880155166d

📥 Commits

Reviewing files that changed from the base of the PR and between 8ff29f0 and ceabba6.

📒 Files selected for processing (13)
  • getstream/chat/async_rest_client.py
  • getstream/chat/rest_client.py
  • getstream/common/async_rest_client.py
  • getstream/common/rest_client.py
  • getstream/feeds/rest_client.py
  • getstream/models/__init__.py
  • getstream/moderation/async_rest_client.py
  • getstream/moderation/rest_client.py
  • getstream/tests/test_webhook.py
  • getstream/video/async_rest_client.py
  • getstream/video/rest_client.py
  • getstream/webhook.py
  • tests/test_chat_misc.py

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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