feature/BA-multiple-profiles-invite-flow-accept-decline

[gabrielalmeida-tsl]

Summary by CodeRabbit

• New Features

  • Profile invitation workflow: send, accept, decline, cancel, and resend invitations via GraphQL with token-based expiry, status transitions, and mobile deep-linking with web fallback.
  • Email invitations with HTML/text templates and configurable acceptance URLs; invitees matched to existing accounts or invited by email with role assignment.

• Tests

  • Extensive test coverage for invitations: creation, tokens, expiry, state transitions, permissions, and email sending.

• Chores

  • DB schema migrations and new expiration constant for invitation handling; deterministic test factory keys.

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

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

• 🔍 Trigger a full review

Walkthrough

Implements a profile invitation system: model fields, migrations and constraints; token generation and expiry; email builders with deep-link fallback; GraphQL mutations for send/accept/decline/cancel/resend; templates, constants, permission branch, and extensive tests.

Changes

Cohort / File(s)	Summary
Models & migrations baseapp_profiles/models.py, baseapp_profiles/migrations/0007_profileuserrole_invitation_expires_at_and_more.py, baseapp_profiles/migrations/0008_profileuserrole_unique_profile_invited_email.py	Add invitation fields (invited_email, invitation_token, invited_at, invitation_expires_at, responded_at), new statuses (DECLINED, EXPIRED), make user nullable, add unique constraint on (profile, invited_email) when not null, and add is_invitation_expired() / generate_invitation_token() with migrations.
Email utilities & templates baseapp_profiles/emails.py, baseapp_profiles/templates/profiles/emails/invitation-subject.txt.j2, baseapp_profiles/templates/profiles/emails/invitation-body.txt.j2, baseapp_profiles/templates/profiles/emails/invitation-body.html.j2	New create_invitation() and send_invitation_email() to persist invitations, generate tokens, compute expiry (INVITATION_EXPIRATION_DAYS), attempt deep-link resolution with fallback to FRONT_ACCEPT_INVITATION_URL, render subject/text/html templates, and send mail.
GraphQL API baseapp_profiles/graphql/mutations.py, baseapp_profiles/graphql/object_types.py	Add mutations: ProfileSendInvitation, ProfileAcceptInvitation, ProfileDeclineInvitation, ProfileCancelInvitation, ProfileResendInvitation and expose on ProfilesMutations. Extend BaseProfileUserRoleObjectType with invitation fields and permission-gated resolvers; mutations handle token lifecycle, statuses, transactions, permissions, and email side effects.
Permissions baseapp_profiles/permissions.py	Add add_profileuserrole permission branch using owner/admin member logic for profile-scoped checks.
Constants & settings baseapp_profiles/constants.py, testproject/settings.py	Add INVITATION_EXPIRATION_DAYS = 7 and frontend/deep-link settings: FRONT_ACCEPT_INVITATION_URL, IOS_ACCEPT_INVITATION_DEEP_LINK, ANDROID_ACCEPT_INVITATION_DEEP_LINK.
Tests baseapp_profiles/tests/test_invitations.py	New comprehensive tests covering invitation creation, token generation/regeneration, expiry checks, and GraphQL flows (send/accept/decline/cancel/resend) with mocked email sending and DB assertions.
Misc baseapp_reports/tests/factories.py	Make ReportTypeFactory.key deterministic via factory.Sequence.

Sequence Diagram(s)

sequenceDiagram
    participant Inviter
    participant Server
    participant DB as Database
    participant DeepLink as DeepLink Service
    participant EmailSvc as Email Service
    participant Invitee

    Inviter->>Server: ProfileSendInvitation(email, role)
    activate Server
    Server->>DB: create_invitation(profile, inviter, email, role)
    activate DB
    DB->>DB: generate_invitation_token(), set status=PENDING, invited_at, invitation_expires_at
    DB-->>Server: invitation
    deactivate DB

    Server->>DeepLink: request_deeplink({token, ios/android hints})
    alt deep link available
        DeepLink-->>Server: deeplink_url
    else error / unavailable
        DeepLink-->>Server: error → fallback to FRONT_ACCEPT_INVITATION_URL
    end

    Server->>EmailSvc: send_invitation_email(invitation, inviter, accept_url)
    EmailSvc->>EmailSvc: render templates (subject, body html/text)
    EmailSvc-->>Invitee: email with accept_url/deeplink
    deactivate Server

Loading

sequenceDiagram
    participant Invitee
    participant Server
    participant DB as Database

    Invitee->>Server: GET accept_url (token)
    activate Server
    Server->>DB: fetch invitation by token
    DB-->>Server: invitation
    Server->>Server: validate token & expiry, check user/auth
    alt valid & authorized
        Server->>DB: set status=ACTIVE, responded_at, assign user (if needed)
        Server-->>Invitee: acceptance confirmation
    else expired/invalid/unauthorized
        Server-->>Invitee: error (expired/invalid/forbidden)
    end
    deactivate Server

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• Epic/HashIds #367 — Modifies baseapp_profiles/graphql/object_types.py; likely overlaps with invitation-related fields/resolvers.

Suggested reviewers

• Hercilio1
• vitorguima
• nossila

Poem

🐇 I nibble code and stitch a token bright,
An invitation bounces into light.
Deep links and emails, all hugged with care,
Click accept, join the circle we share.
🥕 Hooray — the rabbit welcomes you there.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.41% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title refers to an invitation flow feature (accept/decline) for multiple profiles, which directly aligns with the substantial changes implementing invitation creation, acceptance, decline, cancel, and resend mutations, plus email dispatch and database schema updates.

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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

baseapp_profiles/models.py (1)

197-234: Add validation for invitation state consistency.

The model lacks validation to ensure invitation-related fields are consistent. Consider adding a clean() method to enforce:

• If user is None, invited_email should be set
• If status is PENDING and user is None, invitation_token should be set
• Prevent conflicting states (e.g., both user and invited_email set)

🔎 Proposed validation implementation

+    def clean(self):
+        from django.core.exceptions import ValidationError
+        
+        # Validate invitation state
+        if self.user is None and not self.invited_email:
+            raise ValidationError({
+                'invited_email': _('Invited email is required when user is not set')
+            })
+        
+        if self.status == self.ProfileRoleStatus.PENDING and self.user is None:
+            if not self.invitation_token:
+                raise ValidationError({
+                    'invitation_token': _('Invitation token is required for pending invitations')
+                })
+        
+        super().clean()
+
     def __str__(self):
         return f"{self.user} as {self.role} in {self.profile}"

🤖 Fix all issues with AI Agents

In @baseapp_profiles/emails.py:
- Around line 14-37: The function create_invitation has an unused parameter
inviter; remove the inviter parameter from create_invitation's signature and all
call sites, or alternatively wire it into the created ProfileUserRole (e.g., set
a creator or invited_by field) if tracking is desired; update the function
definition (create_invitation) to drop inviter and delete any passing arguments,
or set the inviter on the ProfileUserRole before save (e.g.,
invitation.invited_by = inviter) and ensure the model field exists, then run
tests to ensure callers are updated accordingly.

In @baseapp_profiles/graphql/mutations.py:
- Around line 378-385: The code currently marks expired invitations by setting
invitation.status = ProfileUserRole.ProfileRoleStatus.DECLINED inside the block
that checks invitation.is_invitation_expired() and raises a GraphQLError; change
this to avoid confusing "declined" semantics by either (A) creating and using a
new enum value ProfileUserRole.ProfileRoleStatus.EXPIRED (add the enum member,
migrate any consumers, set invitation.status = EXPIRED, save, and keep/adjust
the GraphQLError), or (B) if you want to keep DECLINED, update the GraphQLError
message and any UI-facing text to explicitly say "expired" (e.g., raise
GraphQLError(str(_("This invitation has expired")),
extensions={"code":"expired_invitation"}) while keeping the status as DECLINED);
ensure you update any tests and downstream logic that inspects invitation.status
to handle the new EXPIRED value if you choose option A.

In @baseapp_profiles/graphql/object_types.py:
- Around line 40-41: The GraphQL field invitation_token is exposed without
queryset/field-level authorization and AcceptProfileInvitation accepts any valid
token regardless of recipient; update ProfileUserRoleObjectType to filter/expose
invitation_token only when the requesting user is the profile owner/admin or
matches the invited user (use the resolve_members logic as reference and add a
field resolver or queryset restriction for invitation_token), and modify the
AcceptProfileInvitation mutation to confirm the authenticated user is the
invitation recipient before accepting (e.g., load the Invitation instance in
AcceptProfileInvitation and check invitation.invited_email == request.user.email
or explicit authorization for profile admins, and reject otherwise).

In @baseapp_profiles/tests/test_invitations.py:
- Around line 1-9: Imports in test_invitations.py are incorrectly ordered
causing the pipeline failure; run isort (or manually reorder) so standard
library imports (pytest, datetime.timedelta) come first, then third-party
(django.utils.timezone, unittest.mock.patch) and finally local app imports, and
ensure within-group alphabetical ordering — specifically reorder the current
imports (pytest, timedelta, timezone, patch, ProfileUserRole, create_invitation,
ProfileFactory, UserFactory) or run `isort` on test_invitations.py to
automatically fix them.

🧹 Nitpick comments (10)

baseapp_profiles/templates/profiles/emails/invitation-subject.txt.j2 (1)

1-1: Add internationalization support to the subject template.

The subject line is not wrapped in a {% trans %} block, which makes it non-translatable. The HTML and text body templates use {% trans %} for internationalization, and this subject template should follow the same pattern for consistency.

🔎 Proposed fix to add i18n support

-You've been invited to join {{ profile_name }}
+{% trans profile=profile_name %}You've been invited to join {{ profile }}{% endtrans %}

baseapp_profiles/templates/profiles/emails/invitation-body.html.j2 (1)

1-16: Consider moving line break outside the translation block.

The <br /> tag on line 13 is embedded inside the {% trans %} block. This can complicate translations and may cause issues with some translation tools. Consider splitting this into two separate translation blocks or moving the line break outside.

🔎 Proposed refactor to improve translation handling

-<p class="t-caption gap-top-3 text-center">
-    {% trans %}Didn't expect this invitation?<br />{% endtrans %}
-    {% trans %}No worries, you can safely ignore this email.{% endtrans %}
-</p>
+<p class="t-caption gap-top-3 text-center">
+    {% trans %}Didn't expect this invitation?{% endtrans %}<br />
+    {% trans %}No worries, you can safely ignore this email.{% endtrans %}
+</p>

baseapp_profiles/templates/profiles/emails/invitation-body.txt.j2 (1)

1-10: Add internationalization support to the text body template.

The plain text email body lacks {% trans %} blocks for internationalization, while the HTML version (invitation-body.html.j2) properly uses them. This means users receiving plain text emails won't see translated content, creating an inconsistent experience.

🔎 Proposed fix to add i18n support

-Hi there,
+{% trans %}Hi there,{% endtrans %}
 
-{{ inviter_name }} has invited you to join {{ profile_name }} as a {{ role }}.
+{% trans inviter=inviter_name, profile=profile_name, role=role %}{{ inviter }} has invited you to join {{ profile }} as a {{ role }}.{% endtrans %}
 
-Click the link below to accept:
+{% trans %}Click the link below to accept:{% endtrans %}
 {{ accept_url }}
 
-This invitation will expire on {{ expiration_date }}.
+{% trans expiration=expiration_date %}This invitation will expire on {{ expiration }}.{% endtrans %}
 
-If you didn't expect this invitation, you can safely ignore this email.
+{% trans %}If you didn't expect this invitation, you can safely ignore this email.{% endtrans %}

baseapp_profiles/tests/test_invitations.py (2)

101-134: Verify email sending is properly tested.

The test mocks send_invitation_email and checks it was called, but doesn't verify the email was sent with correct parameters. Consider adding assertions to validate the email content, recipient, and URL.

💡 Enhanced test with email parameter verification

     @patch('baseapp_profiles.emails.send_invitation_email')
     def test_send_profile_invitation_mutation(self, mock_send_email, django_user_client, graphql_user_client):
         profile = ProfileFactory(owner=django_user_client.user)
 
         mutation = """
             mutation SendInvitation($input: SendProfileInvitationInput!) {
                 sendProfileInvitation(input: $input) {
                     profileUserRole {
                         id
                         invitedEmail
                         role
                         status
                     }
                 }
             }
         """
 
         variables = {
             "input": {
                 "profileId": profile.relay_id,
                 "email": "newmember@test.com",
                 "role": "MANAGER"
             }
         }
 
         response = graphql_user_client(mutation, variables=variables)
         content = response.json()
 
         if "errors" in content:
             print(f"GraphQL Errors: {content['errors']}")
 
         assert "data" in content
         assert content["data"]["sendProfileInvitation"]["profileUserRole"]["invitedEmail"] == "newmember@test.com"
         assert content["data"]["sendProfileInvitation"]["profileUserRole"]["status"] == "PENDING"
         assert mock_send_email.called
+        # Verify email parameters
+        mock_send_email.assert_called_once()
+        call_args = mock_send_email.call_args
+        invitation_arg = call_args[0][0]
+        inviter_arg = call_args[0][1]
+        assert invitation_arg.invited_email == "newmember@test.com"
+        assert inviter_arg == django_user_client.user

---

211-239: Add verification that invitation is actually deleted.

The test asserts the mutation returns success and checks the invitation no longer exists in the database. However, it might be clearer to explicitly verify the deletion was successful by checking the count before and after.

💡 More explicit deletion verification

     def test_cancel_profile_invitation_mutation(self, django_user_client, graphql_user_client):
         profile = ProfileFactory(owner=django_user_client.user)
 
         invitation = create_invitation(
             profile=profile,
             inviter=django_user_client.user,
             invited_email="test@test.com"
         )
+        
+        invitation_pk = invitation.pk
 
         mutation = """
             mutation CancelInvitation($input: CancelProfileInvitationInput!) {
                 cancelProfileInvitation(input: $input) {
                     success
                 }
             }
         """
 
         variables = {
             "input": {
                 "invitationId": invitation.relay_id
             }
         }
 
         response = graphql_user_client(mutation, variables=variables)
         content = response.json()
 
         assert content["data"]["cancelProfileInvitation"]["success"] is True
 
-        assert not ProfileUserRole.objects.filter(pk=invitation.pk).exists()
+        # Verify invitation was deleted
+        assert not ProfileUserRole.objects.filter(pk=invitation_pk).exists()

baseapp_profiles/emails.py (1)

75-81: Consider adding error handling for email sending failures.

The send_mail call could raise exceptions (e.g., SMTPException, ConnectionError). Consider wrapping this in a try-except block to handle failures gracefully or let them propagate to be handled by the caller.

💡 Add error handling for email sending

+    from django.core.mail import SMTPException
+    
+    try:
         send_mail(
             subject,
             message,
             html_message=html_message,
             from_email=None,
             recipient_list=[invitation.invited_email],
         )
+    except (SMTPException, ConnectionError) as e:
+        # Log error and re-raise or handle gracefully
+        import logging
+        logger = logging.getLogger(__name__)
+        logger.error(f"Failed to send invitation email to {invitation.invited_email}: {e}")
+        raise

baseapp_profiles/graphql/mutations.py (2)

432-439: Code duplication: Expired invitation handling.

The expired invitation handling logic is duplicated between AcceptProfileInvitation (lines 378-385) and DeclineProfileInvitation (lines 432-439). Consider extracting this into a shared helper method.

💡 Extract shared expiration check

+def _validate_invitation_not_expired(invitation):
+    """Validate invitation hasn't expired and mark as declined if it has."""
+    from django.utils import timezone
+    
+    if invitation.is_invitation_expired():
+        invitation.status = ProfileUserRole.ProfileRoleStatus.DECLINED
+        invitation.responded_at = timezone.now()
+        invitation.save()
+        raise GraphQLError(
+            str(_("This invitation has expired")),
+            extensions={"code": "expired_invitation"},
+        )
+
 class AcceptProfileInvitation(RelayMutation):
     # ...
     @classmethod
     @login_required
     def mutate_and_get_payload(cls, root, info, **input):
-        from django.utils import timezone
-
         token = input.get("token")

         try:
             invitation = ProfileUserRole.objects.get(invitation_token=token)
         except ProfileUserRole.DoesNotExist:
             raise GraphQLError(
                 str(_("Invalid invitation token")),
                 extensions={"code": "invalid_token"},
             )

-        if invitation.is_invitation_expired():
-            invitation.status = ProfileUserRole.ProfileRoleStatus.DECLINED
-            invitation.responded_at = timezone.now()
-            invitation.save()
-            raise GraphQLError(
-                str(_("This invitation has expired")),
-                extensions={"code": "expired_invitation"},
-            )
+        _validate_invitation_not_expired(invitation)
         
         # ... rest of the method

Apply the same refactor to DeclineProfileInvitation.

---

447-451: Minor inconsistency: User validation differs from AcceptProfileInvitation.

In DeclineProfileInvitation, you only check user mismatch if invitation.user exists. In AcceptProfileInvitation, you assign the user if it's None. Consider if declining should also assign the user for consistency.

baseapp_profiles/models.py (2)

279-284: Consider adding retry logic for token uniqueness.

While collision is astronomically unlikely with 64 random characters, the method doesn't handle potential IntegrityError if a duplicate token is generated. Consider adding a retry loop for defensive programming.

🔎 Proposed implementation with retry logic

 def generate_invitation_token(self):
     from django.utils.crypto import get_random_string
-
-    token = get_random_string(length=64)
+    
+    max_attempts = 5
+    for _ in range(max_attempts):
+        token = get_random_string(length=64)
+        # Check if token already exists
+        if not self.__class__.objects.filter(invitation_token=token).exists():
+            break
+    else:
+        raise ValueError("Failed to generate unique invitation token after multiple attempts")
+    
     self.invitation_token = token
     return token

---

286-287: Handle nullable user in __str__ method.

With user now nullable for pending invitations, the string representation should handle the None case more gracefully.

🔎 Proposed fix to display invited_email for pending invitations

 def __str__(self):
-    return f"{self.user} as {self.role} in {self.profile}"
+    user_display = self.user or self.invited_email or "Pending invitation"
+    return f"{user_display} as {self.role} in {self.profile}"

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ea15d7 and a82d4ce.

📒 Files selected for processing (11)

• baseapp_profiles/emails.py
• baseapp_profiles/graphql/mutations.py
• baseapp_profiles/graphql/object_types.py
• baseapp_profiles/migrations/0007_profileuserrole_invitation_expires_at_and_more.py
• baseapp_profiles/models.py
• baseapp_profiles/permissions.py
• baseapp_profiles/templates/profiles/emails/invitation-body.html.j2
• baseapp_profiles/templates/profiles/emails/invitation-body.txt.j2
• baseapp_profiles/templates/profiles/emails/invitation-subject.txt.j2
• baseapp_profiles/tests/test_invitations.py
• testproject/settings.py

🧰 Additional context used

🧬 Code graph analysis (4)

baseapp_profiles/tests/test_invitations.py (5)

baseapp_profiles/models.py (6)

• ProfileUserRole (296-298)
• ProfileRoles (198-204)
• ProfileRoleStatus (206-214)
• is_invitation_expired (272-277)
• save (170-175)
• generate_invitation_token (279-284)

baseapp_profiles/emails.py (1)

• create_invitation (14-37)

baseapp_payments/tests/factories.py (1)

• UserFactory (5-12)

baseapp_core/tests/fixtures.py (1)

• django_user_client (59-63)

baseapp_core/graphql/testing/fixtures.py (1)

• graphql_user_client (108-112)

baseapp_profiles/graphql/mutations.py (4)

baseapp_core/graphql/utils.py (2)

• get_object_type_for_model (51-55)
• get_pk_from_relay_id (21-23)

baseapp_profiles/models.py (5)

• ProfileUserRole (296-298)
• Profile (192-194)
• ProfileRoleStatus (206-214)
• is_invitation_expired (272-277)
• save (170-175)

baseapp_profiles/emails.py (2)

• create_invitation (14-37)
• send_invitation_email (40-81)

baseapp_profiles/permissions.py (1)

• has_perm (11-67)

baseapp_profiles/emails.py (2)

baseapp_profiles/models.py (5)

• ProfileUserRole (296-298)
• ProfileRoles (198-204)
• ProfileRoleStatus (206-214)
• generate_invitation_token (279-284)
• save (170-175)

baseapp_auth/models.py (1)

• get_full_name (110-113)

baseapp_profiles/models.py (1)

baseapp_profiles/graphql/object_types.py (4)

• Meta (31-46)
• Meta (50-51)
• Meta (133-137)
• Meta (171-172)

🪛 GitHub Actions: gabrielalmeida-tsl is checking refs/heads/feature/BA-2939-fe-multiple-profiles-invite-flow-accept-decline branch 🚀

baseapp_profiles/tests/test_invitations.py

[error] 1-1: isort check failed. Imports are incorrectly sorted and/or formatted. Command: 'docker compose exec backend isort --check ./'

🪛 GitHub Actions: gabrielalmeida-tsl is checking refs/heads/feature/BA-2939-multiple-profiles-invite-flow-accept-decline branch 🚀

baseapp_profiles/tests/test_invitations.py

[error] 1-1: Imports are incorrectly sorted and/or formatted.

🪛 Ruff (0.14.10)

testproject/settings.py

253-253: FRONT_URL may be undefined, or defined from star imports

(F405)

baseapp_profiles/graphql/object_types.py

34-45: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

baseapp_profiles/graphql/mutations.py

302-302: Unused class method argument: root

(ARG003)

---

365-365: Unused class method argument: root

(ARG003)

---

373-376: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

---

419-419: Unused class method argument: root

(ARG003)

---

427-430: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

---

468-468: Unused class method argument: root

(ARG003)

---

475-478: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

baseapp_profiles/emails.py

14-14: Unused function argument: inviter

(ARG001)

baseapp_profiles/models.py

270-270: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

baseapp_profiles/migrations/0007_profileuserrole_invitation_expires_at_and_more.py

10-13: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

---

15-88: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

🔇 Additional comments (18)

baseapp_profiles/permissions.py (1)

52-67: LGTM! Permission logic correctly extends to invitation flow.

The addition of add_profileuserrole to the permission checks is consistent with the existing pattern and correctly allows profile owners and admins to send invitations.

testproject/settings.py (1)

253-265: LGTM! Configuration follows existing patterns.

The new invitation URL and deep-link constants are correctly structured and consistent with the existing configuration pattern for email confirmation, password reset, and email change flows (lines 248-252, 256-264).

Note: The static analysis tool flagged FRONT_URL as potentially undefined due to star imports. However, this is consistent with the existing codebase pattern (see lines 248-252 which also use FRONT_URL), and star imports are acceptable in test settings files.

baseapp_profiles/graphql/object_types.py (1)

34-45: LGTM! Fields properly expose invitation data.

The expanded fields list correctly includes all invitation-related attributes needed for the GraphQL API, aligning with the database migration changes.

baseapp_profiles/tests/test_invitations.py (1)

14-34: LGTM! Comprehensive invitation creation test.

The test thoroughly validates invitation creation, including token generation, role assignment, status, timestamps, and the 15-day expiration period.

baseapp_profiles/emails.py (2)

40-61: LGTM! Robust deep link handling with fallback.

The deep link generation with exception handling provides a solid fallback mechanism when deep link services are unavailable. The try-except-else pattern is appropriate here.

---

63-69: The inviter parameter is a User instance which inherits from AbstractUser (defined in baseapp_auth/models.py). The get_full_name() method is properly implemented on the AbstractUser class and safely returns a full name or falls back to email. The code at line 64 is correct and requires no changes.

baseapp_profiles/migrations/0007_profileuserrole_invitation_expires_at_and_more.py (3)

26-37: LGTM! Unique constraint on invitation_token is appropriate.

The unique constraint on invitation_token ensures each invitation has a distinct token, which is essential for security and prevents token collision.

---

76-87: LGTM! Nullable user field enables invitations to non-existent users.

Making the user field nullable is the correct approach for supporting invitations sent to email addresses not yet registered in the system. The invitation can be linked to a user account later when they accept.

---

69-75: Remove this comment—the concern is based on incorrect information.

The status field has had a default of PENDING (2) since its initial creation in migration 0005. Migration 0007 only adds a new choice option ("declined") without changing the default. The single ProfileUserRole creation in the codebase explicitly sets the status, and the factory appropriately relies on the PENDING default that is already correct in the model.

Likely an incorrect or invalid review comment.

baseapp_profiles/graphql/mutations.py (5)

292-354: LGTM! Comprehensive duplicate and member validation.

The mutation properly checks for existing pending invitations and active members before creating a new invitation. The permission check and email sending flow are correctly implemented.

---

320-330: Consider case-insensitive duplicate check alignment.

The duplicate check uses invited_email__iexact (line 322) which is case-insensitive, matching the email.lower() normalization in create_invitation. This is good for consistency.

---

393-399: LGTM! Correct handling of user mismatch.

The logic properly handles invitations sent to non-existent users (assigns current user) and validates that existing users match the invitation recipient.

---

488-492: Good validation: Only pending invitations can be cancelled.

The check to only allow cancelling pending invitations is correct and prevents unintended state changes.

---

505-508: LGTM! All mutations properly registered.

The four new invitation mutations are correctly registered on the ProfilesMutations class.

baseapp_profiles/models.py (4)

210-210: LGTM: New status enum value.

The DECLINED status addition is consistent with the existing enum pattern and properly supports the invitation decline flow.

---

235-266: LGTM: Well-designed invitation fields.

The invitation-related fields are properly structured with:

• Appropriate nullability for optional invitation data
• Unique constraint on invitation_token for security
• Clear help text for each field's purpose

---

272-277: LGTM: Correct expiration logic.

The expiration check correctly handles the None case and uses timezone-aware comparison.

---

216-223: I need you to provide the review comment that you would like me to rewrite. Please share:

1. The original review comment text
2. The relevant code context or file path
3. Any additional context about the repository or pull request

[nossila]

In my opinion we don't need the random generated token and the expiration fields, we could use https://docs.djangoproject.com/en/6.0/topics/signing, which handles both cases without the need to save them to the table

I'm fine with current implementation, I'm just commenting to let you know for future implementations that might need a similar thing.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@baseapp_profiles/graphql/mutations.py`:
- Around line 310-311: The call to Profile.objects.get(pk=profile_pk) (after
get_pk_from_relay_id(profile_id)) can raise Profile.DoesNotExist and must be
handled; wrap the lookup in a try/except catching Profile.DoesNotExist and
convert it into a GraphQL-friendly error (e.g., raise GraphQLError or return the
mutation error payload) with a clear message like "Profile not found" including
the provided profile_id; ensure you import GraphQLError (or use the project's
standard GraphQL error helper) and keep the original flow/return shape expected
by this mutation.
- Around line 321-336: The code only checks PENDING and ACTIVE; add handling for
ProfileUserRole.ProfileRoleStatus.DECLINED and .INACTIVE by checking
existing_role before calling create_invitation: for DECLINED/INACTIVE either
transition the existing_role back to PENDING (e.g., set existing_role.status =
ProfileUserRole.ProfileRoleStatus.PENDING, update invited_* fields as
appropriate, call existing_role.save() and proceed without creating a duplicate)
or reject the re-invite with a GraphQLError (provide distinct extensions codes
like "re_invite_declined" / "re_invite_inactive") to prevent creating duplicate
records or violating the unique_together constraint; update logic around
existing_role and the create_invitation call accordingly.

In `@baseapp_profiles/graphql/object_types.py`:
- Around line 34-44: The new PII fields (invited_email, invited_at,
invitation_expires_at, responded_at) are exposed on the node type and can bypass
permission checks; update the GraphQL type in
baseapp_profiles/graphql/object_types.py to gate those fields by the same
permission used by resolve_members (view_profile_members). Implement this by
either overriding get_queryset on the ProfileUserRole (or the
DjangoObjectType/class representing these nodes) to filter out rows unless the
requesting user has view_profile_members, or add explicit field resolvers for
invited_email and the timestamp fields that check context.user permissions
(calling the existing view_profile_members logic) and return None or raise a
permission error when unauthorized. Ensure you reference the existing
resolve_members permission helper when performing the check.

♻️ Duplicate comments (3)

baseapp_profiles/emails.py (1)

14-37: Drop or use the unused inviter parameter.
inviter is still unused in create_invitation, which makes the API misleading and triggers lint failures.

🧰 Proposed adjustment

-def create_invitation(profile, inviter, invited_email, role=ProfileUserRole.ProfileRoles.MANAGER):
+def create_invitation(profile, invited_email, role=ProfileUserRole.ProfileRoles.MANAGER):

Update call sites accordingly (or wire inviter into a persisted field if needed).

baseapp_profiles/tests/test_invitations.py (1)

1-11: Fix import ordering to satisfy isort checks.
Current grouping/order likely still fails the existing isort gate.

🧹 Proposed reorder

-from datetime import timedelta
-from unittest.mock import patch
-
-import pytest
-from django.utils import timezone
+from datetime import timedelta
+from unittest.mock import patch
+
+from django.utils import timezone
+import pytest

baseapp_profiles/graphql/mutations.py (1)

370-377: Expired invitations marked as DECLINED - previous concern still applies.

Setting status = DECLINED for expired invitations may confuse users who see "declined" when the invitation actually expired. Consider using a separate EXPIRED status or updating the error message to clarify.

🧹 Nitpick comments (5)

baseapp_profiles/graphql/mutations.py (3)

362-368: Use exception chaining for better debugging.

When re-raising a different exception within an except block, use raise ... from None or raise ... from err to properly chain exceptions. This improves debugging by clarifying the exception origin.

♻️ Proposed fix

         try:
             invitation = ProfileUserRole.objects.get(invitation_token=token)
         except ProfileUserRole.DoesNotExist:
             raise GraphQLError(
                 str(_("Invalid invitation token")),
                 extensions={"code": "invalid_token"},
-            )
+            ) from None

---

414-420: Use exception chaining for better debugging.

Same as AcceptProfileInvitation - use raise ... from None for proper exception handling.

♻️ Proposed fix

         try:
             invitation = ProfileUserRole.objects.get(invitation_token=token)
         except ProfileUserRole.DoesNotExist:
             raise GraphQLError(
                 str(_("Invalid invitation token")),
                 extensions={"code": "invalid_token"},
-            )
+            ) from None

---

462-468: Use exception chaining for better debugging.

Same pattern - use raise ... from None for proper exception handling.

♻️ Proposed fix

         try:
             invitation = ProfileUserRole.objects.get(pk=invitation_pk)
         except ProfileUserRole.DoesNotExist:
             raise GraphQLError(
                 str(_("Invitation not found")),
                 extensions={"code": "not_found"},
-            )
+            ) from None

baseapp_profiles/models.py (2)

279-284: Token generation is secure but consider collision handling.

Using get_random_string(length=64) with the default charset provides good entropy. However, there's a very small theoretical chance of token collision since the field has unique=True. Consider adding retry logic or checking for existing tokens, though the probability is extremely low with 64 characters.

💡 Optional: Add collision retry

     def generate_invitation_token(self):
         from django.utils.crypto import get_random_string
 
-        token = get_random_string(length=64)
-        self.invitation_token = token
-        return token
+        ProfileUserRole = swapper.load_model("baseapp_profiles", "ProfileUserRole")
+        while True:
+            token = get_random_string(length=64)
+            if not ProfileUserRole.objects.filter(invitation_token=token).exists():
+                self.invitation_token = token
+                return token

---

286-287: Fix potential AttributeError when user is None.

The __str__ method references self.user, but since user is now nullable, this could display None as <role> in <profile> which may be acceptable, but could also cause issues if any code expects a string representation of a user object.

💡 Optional: Improve string representation for pending invitations

     def __str__(self):
-        return f"{self.user} as {self.role} in {self.profile}"
+        user_display = self.user or self.invited_email or "Pending"
+        return f"{user_display} as {self.role} in {self.profile}"

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Handle Profile.DoesNotExist exception.

Profile.objects.get(pk=profile_pk) can raise Profile.DoesNotExist if an invalid ID is provided. This will result in an unhandled exception rather than a user-friendly GraphQL error.

🐛 Proposed fix

         profile_pk = get_pk_from_relay_id(profile_id)
-        profile = Profile.objects.get(pk=profile_pk)
+        try:
+            profile = Profile.objects.get(pk=profile_pk)
+        except Profile.DoesNotExist:
+            raise GraphQLError(
+                str(_("Profile not found")),
+                extensions={"code": "not_found"},
+            ) from None

🤖 Prompt for AI Agents

In `@baseapp_profiles/graphql/mutations.py` around lines 310 - 311, The call to
Profile.objects.get(pk=profile_pk) (after get_pk_from_relay_id(profile_id)) can
raise Profile.DoesNotExist and must be handled; wrap the lookup in a try/except
catching Profile.DoesNotExist and convert it into a GraphQL-friendly error
(e.g., raise GraphQLError or return the mutation error payload) with a clear
message like "Profile not found" including the provided profile_id; ensure you
import GraphQLError (or use the project's standard GraphQL error helper) and
keep the original flow/return shape expected by this mutation.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@baseapp_profiles/models.py`:
- Around line 256-261: The help_text for the DateTimeField invitation_expires_at
is incorrect (mentions 15 days) and should match the 7-day logic used elsewhere;
update the verbose/help text for invitation_expires_at to state "When the
invitation expires (7 days from invited_at)" so it aligns with the
creation/resend logic that sets a 7-day expiry.

♻️ Duplicate comments (3)

baseapp_profiles/emails.py (1)

14-15: Remove or use the unused inviter argument.

inviter isn’t referenced in create_invitation, so it adds noise unless you plan to persist it. Either drop it from the signature/callers or store it on the invitation.

baseapp_profiles/graphql/mutations.py (2)

311-313: Handle missing profile IDs to avoid unhandled errors.

Profile.objects.get(pk=profile_pk) can raise Profile.DoesNotExist, which currently bubbles into a 500. Convert this to a GraphQL error.

🐛 Proposed fix

-        profile = Profile.objects.get(pk=profile_pk)
+        try:
+            profile = Profile.objects.get(pk=profile_pk)
+        except Profile.DoesNotExist:
+            raise GraphQLError(
+                str(_("Profile not found")),
+                extensions={"code": "not_found"},
+            ) from None

---

322-337: Prevent duplicate roles when re‑inviting.

existing_role only checks invited_email and only blocks PENDING/ACTIVE. If a member exists without invited_email or has DECLINED/INACTIVE/EXPIRED status, this falls through and create_invitation can violate the (user, profile) unique constraint or create duplicates. Expand the lookup to include user__email and handle non‑pending statuses (either reuse the existing record or return a targeted error).

🐛 Suggested adjustment (lookup + status handling)

+from django.db.models import Q
...
-        existing_role = ProfileUserRole.objects.filter(
-            profile=profile,
-            invited_email__iexact=email,
-        ).first()
+        existing_role = (
+            ProfileUserRole.objects.filter(profile=profile)
+            .filter(Q(invited_email__iexact=email) | Q(user__email__iexact=email))
+            .first()
+        )
...
-            if existing_role.status == ProfileUserRole.ProfileRoleStatus.ACTIVE:
+            if existing_role.status == ProfileUserRole.ProfileRoleStatus.ACTIVE:
                 raise GraphQLError(
                     str(_("This user is already a member of this profile")),
                     extensions={"code": "already_member"},
                 )
+            if existing_role.status in [
+                ProfileUserRole.ProfileRoleStatus.DECLINED,
+                ProfileUserRole.ProfileRoleStatus.INACTIVE,
+                ProfileUserRole.ProfileRoleStatus.EXPIRED,
+            ]:
+                raise GraphQLError(
+                    str(_("An invitation already exists for this email")),
+                    extensions={"code": "invitation_exists"},
+                )

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@baseapp_profiles/graphql/mutations.py`:
- Around line 322-337: The existing lookup only checks invited_email and misses
existing roles linked by user (legacy rows with invited_email null), causing
potential (user, profile) unique constraint violations; update the existing_role
query in the block that builds ProfileUserRole lookup (variable existing_role)
to also check for an existing role by user (e.g., filter by
Q(invited_email__iexact=email) | Q(user=user) or include user if provided) for
the same profile, and keep the same status checks/GraphQLError raises
(duplicate_invitation / already_member) when a matching role is found.

♻️ Duplicate comments (2)

baseapp_profiles/emails.py (1)

14-37: Remove or use the unused inviter parameter.
It’s not referenced in Line 14–37; either drop it and update call sites, or persist it on the invitation.

♻️ Proposed fix (remove parameter)

-def create_invitation(profile, inviter, invited_email, role=ProfileUserRole.ProfileRoles.MANAGER):
+def create_invitation(profile, invited_email, role=ProfileUserRole.ProfileRoles.MANAGER):
     from django.contrib.auth import get_user_model

baseapp_profiles/graphql/mutations.py (1)

311-313: Handle missing profile lookup.
Profile.objects.get(pk=profile_pk) can raise Profile.DoesNotExist; convert to a GraphQL-friendly error.

🐛 Proposed fix

-        profile = Profile.objects.get(pk=profile_pk)
+        try:
+            profile = Profile.objects.get(pk=profile_pk)
+        except Profile.DoesNotExist:
+            raise GraphQLError(
+                str(_("Profile not found")),
+                extensions={"code": "not_found"},
+            ) from None

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@baseapp_profiles/graphql/mutations.py`:
- Around line 445-451: When invitation.user is None, do not blindly bind the
current logged-in user; first verify that info.context.user.email (or a
normalized form) matches invitation.invited_email and only then set
invitation.user = info.context.user, otherwise raise a GraphQLError (include an
extensions code like "wrong_user" or "email_mismatch"); also handle cases where
invited_email or user.email are missing. Apply the same email-match check to the
other similar block around the invitation.accept/decline logic referenced (the
block at ~496-500) so both paths enforce invited_email matching before attaching
info.context.user.

In `@baseapp_profiles/tests/test_invitations.py`:
- Around line 243-283: The test test_expired_invitation_sets_expired_status is
asserting the wrong final status: AcceptProfileInvitation mutation sets
invitation.status to EXPIRED before raising, so update the assertion to expect
ProfileUserRole.ProfileRoleStatus.EXPIRED rather than PENDING; locate the test
function and change the final assertion after invitation.refresh_from_db() to
compare invitation.status to ProfileUserRole.ProfileRoleStatus.EXPIRED (no other
behavioral changes), keeping other checks for the error code
"expired_invitation" intact.

♻️ Duplicate comments (1)

baseapp_profiles/emails.py (1)

15-38: inviter is still unused in create_invitation.

Either persist the inviter (audit trail) or remove it and update call sites to avoid an unused public parameter.

🧹 Suggested signature cleanup (update call sites accordingly)

-def create_invitation(profile, inviter, invited_email, role=ProfileUserRole.ProfileRoles.MANAGER):
+def create_invitation(profile, invited_email, role=ProfileUserRole.ProfileRoles.MANAGER):

🧹 Nitpick comments (1)

baseapp_profiles/tests/test_invitations.py (1)

438-448: Unused mock_send_email parameters.

If the mock isn’t asserted, rename to _mock_send_email (or add an assertion) to avoid unused‑argument warnings.

🧹 Rename unused mock parameters

-    def test_send_invitation_reuses_declined_row(
-        self, mock_send_email, django_user_client, graphql_user_client
-    ):
+    def test_send_invitation_reuses_declined_row(
+        self, _mock_send_email, django_user_client, graphql_user_client
+    ):
@@
-    def test_send_invitation_reuses_inactive_row(
-        self, mock_send_email, django_user_client, graphql_user_client
-    ):
+    def test_send_invitation_reuses_inactive_row(
+        self, _mock_send_email, django_user_client, graphql_user_client
+    ):
@@
-    def test_send_invitation_reuses_expired_row(
-        self, mock_send_email, django_user_client, graphql_user_client
-    ):
+    def test_send_invitation_reuses_expired_row(
+        self, _mock_send_email, django_user_client, graphql_user_client
+    ):
@@
-    def test_send_invitation_handles_expired_pending(
-        self, mock_send_email, django_user_client, graphql_user_client
-    ):
+    def test_send_invitation_handles_expired_pending(
+        self, _mock_send_email, django_user_client, graphql_user_client
+    ):

Also applies to: 486-498, 531-542, 647-659

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@baseapp_profiles/emails.py`:
- Around line 15-38: The create_invitation function has an unused inviter
parameter; remove inviter from the function signature in create_invitation and
update all callers (e.g., tests like test_invitations.py and any usages that
previously passed an inviter, as well as any code paths in ProfileSendInvitation
or other call sites) to call create_invitation(profile, invited_email, role=...)
or with positional args matching the new signature; ensure imports and tests are
updated accordingly and run tests to confirm no references to the removed
inviter remain.

🧹 Nitpick comments (3)

baseapp_profiles/models.py (1)

287-292: Token collision not explicitly handled in generate_invitation_token.

The method generates a random token and assigns it but doesn't retry on collision. While 64-character random strings have astronomically low collision probability, the invitation_token field has a unique constraint. If a collision occurs (extremely rare), save() will raise IntegrityError.

The current implementation is acceptable given the collision probability (~1 in 10^77), but consider either:

1. Adding a retry loop, or
2. Documenting that callers must handle IntegrityError (which ProfileSendInvitation mutation does handle via its try/except IntegrityError block).

baseapp_profiles/graphql/mutations.py (1)

398-404: Use raise ... from None for exception chaining.

Per static analysis (B904), when re-raising a different exception within an except clause, use from None to suppress the original exception context and clarify that this is a deliberate transformation.

✏️ Proposed fix

                 except IntegrityError:
                     # Race condition: another request created the row first.
                     # Re-fetch and return duplicate_invitation error.
                     raise GraphQLError(
                         str(_("An invitation has already been sent to this email")),
                         extensions={"code": "duplicate_invitation"},
-                    )
+                    ) from None

baseapp_profiles/tests/test_invitations.py (1)

126-129: Fail fast on GraphQL errors instead of printing.

Printing errors can let the test pass with partial data. Prefer an explicit assertion so failures surface immediately.

♻️ Proposed change

-        if "errors" in content:
-            print(f"GraphQL Errors: {content['errors']}")
+        assert "errors" not in content, f"GraphQL Errors: {content.get('errors')}"

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -nP --type=py '\bcreate_invitation\b' baseapp_profiles/

Repository: silverlogic/baseapp-backend

Length of output: 2739

---

🏁 Script executed:

# Also check in the entire repository for any imports or calls
rg -nP --type=py '\bcreate_invitation\b'

Repository: silverlogic/baseapp-backend

Length of output: 2739

---

🏁 Script executed:

# Check mutations.py to see how ProfileSendInvitation creates invitations
fd -e py mutations baseapp_profiles/

Repository: silverlogic/baseapp-backend

Length of output: 220

---

🏁 Script executed:

# Read the create_invitation function to check if inviter is used
sed -n '15,38p' baseapp_profiles/emails.py

Repository: silverlogic/baseapp-backend

Length of output: 831

---

🏁 Script executed:

# Check the test file to see how inviter is being passed
sed -n '650,660p' baseapp_profiles/tests/test_invitations.py
sed -n '810,825p' baseapp_profiles/tests/test_invitations.py

Repository: silverlogic/baseapp-backend

Length of output: 1105

---

🏁 Script executed:

# Check mutations.py to see how ProfileSendInvitation creates invitations
head -150 baseapp_profiles/graphql/mutations.py | tail -100

Repository: silverlogic/baseapp-backend

Length of output: 3943

---

🏁 Script executed:

# Search for ProfileSendInvitation in mutations.py
rg -n 'ProfileSendInvitation' baseapp_profiles/graphql/mutations.py -A 30

Repository: silverlogic/baseapp-backend

Length of output: 2822

---

🏁 Script executed:

# Also check if create_invitation is called from mutations.py
rg -n 'create_invitation' baseapp_profiles/graphql/mutations.py

Repository: silverlogic/baseapp-backend

Length of output: 53

---

🏁 Script executed:

# Get the full ProfileSendInvitation.mutate_and_get_payload method
sed -n '296,408p' baseapp_profiles/graphql/mutations.py

Repository: silverlogic/baseapp-backend

Length of output: 5085

---

Remove unused inviter parameter from create_invitation.

The inviter parameter is never used in the function body and should be removed from the signature. While ProfileSendInvitation in mutations.py creates invitations directly with its own logic, create_invitation is actively used in test_invitations.py and serves as a utility function for tests.

🧰 Tools

🪛 Ruff (0.14.14)

15-15: Unused function argument: inviter

(ARG001)

🤖 Prompt for AI Agents

In `@baseapp_profiles/emails.py` around lines 15 - 38, The create_invitation
function has an unused inviter parameter; remove inviter from the function
signature in create_invitation and update all callers (e.g., tests like
test_invitations.py and any usages that previously passed an inviter, as well as
any code paths in ProfileSendInvitation or other call sites) to call
create_invitation(profile, invited_email, role=...) or with positional args
matching the new signature; ensure imports and tests are updated accordingly and
run tests to confirm no references to the removed inviter remain.