Skip to content

chore: Add tests for company transfer permissions#4396

Merged
Rockyy174 merged 1 commit into
operately:mainfrom
Rockyy174:company-transfer-permissions
Apr 24, 2026
Merged

chore: Add tests for company transfer permissions#4396
Rockyy174 merged 1 commit into
operately:mainfrom
Rockyy174:company-transfer-permissions

Conversation

@Rockyy174
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The permission snapshot helpers (member_people/1, guest_people/1, permission_resources/0, resource_records/2) rely heavily on hard-coded names; consider threading through IDs from the factory-generated records instead to make the tests more robust against renames and localization changes.
  • The various *_access_level helpers each embed their own access lookup logic (including a custom Ecto query for resource hubs); if there is a consolidated permission API (e.g., in Access.Fetch), it would be cleaner and less fragile to use that consistently rather than duplicating access-resolution logic.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The permission snapshot helpers (`member_people/1`, `guest_people/1`, `permission_resources/0`, `resource_records/2`) rely heavily on hard-coded names; consider threading through IDs from the factory-generated records instead to make the tests more robust against renames and localization changes.
- The various `*_access_level` helpers each embed their own access lookup logic (including a custom Ecto query for resource hubs); if there is a consolidated permission API (e.g., in `Access.Fetch`), it would be cleaner and less fragile to use that consistently rather than duplicating access-resolution logic.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Signed-off-by: Adriano Lazzaretti <lazzaretti136@gmail.com>
@Rockyy174 Rockyy174 merged commit 0c877ca into operately:main Apr 24, 2026
3 checks passed
@Rockyy174 Rockyy174 deleted the company-transfer-permissions branch April 24, 2026 15:48
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