-
Notifications
You must be signed in to change notification settings - Fork 1
Referral Program Reward: 5 Referrals = 6 Months Free #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: saas
Are you sure you want to change the base?
Referral Program Reward: 5 Referrals = 6 Months Free #59
Conversation
- Updated `ReferralService.apply_referral` to grant 6 months of `plus_tier` when a referrer reaches 5 successful referrals. - Implemented `grant_referral_reward` logic to handle both new subscriptions and extensions of existing ones. - Added integration tests in `tests/integration/test_referral_upgrade.py` covering both grant and extension scenarios. - Documented the feature in `docs/REFERRALS.md`.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
- Updated `ReferralService.apply_referral` to grant 6 months of `plus_tier` when a referrer reaches 5 successful referrals. - Implemented `grant_referral_reward` logic to handle both new subscriptions and extensions of existing ones. - Added integration tests in `tests/integration/test_referral_upgrade.py` covering both grant and extension scenarios. - Documented the feature in `docs/REFERRALS.md`. - Passed all CI checks (ruff, vulture, ty).
- Added `ReferralConfig` to `common/config_models.py` and `SubscriptionConfig`. - Updated `common/global_config.yaml` to include default values for `referrals_required` (5) and `reward_months` (6). - Refactored `src/api/services/referral_service.py` to use values from `global_config` instead of hardcoded constants. - Updated `tests/integration/test_referral_upgrade.py` to dynamically use the configured values for assertions. - Updated `docs/REFERRALS.md` to document the configuration options. - Verified with `make ci` and integration tests.
Greptile SummaryImplements referral reward system granting 6 months of Plus tier after 5 successful referrals. Configuration is externalized to Critical Issue Found:
Additional Issues:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as New User
participant API as /referrals/apply
participant RS as ReferralService
participant DB as Database
participant Referrer as Referrer Profile
User->>API: POST /referrals/apply {referral_code}
API->>RS: validate_referral_code(code)
RS->>DB: Query Profiles by referral_code
DB-->>RS: Return referrer profile
RS->>API: Return referrer
API->>RS: apply_referral(user_profile, code)
Note over RS,DB: Begin db_transaction
RS->>DB: Set user_profile.referrer_id
RS->>DB: Atomic increment referrer.referral_count
RS->>DB: Refresh referrer to get new count
alt Count == required_referrals (e.g., 5)
RS->>RS: grant_referral_reward(referrer_id)
RS->>DB: Query UserSubscriptions
alt Has existing subscription
RS->>DB: Update tier to PLUS & extend end_date
else No subscription
RS->>DB: Create new PLUS subscription
end
end
Note over RS,DB: Commit transaction
DB-->>RS: Success
RS-->>API: Return true
API-->>User: {"message": "success"}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8 files reviewed, 4 comments
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
- Added `ReferralConfig` to `common/config_models.py` and `SubscriptionConfig`. - Updated `common/global_config.yaml` to include default values for `referrals_required` (5) and `reward_months` (6). - Refactored `src/api/services/referral_service.py` to use values from `global_config` instead of hardcoded constants. - Updated `tests/integration/test_referral_upgrade.py` to dynamically use the configured values for assertions. - Updated `docs/REFERRALS.md` to document the configuration options. - Addressed PR feedback by clarifying the type cast comment in `referral_service.py`. - Verified with `make ci` and integration tests.
- Added `ReferralConfig` to `common/config_models.py` and `SubscriptionConfig`. - Updated `common/global_config.yaml` to include default values for `referrals_required` (5) and `reward_months` (6). - Refactored `src/api/services/referral_service.py` to use values from `global_config` instead of hardcoded constants. - Updated `tests/integration/test_referral_upgrade.py` to dynamically use the configured values for assertions and use nested transactions for DB rollback. - Updated `docs/REFERRALS.md` to document the configuration options. - Addressed PR feedback by clarifying the type cast comment in `referral_service.py`. - Verified with `make ci` and integration tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 files reviewed, 2 comments
| def grant_referral_reward(db: Session, user_id: uuid.UUID): | ||
| """ | ||
| Grant Plus Tier to the user based on configured reward duration. | ||
| """ | ||
| now = datetime.now(timezone.utc) | ||
| reward_months = global_config.subscription.referral.reward_months | ||
| reward_duration = timedelta(days=30 * reward_months) | ||
|
|
||
| subscription = ( | ||
| db.query(UserSubscriptions) | ||
| .filter(UserSubscriptions.user_id == user_id) | ||
| .first() | ||
| ) | ||
|
|
||
| if subscription: | ||
| subscription.subscription_tier = SubscriptionTier.PLUS.value | ||
| subscription.is_active = True | ||
|
|
||
| # If current subscription is valid and ends in the future, extend it | ||
| # Otherwise start from now | ||
| current_end = subscription.subscription_end_date | ||
| if current_end and current_end.tzinfo is None: | ||
| current_end = current_end.replace(tzinfo=timezone.utc) | ||
|
|
||
| if current_end and current_end > now: | ||
| subscription.subscription_end_date = current_end + reward_duration | ||
| else: | ||
| subscription.subscription_end_date = now + reward_duration | ||
|
|
||
| logger.info( | ||
| f"Updated subscription for user {user_id} via referral reward ({reward_months} months)" | ||
| ) | ||
| else: | ||
| new_subscription = UserSubscriptions( | ||
| user_id=user_id, | ||
| subscription_tier=SubscriptionTier.PLUS.value, | ||
| is_active=True, | ||
| subscription_start_date=now, | ||
| subscription_end_date=now + reward_duration, | ||
| ) | ||
| db.add(new_subscription) | ||
| logger.info( | ||
| f"Created subscription for user {user_id} via referral reward ({reward_months} months)" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Function modifies database but doesn't handle its own transaction. Called from within db_transaction context in apply_referral, but changes won't be committed if called elsewhere. Either make this function handle its own transaction, or document that it must be called within an active transaction context.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/api/services/referral_service.py
Line: 29:72
Comment:
**logic:** Function modifies database but doesn't handle its own transaction. Called from within `db_transaction` context in `apply_referral`, but changes won't be committed if called elsewhere. Either make this function handle its own transaction, or document that it must be called within an active transaction context.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by this? Can you explain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that grant_referral_reward modifies the database (lines 44-45, 54/56, and 69) but doesn't handle database transactions itself. Let me explain the problem:
Current situation:
grant_referral_rewardmakes database changes but has nodb.commit()or transaction management- It's called from
apply_referral(line 109) which wraps its work indb_transaction - If someone calls
grant_referral_rewarddirectly from elsewhere, the changes won't be saved to the database
The problem:
# This would NOT save changes to the database:
ReferralService.grant_referral_reward(db, user_id)
# Changes are lost because no commit happenedTwo solutions:
Option 1: Make it handle its own transaction
| def grant_referral_reward(db: Session, user_id: uuid.UUID): | |
| """ | |
| Grant Plus Tier to the user based on configured reward duration. | |
| """ | |
| now = datetime.now(timezone.utc) | |
| reward_months = global_config.subscription.referral.reward_months | |
| reward_duration = timedelta(days=30 * reward_months) | |
| subscription = ( | |
| db.query(UserSubscriptions) | |
| .filter(UserSubscriptions.user_id == user_id) | |
| .first() | |
| ) | |
| if subscription: | |
| subscription.subscription_tier = SubscriptionTier.PLUS.value | |
| subscription.is_active = True | |
| # If current subscription is valid and ends in the future, extend it | |
| # Otherwise start from now | |
| current_end = subscription.subscription_end_date | |
| if current_end and current_end.tzinfo is None: | |
| current_end = current_end.replace(tzinfo=timezone.utc) | |
| if current_end and current_end > now: | |
| subscription.subscription_end_date = current_end + reward_duration | |
| else: | |
| subscription.subscription_end_date = now + reward_duration | |
| logger.info( | |
| f"Updated subscription for user {user_id} via referral reward ({reward_months} months)" | |
| ) | |
| else: | |
| new_subscription = UserSubscriptions( | |
| user_id=user_id, | |
| subscription_tier=SubscriptionTier.PLUS.value, | |
| is_active=True, | |
| subscription_start_date=now, | |
| subscription_end_date=now + reward_duration, | |
| ) | |
| db.add(new_subscription) | |
| logger.info( | |
| f"Created subscription for user {user_id} via referral reward ({reward_months} months)" | |
| ) | |
| @staticmethod | |
| def grant_referral_reward(db: Session, user_id: uuid.UUID): | |
| """ | |
| Grant Plus Tier to the user based on configured reward duration. | |
| """ | |
| with db_transaction(db): | |
| now = datetime.now(timezone.utc) | |
| reward_months = global_config.subscription.referral.reward_months | |
| reward_duration = timedelta(days=30 * reward_months) | |
| subscription = ( | |
| db.query(UserSubscriptions) | |
| .filter(UserSubscriptions.user_id == user_id) | |
| .first() | |
| ) | |
| if subscription: | |
| subscription.subscription_tier = SubscriptionTier.PLUS.value | |
| subscription.is_active = True | |
| # If current subscription is valid and ends in the future, extend it | |
| # Otherwise start from now | |
| current_end = subscription.subscription_end_date | |
| if current_end and current_end.tzinfo is None: | |
| current_end = current_end.replace(tzinfo=timezone.utc) | |
| if current_end and current_end > now: | |
| subscription.subscription_end_date = current_end + reward_duration | |
| else: | |
| subscription.subscription_end_date = now + reward_duration | |
| logger.info( | |
| f"Updated subscription for user {user_id} via referral reward ({reward_months} months)" | |
| ) | |
| else: | |
| new_subscription = UserSubscriptions( | |
| user_id=user_id, | |
| subscription_tier=SubscriptionTier.PLUS.value, | |
| is_active=True, | |
| subscription_start_date=now, | |
| subscription_end_date=now + reward_duration, | |
| ) | |
| db.add(new_subscription) | |
| logger.info( | |
| f"Created subscription for user {user_id} via referral reward ({reward_months} months)" | |
| ) |
Option 2: Document the requirement
Add to the docstring: "Must be called within an active database transaction context."
I'd recommend Option 1 since it makes the function safer to use independently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic determines the start and end dates of the free period:
- Extension: If the user already has an active subscription ending in the future (e.g., ends on Dec 31st), we add the reward duration to that existing date (New End Date = Dec 31st + 6 months). This ensures they don't lose the time they already paid for.
- Immediate Start: If they don't have an active subscription (or it expired in the past), the 6-month free period starts immediately from today (
now).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok @jules please do as greptile says and go with option 1 and handle the DB transaction. (unless you disagree. if you disagree pls say)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I apologize, but I cannot see the comment from @greptile to reference 'Option 1'. However, regarding transaction handling: The grant_referral_reward method is currently designed to run within the active transaction managed by apply_referral. This ensures atomicity: if the reward fails, the referral count update rolls back too. If 'Option 1' suggests wrapping grant_referral_reward in its own db_transaction, please note that this would commit the transaction immediately, potentially affecting the atomicity of the calling function. I believe the current approach is the safest for data integrity. Please let me know if you'd like me to proceed differently!
- Added `ReferralConfig` to `common/config_models.py` and `SubscriptionConfig`. - Updated `common/global_config.yaml` to include default values for `referrals_required` (5) and `reward_months` (6). - Refactored `src/api/services/referral_service.py` to use values from `global_config` instead of hardcoded constants. - Updated `tests/integration/test_referral_upgrade.py` to dynamically use the configured values for assertions and use nested transactions for DB rollback. - Updated `docs/REFERRALS.md` to document the configuration options. - Addressed PR feedback by clarifying the type cast comment in `referral_service.py` and explaining transaction logic. - Verified with `make ci` and integration tests.
Implemented the referral reward system where users receive 6 months of the paid tier for free upon referring 5 users. This includes backend logic updates, new integration tests, and documentation.
PR created automatically by Jules for task 4126785755739167118 started by @Miyamura80