Skip to content

Conversation

@aaguiarz
Copy link
Member

@aaguiarz aaguiarz commented Sep 10, 2025

Description

What problem is being solved?

How is it being solved?

What changes are made to solve it?

References

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

  • Documentation
    • Refined testing examples to illustrate multi-user and multi-object scenarios more clearly.
    • Replaced placeholder content with concrete assertions (e.g., member: true) and explicit user/group mappings.
    • Consolidated and removed redundant or confusing blocks to reduce ambiguity.
    • Improved formatting and indentation for readability.
    • Clarified examples to help users validate membership checks across different groups and users.

@aaguiarz aaguiarz requested review from a team as code owners September 10, 2025 16:57
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 10, 2025

Walkthrough

Updated README test YAML examples: replaced placeholders with explicit multi-user and multi-object scenarios, added member: true assertions, consolidated/removal of extraneous user blocks, and adjusted formatting/indentation.

Changes

Cohort / File(s) Summary
Docs: README updates
README.md
Rewrote test YAML examples: added multi-user (user:1, user:2) for group:employees with assertions member: true; added multi-object ([group:admins, group:employees]) for user:1 with member: true; removed separate user:3/user:4 block and exclusivity comments; fixed indentation/formatting.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Fix checks for multiple users and objects” directly reflects the primary update in the README’s sample YAML, where test blocks were revised to include explicit multi-user and multi-object assertions instead of placeholders, making it clear and specific to the main change.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title succinctly and accurately summarizes the primary change — documentation fixes for checks involving multiple users and objects — and the "chore(docs)" prefix correctly signals a docs-only change; it directly maps to the README edits described in the changeset.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/test-doc-fixes

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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
Contributor

@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 (1)
README.md (1)

315-320: Minor consistency nit: align indentation and wording with earlier examples.

  • Indentation before member: true is one space deeper than surrounding assertions; not wrong, but inconsistent.
  • Consider matching the earlier phrasing (“checks can group multiple objects that share the same expected results”).

Apply this diff if you want to normalize:

-        # checks can also target multiple objects with the same expectation
+        # checks can group multiple objects that share the same expected results
         - objects:
             - group:admins
             - group:employees
           user: user:1
           assertions:
-             member: true
+            member: true
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 04de3dc and aa5a688.

📒 Files selected for processing (1)
  • README.md (1 hunks)
⏰ 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). (1)
  • GitHub Check: Tests
🔇 Additional comments (1)
README.md (1)

306-313: Nice example for multi-user checks.

Matches the documented tests format (“users” + single “object”), and aligns with earlier examples in this README.

Copy link
Member

@Siddhant-K-code Siddhant-K-code left a comment

Choose a reason for hiding this comment

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

much cleaner and easy to grasp now!!

@Siddhant-K-code
Copy link
Member

Siddhant-K-code commented Sep 17, 2025

@aaguiarz, could you also format that YAML example so it’s easy for users to copy, paste, and use directly?

Edit: fixed in 6fe09ce

@rhamzeh rhamzeh changed the title Fix checks for multiple users and objects chore(docs): fix checks for multiple users and objects Sep 18, 2025
@rhamzeh rhamzeh added this pull request to the merge queue Sep 18, 2025
Merged via the queue into main with commit 1c87257 Sep 18, 2025
22 checks passed
@rhamzeh rhamzeh deleted the chore/test-doc-fixes branch September 18, 2025 13:37
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.

4 participants