Add API endpoint for on-site order creation #1556
Closed
+188
−0
Conversation
This file contains hidden or 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
Contributor
Reviewer's GuideImplements a new staff-only on-site order creation API endpoint that reuses the existing order creation serializer, forces manual paid payments, sets box_office as the default sales channel, suppresses emails by default, and wires it into existing order and payment workflows with accompanying tests. Sequence diagram for staff on-site order creation APIsequenceDiagram
actor StaffUser
participant Client as APIClient
participant View as OrderViewSet
participant SerCreate as OrderCreateSerializer
participant DB as Database
participant SerOut as OrderSerializer
participant SigPlaced as order_placed_signal
participant SigPaid as order_paid_signal
StaffUser->>Client: Submit on-site order data
Client->>View: POST /api/v1/.../orders/create_onsite/
View->>View: Enforce staff and can_change_orders
View->>View: Copy request.data into data
View->>View: Set payment_provider manual
View->>View: Set status p (paid)
View->>View: Set sales_channel box_office (default)
View->>View: Set send_email False (default)
View->>SerCreate: Instantiate with data and context
SerCreate->>SerCreate: Validate data
SerCreate-->>View: Validated data
View->>DB: transaction.atomic begin
View->>View: perform_create(serializer)
View->>DB: Insert Order row
DB-->>View: Persisted Order
View->>View: Ensure order.pk exists
View->>View: order.log_action eventyay.event.order.placed
View->>SerOut: Instantiate with order and context
SerOut-->>View: Serialized order data
View->>SigPlaced: order_placed.send(event, order)
alt Order status is paid
View->>SigPaid: order_paid.send(event, order)
end
View-->>Client: 201 Created with order details
Client-->>StaffUser: Show created paid on-site order
Class diagram for updated OrderViewSet on-site creation flowclassDiagram
class OrderViewSet {
+create_onsite(request, **kwargs)
+perform_create(serializer)
+get_serializer_context()
}
class OrderCreateSerializer {
+is_valid(raise_exception)
+save()
+instance
}
class OrderSerializer {
+OrderSerializer(order, context)
+data
}
class Order {
+pk
+code
+status
+total
+locale
+STATUS_PAID
+log_action(action, user, auth, data)
}
class TaxRule {
+SaleNotAllowed
}
class Signals {
+order_placed
+order_paid
}
class Transaction {
+atomic()
}
class LanguageContext {
+language(locale, region)
}
OrderViewSet --> OrderCreateSerializer : uses for input validation
OrderViewSet --> OrderSerializer : uses for response data
OrderViewSet --> Order : creates and logs actions
OrderViewSet --> TaxRule : handles SaleNotAllowed
OrderViewSet --> Signals : sends order_placed and order_paid
OrderViewSet --> Transaction : wraps create_onsite in atomic
OrderViewSet --> LanguageContext : wraps signal sending in language
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}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The
create_onsiteaction currently relies on whatever permission/authorization is configured at the viewset level, but the description promises a staff-onlycan_change_ordersrestriction; consider enforcing this explicitly (e.g., with a decorator or permission check) so the behavior is guaranteed regardless of viewset reuse. - In
create_onsite, you trust client-suppliedsales_channelandsend_emailvalues after seeding defaults; if this endpoint is meant to consistently behave like box-office/import flows, you may want to either forcesales_channel='box_office'and/or constrain overrides, and coercesend_emailto a strict boolean to avoid unexpected truthy values from user input.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `create_onsite` action currently relies on whatever permission/authorization is configured at the viewset level, but the description promises a staff-only `can_change_orders` restriction; consider enforcing this explicitly (e.g., with a decorator or permission check) so the behavior is guaranteed regardless of viewset reuse.
- In `create_onsite`, you trust client-supplied `sales_channel` and `send_email` values after seeding defaults; if this endpoint is meant to consistently behave like box-office/import flows, you may want to either force `sales_channel='box_office'` and/or constrain overrides, and coerce `send_email` to a strict boolean to avoid unexpected truthy values from user input.
## Individual Comments
### Comment 1
<location> `src/tests/api/test_orders.py:4923-4924` </location>
<code_context>
assert answ.answer.startswith('file://')
+
+
[email protected]_db
+def test_order_create_onsite(token_client, organizer, event, item, quota, question):
+ res = {
+ 'email': '[email protected]',
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for permission behavior with authenticated users (with and without `can_change_orders`).
Currently the only negative permission test is `test_order_create_onsite_without_permission`, which checks an anonymous client gets a `401`. Please also add coverage for:
* An authenticated user without `can_change_orders` (and/or not staff) receiving the expected forbidden response (e.g. `403`).
* An authenticated staff user with `can_change_orders` successfully creating an order.
This will exercise the permission logic for authenticated roles, not just anonymous access.
Suggested implementation:
```python
@pytest.mark.django_db
def test_order_create_onsite_without_can_change_orders(api_client, organizer, event, item, quota, question, user):
"""
Authenticated user without ``can_change_orders`` should not be allowed
to create onsite orders.
"""
# Make sure the user is authenticated but does not have the required permission
user.is_staff = False
user.user_permissions.clear()
user.save()
api_client.force_authenticate(user=user)
payload = {
'email': '[email protected]',
'locale': 'en',
'positions': [
{
'item': item.pk,
'variation': None,
'price': '23.00',
'attendee_name_parts': {'full_name': 'John Doe'},
'attendee_email': '[email protected]',
'answers': [{'question': question.pk, 'answer': 'M', 'options': []}],
'subevent': None,
}
],
'sales_channel': 'web',
}
url = f'/api/v1/organizers/{organizer.slug}/events/{event.slug}/orders/onsite/'
resp = api_client.post(url, payload, format='json')
assert resp.status_code == 403
@pytest.mark.django_db
def test_order_create_onsite_with_can_change_orders(api_client, organizer, event, item, quota, question, user, django_content_type):
"""
Authenticated staff user with ``can_change_orders`` should be able to
create onsite orders successfully.
"""
# Grant the required permission
from django.contrib.auth.models import Permission
perm = Permission.objects.get(
content_type=django_content_type('orders', 'order'),
codename='can_change_orders',
)
user.is_staff = True
user.user_permissions.add(perm)
user.save()
api_client.force_authenticate(user=user)
payload = {
'email': '[email protected]',
'locale': 'en',
'positions': [
{
'item': item.pk,
'variation': None,
'price': '23.00',
'attendee_name_parts': {'full_name': 'John Doe'},
'attendee_email': '[email protected]',
'answers': [{'question': question.pk, 'answer': 'M', 'options': []}],
'subevent': None,
}
],
'sales_channel': 'web',
}
url = f'/api/v1/organizers/{organizer.slug}/events/{event.slug}/orders/onsite/'
resp = api_client.post(url, payload, format='json')
assert resp.status_code in (201, 200)
data = resp.json()
assert data.get('email') == '[email protected]'
assert data.get('code')
@pytest.mark.django_db
def test_order_create_onsite(token_client, organizer, event, item, quota, question):
```
Because only a fragment of the file is visible, you may need to adjust the following to integrate these tests correctly:
1. **Client fixture names**
- Replace `api_client` with the actual authenticated DRF client fixture used in this file (e.g. `client`, `auth_client`, or similar).
- If you already use `token_client` for authenticated calls, you can:
- Reuse it in these tests, or
- Introduce a dedicated fixture that creates an authenticated user without `can_change_orders`.
2. **Permission/ContentType helpers**
- The helper `django_content_type('orders', 'order')` is a placeholder. If your test suite already has a way to get the `ContentType` or the permission object for `can_change_orders`, reuse that instead.
- If you already import `Permission` elsewhere at the top of the file, remove the inline import in the test.
3. **URL and payload alignment**
- Ensure the URL path matches the existing onsite order creation endpoint used in `test_order_create_onsite`. If that test uses a helper (e.g. `url = api_client._get_onsite_url(event)`), reuse it instead of hardcoding.
- Keep the payload in these new tests consistent with the one in `test_order_create_onsite` (add/remove fields as needed so the request is valid).
4. **Expected status code**
- If your API returns a specific success code (e.g. always `201`) for onsite order creation, narrow `assert resp.status_code in (201, 200)` to the exact expected code.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
MukundC25
force-pushed
the
feature/onsite-order-api
branch
from
28f114b to
aaa6839
Compare
Author
|
Thanks for the review! I’ve addressed the feedback by:
|
Member
|
This is no longer part of the initial development plan. Hence closing it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Adds a staff-only API endpoint that allows creating ticket orders directly from the backend for on-site / box-office use cases.
Changes:
manualpayment provider, assuming payment is completed at the counterbox_officeFixes #1222
Summary by Sourcery
Add a staff-only API endpoint for creating paid on-site ticket orders via the backend.
New Features:
Enhancements:
Tests: