I have studied DRF myself and one question bothers me, unfortunately I have no one to ask and this question bothers me all the time. Is writing application logic in views (as in the attached code) a good approach? When sending a query to the server with the intention, for example, to accept an order, I have to update the values from models other than the order or carry out validation of statuses, change the data on their basis. In the first tutorials from which I learned, among others, JustDjango all logic is in the view. However, after writing some code and watching other tutorials, I feel that this is not the right approach. Should not save and update models be done through a serializer? If so, whether through one large serializer or several smaller serializers.
class ZK_AddUpdateItemWithInstaReservation(APIView):
permission_classes = (IsAuthenticated,)
def post(self, request, *args, **kwargs):
pzitem_id = request.data.get('pzitem_id')
ordered_quantity = request.data.get('ordered_quantity')
order_id = request.data.get('order_id')
user_id = request.data.get('user_id')
item_id = request.data.get('item_id')
if ordered_quantity:
if int(ordered_quantity) < 0:
return Response({'error': 'Próbujesz zamówić ujemną wartość'})
try:
item = Item.objects.get(id=item_id)
except:
return Response({'error': 'Item with given id does not exists'})
try:
order = Order.objects.get(id=order_id)
except:
return Response({'error': 'Order with given id does not exist'})
if order.status not in ['SD', 'WTZ']:
return Response({'message': 'Nie można zmieniać ilości na zamówionym zamówieniu lub w realizacji'})
order.status = 'WTZ'
order.save()
# ORDERED ITEM WITHOUT PZITEM ON STOCK
if not pzitem_id:
# CHECK IF ORDER CONTAINS ORDERITEM
try:
orderitem = OrderItem.objects.get(pzitem=None, order=order, item=item)
prev_ordered_quantity = orderitem.quantity
# UPDATE ORDER ITEM VALUES
orderitem.quantity = ordered_quantity
orderitem.quantity_do_wydania = ordered_quantity
orderitem.status = 'ZBR'
orderitem.save()
# UPDATE ITEM VALUES
item = Item.objects.get(id=orderitem.item_id)
prev_item_not_ordered_plus_qty_prevordered = item.quantity_not_ordered prev_ordered_quantity
item.quantity_not_ordered = int(prev_item_not_ordered_plus_qty_prevordered) - int(ordered_quantity)
item.save()
return Response({'message': 'Zaktualizowano ilość przedmiotu bez rezerwacji stanu'})
except:
# IF NOT CREATE ONE
orderitem = OrderItem.objects.create(user_id=user_id, pzitem=None, item=item, quantity=ordered_quantity, quantity_do_wydania=ordered_quantity, status='ZBR', order=order)
orderitem.save()
# UPDATE ITEM VALUES
item = Item.objects.get(id=orderitem.item_id)
item.quantity_not_ordered -= int(ordered_quantity)
item.save()
return Response({'messsage': 'Dodano nowy przedmiot do zamowienia, status bez rezerwacji stanu.'})
# ORDERED WITH STOCK
else:
try:
pzitem = PZItem.objects.get(id=pzitem_id)
except:
return Response({'error': 'Nie znaleziono pzitem z podanym pzitem_id'})
# CHECK IF ORDER CONTAINS ORDERITEM
try:
orderitem = OrderItem.objects.get(pzitem=pzitem, order=order, item=item)
prev_ordered_quantity = orderitem.quantity
if int(ordered_quantity) <= prev_ordered_quantity orderitem.pzitem.quantity_available_to_order:
prev_ordered_quantity = orderitem.quantity
# UPDATE ORDER ITEM VALUES
orderitem.status = 'ZZR'
orderitem.quantity = ordered_quantity
orderitem.quantity_do_wydania = ordered_quantity
orderitem.save()
# UPDATE ITEM VALUES
item = Item.objects.get(id=orderitem.item_id)
prev_item_not_ordered_plus_qty_prevordered = item.quantity_not_ordered prev_ordered_quantity
item.quantity_not_ordered = int(prev_item_not_ordered_plus_qty_prevordered) - int(ordered_quantity)
item.save()
# UPDATE PZITEM VALUES
prev_pzitem_quantity_ordered = int(orderitem.pzitem.quantity_ordered)
orderitem.pzitem.quantity_ordered = int(orderitem.pzitem.quantity_ordered) - int(prev_ordered_quantity) int(ordered_quantity)
orderitem.pzitem.save()
return Response({'message': 'Zaktualizowano ilość przedmiotu z rezerwacją stanu'})
else:
return Response({'warning': 'Zamówiono więcej niż dostępne'})
except:
# IF NOT CREATE ONE
if int(ordered_quantity) or 0 <= pzitem.quantity_available_to_order:
orderitem = OrderItem.objects.create(user_id=user_id, pzitem=pzitem, item=item, quantity=ordered_quantity, quantity_do_wydania=ordered_quantity, status='ZZR', order=order)
orderitem.save()
# UPDATE ITEM VALUES
item = Item.objects.get(id=orderitem.item_id)
item.quantity_not_ordered -= int(ordered_quantity)
item.save()
# UPDATE PZITEM VALUES
orderitem.pzitem.quantity_ordered = int(ordered_quantity)
orderitem.pzitem.save()
return Response({'messsage': 'Dodano nowy przedmiot do zamowienia, status zamówiono z rezerwacją stanu.'})
else:
return Response({'warning': 'Zamówiono więcej niż dostępne'})
CodePudding user response:
You have so much logic in views.py its not a best way. Separate your model actions to other views and serializer. For example for your order operations you can implement view and serializers like this.
class UpdateOrderSerializer(serializers.ModelSerializer):
pzitem_id = serializers.CharField()
ordered_quantity = serializers.IntegerField()
user = serializers.PrimaryKeyRelatedField(User.objects.all())
item = serializers.PrimaryKeyRelatedField(OrderItem.objects.all())
class Meta:
model = Order
def validate(self, attrs):
attrs = super().validate(attrs)
if attrs["ordered_quantity"] < 0:
raise serializers.ValidationError(dict(ordered_quantity="Próbujesz zamówić ujemną wartość"))
def update(self, instance, validated_data):
instance.status = "WTZ"
# do some other things
instance.save()
return instance
class ZK_AddUpdateItemWithInstaReservation(ModelViewSet):
permission_classes = (IsAuthenticated,)
serializer_class = OrderSerializer
queryset = Order.objects.all()
@action(detail=True)
def update_order(self, request, pk):
serializer = self.get_serializer_class(data=request.data)
serializer.is_valid(raise_exception=True)
serializer.save()
return Response(status=HTTP_200_OK, data={})
If you have so much operations which is related between them. You can create a pipelines. Like this;
class YourPipeline:
def __init__(self, past_some_data):
self.past_some_data = past_some_data
def update_order_status(self):
# update your order
def create_order_item(self):
# do something else
def some_function(self):
# another operation
def run(self):
self.update_order_status()
self.create_order_item()
self.some_function()
return instance
class UpdateOrderSerializer(serializers.ModelSerializer):
pzitem_id = serializers.CharField()
ordered_quantity = serializers.IntegerField()
user = serializers.PrimaryKeyRelatedField(User.objects.all())
item = serializers.PrimaryKeyRelatedField(OrderItem.objects.all())
class Meta:
model = Order
def validate(self, attrs):
attrs = super().validate(attrs)
if attrs["ordered_quantity"] < 0:
raise serializers.ValidationError(dict(ordered_quantity="Próbujesz zamówić ujemną wartość"))
def update(self, instance, validated_data):
pipeline = YourPipeline(past_some_data=past_some_data)
return pipeline.run()
class ZK_AddUpdateItemWithInstaReservation(ModelViewSet):
permission_classes = (IsAuthenticated,)
serializer_class = OrderSerializer
queryset = Order.objects.all()
@action(detail=True)
def update_order(self, request, pk):
serializer = self.get_serializer_class(data=request.data)
serializer.is_valid(raise_exception=True)
serializer.save()
return Response(status=HTTP_200_OK)
CodePudding user response:
In my opinion, there are a few things wrong with the code in your case:
As you wrote, saving the data should be done through the serializer. In my opinion, it should be one serializer that will write data in one transaction.
In case of errors you should not return status code 200, just one of the 4XX errors
If you want to reduce or enlarge the model field:
item.quantity_not_ordered -= int(ordered_quantity)
u should use https://docs.djangoproject.com/en/4.0/ref/models/expressions/#avoiding-race-conditions-using-f
- You shouldn't use
try:
foo()
except:
pass
It is good practice to explicitly catch any exceptions that may occur