svthalia / concrexit

Thalia Website built on Django.
https://thalia.nu
Other
22 stars 12 forks source link

Rebuild `pizzas` app on top of `sales` app #1654

Open JobDoesburg opened 3 years ago

JobDoesburg commented 3 years ago

Is your feature request related to a problem? Please describe.

Describe the solution you'd like

Rebuild pizzas on top of sales:

Motivation

In the end: simpler and better code

This issue should be a place to discuss possible design decisions

DeD1rk commented 3 years ago

I think it would be nice to do this. Something to think about is how to determine whether someone is allowed to order only 1 (or maybe up to some other number) item (in a shift, from a product list, or of a specific product).

JobDoesburg commented 3 years ago

https://github.com/KiOui/TOSTI/blob/2a3519cd1bb0865b3c33f78a9d3e76fcd97605b5/website/orders/models.py#L268-L274

finally something in the sales app that we could actually use the TOSTI code for

But I agree that we should carefully think this through because I want to keep the code as clean as possible

se-bastiaan commented 3 years ago

A field doesn't really count as using the code 😆

I think the motivation of this issue is wrong. It wouldn't really end up as simpler or better code. The pizzas app is not written perfectly, but it is quite nice because it only does 1 single thing. And it does it well.

The real motivation should be rethinking how often we order food and will do this in the future. The functionality is currently lacking, like food at GMs for example. Or having multiple events at the same time with takeout (should probably be the name new) offerings. More focus on removing code duplication and making this part ready for the future.

I suggest removing the solution you'd like, or at least make it clear that this is 1 possible solution.