Implement organizer onboarding process | Check if Organizer has unpaid invoice #430
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sk' into feature-380
…implement-schedule-task
…implement-schedule-task
…lement-schedule-task
…lement-schedule-task
…lement-schedule-task
…lement-schedule-task
…lement-schedule-task
Reviewer's Guide by SourceryThis PR implements an organizer billing system with invoice generation and payment processing through Stripe. The key implementation includes monthly billing collection, invoice generation, payment processing, and a condition to prevent event publication if there are unpaid invoices. Sequence diagram for event publication check with unpaid invoicessequenceDiagram
actor Organizer
participant System
participant BillingInvoice
participant Event
Organizer->>System: Request to publish event
System->>BillingInvoice: Check for unpaid invoices
BillingInvoice-->>System: Return unpaid invoice status
alt Unpaid invoices exist
System-->>Organizer: Event cannot be published due to unpaid invoices
else No unpaid invoices
System->>Event: Publish event
Event-->>Organizer: Event published successfully
end
ER diagram for billing and organizer relationshiperDiagram
ORGANIZER {
int id
string name
}
BILLING_INVOICE {
int id
int organizer_id
int event_id
string status
decimal amount
string currency
decimal ticket_fee
date monthly_bill
}
ORGANIZER_BILLING_MODEL {
int id
int organizer_id
string primary_contact_name
string primary_contact_email
string company_or_organization_name
string address_line_1
string address_line_2
string city
string zip_code
string country
string preferred_language
string tax_id
string stripe_customer_id
string stripe_payment_method_id
}
ORGANIZER ||--o{ BILLING_INVOICE : has
ORGANIZER ||--o{ ORGANIZER_BILLING_MODEL : has
Class diagram for billing systemclassDiagram
class Organizer {
+has_unpaid_invoice() bool
}
class BillingInvoice {
+organizer: Organizer
+event: Event
+status: String
+amount: Decimal
+currency: String
+ticket_fee: Decimal
+payment_method: String
+paid_datetime: DateTime
+monthly_bill: Date
+reminder_schedule: List
+reminder_enabled: bool
}
class OrganizerBillingModel {
+organizer: Organizer
+primary_contact_name: String
+primary_contact_email: String
+company_or_organization_name: String
+address_line_1: String
+address_line_2: String
+city: String
+zip_code: String
+country: String
+preferred_language: String
+tax_id: String
+stripe_customer_id: String
+stripe_payment_method_id: String
}
Organizer --> BillingInvoice
Organizer --> OrganizerBillingModel
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @lcduong - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding comprehensive unit tests for the billing and payment processing logic to ensure reliability
- The monthly_billing_collect task could be broken down into smaller, more focused functions to improve maintainability
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -180,3 +198,417 @@ def get_header_token(user_id): | |||
| "Content-Type": "application/json", | |||
| } | |||
| return headers | |||
|
|
|||
|
|
|||
| @shared_task( | |||
There was a problem hiding this comment.
issue (complexity): Consider refactoring the monthly billing collection logic to reduce nesting and centralize error handling.
The monthly billing collection has some unnecessary complexity that can be simplified:
- Extract the duplicated retry logic into a decorator:
def with_retries(max_retries=5, delay=60):
def decorator(func):
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except Exception as e:
logger.error(f"Error in {func.__name__}: {e}")
if current_retry < max_retries:
time.sleep(delay)
return wrapper(*args, **kwargs, current_retry=current_retry+1)
logger.error(f"Max retries exceeded for {func.__name__}")
return wrapper
return decorator- Flatten the nested loops with early returns:
@shared_task
@with_retries()
def monthly_billing_collect(self):
today = datetime.today()
first_day = today.replace(day=1)
last_month = (first_day - relativedelta(months=1)).date()
gs = GlobalSettingsObject()
ticket_rate = gs.settings.get("ticket_fee_percentage", 2.5)
for event in Event.objects.select_related('organizer'):
if _should_skip_billing(event, last_month):
continue
total_amount = calculate_total_amount_on_monthly(event, last_month)
tickets_fee = calculate_ticket_fee(total_amount, ticket_rate)
_create_billing_invoice(
event, total_amount, tickets_fee,
last_month, today
)
def _should_skip_billing(event, last_month):
if BillingInvoice.objects.filter(
event=event, monthly_bill=last_month
).exists():
return True
if not event.orders.filter(
status=Order.STATUS_PAID,
datetime__range=[
last_month,
last_month + relativedelta(months=1, day=1) - relativedelta(days=1)
]
).exists():
return True
return FalseThis simplifies the code by:
- Centralizing retry logic in a reusable decorator
- Removing nested exception handling
- Flattening nested loops with helper functions
- Making the main flow more linear and easier to follow
The core billing logic remains intact but is now more maintainable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR partly resolves issue #379 Implement organizer onboarding process
This PR implement:
Summary by Sourcery
Implement a comprehensive billing system for organizers, including invoice management, payment processing with Stripe, and automated billing tasks. Prevent event publication if the organizer has unpaid invoices. Introduce a billing settings form for organizers to manage their billing information. Enhance the deployment with Celery beat schedules for billing tasks.
New Features:
Enhancements:
Build:
Deployment:
Tests: