Add banner creation via admin panel #104
Conversation
Co-authored-by: me <me@rasulkireev.com>
|
Cursor Agent can help with this pull request. Just |
|
Warning Rate limit exceeded@rasulkireev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (15)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR implements a referrer banner system that displays promotional messages to users coming from specific referral sources (e.g., Product Hunt). The implementation includes:
- Backend: New
ReferrerBannerDjango model with fields for referrer code, display name, discount details, and expiry date. The model includes properties for checking expiry status and display eligibility. - Admin Interface: Comprehensive Django admin configuration with list views, filters, and organized fieldsets for managing banners.
- View Logic: Updated
LandingViewto check for?ref=URL parameters and pass matching active banners to the template context. - Frontend Controller: Stimulus.js controller that manages banner dismissal state using localStorage to prevent showing dismissed banners on subsequent visits.
- UI Component: Responsive banner template with conditional discount display, accessibility attributes, and smooth dismiss functionality.
The feature integrates cleanly with the existing codebase and follows the project's patterns (Django fat models, Stimulus.js for interactivity, server-side rendering). Minor improvements needed for production readiness.
Confidence Score: 4/5
- This PR is safe to merge after creating the database migration, with only minor style improvements recommended.
- The implementation is well-structured and follows Django and Stimulus.js best practices. However, the missing database migration is a critical blocker that must be addressed before deployment. The code has good separation of concerns and proper error handling in the backend (using try-except for DoesNotExist). Minor style improvements include moving imports to module level and adding localStorage error handling. The logic is sound and there are no security vulnerabilities.
- Pay close attention to
core/models.py- ensure migrations are generated and run before deploying to prevent database errors.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| core/models.py | 3/5 | Added ReferrerBanner model with properties for expiry checking and display logic, but missing database migration and potential validation constraints |
| core/views.py | 4/5 | Added context data method to fetch and validate referrer banner based on URL parameter, logic is sound but has minor performance consideration |
| frontend/src/controllers/referrer_banner_controller.js | 4/5 | Stimulus controller for banner dismissal using localStorage, follows project patterns but missing error handling for localStorage operations |
Sequence Diagram
sequenceDiagram
participant User
participant Browser
participant LandingView
participant Database
participant Template
participant StimulusController
participant LocalStorage
User->>Browser: Visit landing page with ?ref=producthunt
Browser->>LandingView: GET request with ref parameter
LandingView->>Database: Query ReferrerBanner.objects.get(referrer='producthunt')
alt Banner exists
Database-->>LandingView: Return banner object
LandingView->>LandingView: Check banner.should_display (is_active && !is_expired)
alt Should display
LandingView-->>Template: Pass banner in context
Template->>Browser: Render banner HTML with Stimulus controller
Browser->>StimulusController: Initialize referrer-banner controller
StimulusController->>LocalStorage: Check dismissed banners
alt Not dismissed
StimulusController-->>Browser: Display banner
Browser-->>User: Show referrer banner
User->>Browser: Click dismiss button
Browser->>StimulusController: Trigger dismiss action
StimulusController->>LocalStorage: Save referrer to dismissedReferrerBanners
StimulusController->>Browser: Remove banner element
else Already dismissed
StimulusController->>Browser: Remove banner element immediately
end
else Should not display
LandingView-->>Template: No banner in context
end
else Banner does not exist
Database-->>LandingView: ReferrerBanner.DoesNotExist
LandingView-->>Template: No banner in context
end
Template-->>User: Render landing page
6 files reviewed, 2 comments
core/views.py
Outdated
| template_name = "pages/landing.html" | ||
|
|
||
| def get_context_data(self, **kwargs): | ||
| from core.models import ReferrerBanner |
There was a problem hiding this comment.
style: Move import to top of file per PEP 8 style guidelines (imports should be at the module level).
Prompt To Fix With AI
This is a comment left during a code review.
Path: core/views.py
Line: 51:51
Comment:
**style:** Move import to top of file per PEP 8 style guidelines (imports should be at the module level).
How can I resolve this? If you propose a fix, please make it concise.| getDismissedBanners() { | ||
| const dismissed = localStorage.getItem("dismissedReferrerBanners"); | ||
| return dismissed ? JSON.parse(dismissed) : []; | ||
| } | ||
|
|
||
| saveDismissedBanner(referrer) { | ||
| const dismissed_banners = this.getDismissedBanners(); | ||
| if (!dismissed_banners.includes(referrer)) { | ||
| dismissed_banners.push(referrer); | ||
| localStorage.setItem("dismissedReferrerBanners", JSON.stringify(dismissed_banners)); | ||
| } |
There was a problem hiding this comment.
style: Wrap localStorage operations in try-catch blocks to handle QuotaExceededError or when localStorage is disabled (e.g., in private browsing mode).
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/controllers/referrer_banner_controller.js
Line: 20:30
Comment:
**style:** Wrap localStorage operations in try-catch blocks to handle `QuotaExceededError` or when localStorage is disabled (e.g., in private browsing mode).
How can I resolve this? If you propose a fix, please make it concise.
This pull request contains changes generated by a Cursor Cloud Agent