Implement voucher per event and for all events of an organizer | Create Voucher #473
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
Reviewer's Guide by SourceryThis PR implements a new voucher management system in the admin area that allows administrators to create, update, and delete vouchers. The vouchers can be used by organizers to reduce billing invoice prices, with options to limit their usage to specific events or organizers. The implementation includes new model classes, views, forms, and templates to support the voucher management functionality. Class diagram for the new InvoiceVoucher modelclassDiagram
class InvoiceVoucher {
+String code
+PositiveInteger max_usages
+PositiveInteger redeemed
+Decimal budget
+DateTime valid_until
+String price_mode
+Decimal value
+ManyToManyField limit_events
+ManyToManyField limit_organizer
+DateTime created_at
+String created_by
+DateTime updated_at
+String updated_by
+is_active() bool
}
class Event
class Organizer
InvoiceVoucher --> Event : limit_events
InvoiceVoucher --> Organizer : limit_organizer
note for InvoiceVoucher "Represents a voucher that can be used to reduce billing invoice prices."
File-Level Changes
Assessment against linked issues
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 @odkhang - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider removing redundant method overrides that only call super() (e.g. get(), post(), get_form_kwargs()) to reduce code duplication and improve maintainability.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 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.
| ctx = super().get_context_data(**kwargs) | ||
| return ctx | ||
|
|
||
| def get(self, request, *args, **kwargs): |
There was a problem hiding this comment.
suggestion: Remove unnecessary method overrides that just call super()
Several methods in these views (get(), get_context_data(), get_form_kwargs(), form_valid() in VoucherCreate) just call super() without adding functionality. These can be removed to reduce code noise.
Suggested implementation:
If there are other methods in the file that just call super() without adding functionality (like get(), get_form_kwargs(), form_valid()), they should also be removed in the same way. However, I can't see them in the provided code snippet to make those specific changes.
| form_class = InvoiceVoucherForm | ||
| return form_class | ||
|
|
||
| def get_object(self, queryset=None) -> InvoiceVoucherForm: |
There was a problem hiding this comment.
issue: Fix incorrect return type annotation
The return type should be InvoiceVoucher, not InvoiceVoucherForm, as this method returns an InvoiceVoucher instance.
| @@ -127,3 +132,113 @@ | |||
| ) | |||
|
|
|||
| return HttpResponseRedirect(reverse('control:admin.task_management')) | |||
|
|
|||
|
|
|||
| class VoucherList(PaginationMixin, AdministratorPermissionRequiredMixin, ListView): | |||
There was a problem hiding this comment.
issue (complexity): Consider creating a base class for voucher views to reduce code duplication and simplify the create/update views.
The code can be simplified while maintaining all functionality:
- Create a base class for shared voucher view logic:
class BaseVoucherView(AdministratorPermissionRequiredMixin):
model = InvoiceVoucher
template_name = 'pretixcontrol/admin/vouchers/detail.html'
context_object_name = 'voucher'
form_class = InvoiceVoucherForm
def get_context_data(self, **kwargs):
ctx = super().get_context_data(**kwargs)
ctx['currency'] = settings.DEFAULT_CURRENCY
return ctx
def get_success_url(self) -> str:
return reverse('control:admin.vouchers')- Simplify the create/update views to only include unique behavior:
class VoucherCreate(BaseVoucherView, CreateView):
pass
class VoucherUpdate(BaseVoucherView, UpdateView):
def get_object(self, queryset=None) -> InvoiceVoucher:
try:
return InvoiceVoucher.objects.get(id=self.kwargs['voucher'])
except InvoiceVoucher.DoesNotExist:
raise Http404(_("The requested voucher does not exist."))
def form_valid(self, form):
messages.success(self.request, _('Your changes have been saved.'))
return super().form_valid(form)This removes empty method overrides and duplicated code while preserving all functionality. The base class makes the relationship between these views explicit and easier to maintain.
hongquan
approved these changes
Dec 18, 2024
| self.fields['event_effect'].queryset = Event.objects.all() | ||
| self.fields['organizer_effect'].queryset = Organizer.objects.all() | ||
|
|
||
| def clean(self): |
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 - #382 Implement voucher per event and for all events of an organizer
This PR implement:
Summary by Sourcery
Implement a voucher management system in the admin area, enabling the creation, update, and deletion of vouchers that can be used by organizers to adjust billing invoices. This includes a new admin page for managing vouchers and the ability to limit voucher usage to specific events or organizers.
New Features:
Enhancements:
Summary by Sourcery
Implement a new voucher management system in the admin area, allowing administrators to create, update, and delete vouchers for organizers to use in reducing billing invoice prices. This includes a new 'Voucher' page and associated views and templates for managing voucher details and effects.
New Features:
Enhancements: