Moved sulten #925
Moved sulten #925
Conversation
tiniuspre
requested review from
emilte,
evgiz,
Mathias-a and
robines
as code owners
backend/sulten/admin.py
Outdated
|
|
||
|
|
||
| @admin.register(Booking) | ||
| class BookingAdmin(CustomBaseAdmin): |
There was a problem hiding this comment.
Det stemmer, vi ønsker støtte for at alle lokaler på samf skal ha mulighet til å ha meny og bord som kan bookes. Spesielt med nybygg kan det hende det blir nødvendig.
There was a problem hiding this comment.
Enig selv, om man skal kunne booke flere rom, men med tanke på å dele opp ting enda mer for jeg ser backend trenger det. Skal vi lage en egen django app for venues eller ha venues i samfundet som jeg ser for meg burde kun ha "generelle" ting.
Hvis venues blir stort og ender opp med mye kode burde vi ha en egen app.
Hva tenker dere?
There was a problem hiding this comment.
Mye kode er ikke et stort problem, det øker ikke kompleksiteten. I Samf4 henger mye sammen, så en egen app hjelper ikke nødvendigvis så veldig mye. Foreløpig tror jeg det er best om man bare splitter ut når en fil blir for stor. Da har man det bare ved siden med ny fil. Her kunne man f.eks. bare hatt samfundet.models.sulten. Vi prøvde strukturen med flere apper i rekenett, og det var ikke noe særlig å vedlikeholde.
There was a problem hiding this comment.
Sulten er på en måte en egen nettside, så jeg er ikke i mot at det er en egen app. Ser den låner Venue fra samfundet da. Delte modeller på tvers av apper kan føre til sirkulære avhengigheter som er vanskelig å forholde seg til.
There was a problem hiding this comment.
Diskusjonen er ganske nyansert og går i begge retninger. Vet ikke helt om jeg vet hva du er enig med :P
There was a problem hiding this comment.
Det å dele opp kode, enig at apper kan bli litt meget om det er mye fram/tilbake. Selv erfart det at ting blir messy fort om flere jobber og ikke alle følger en "struktur" som er løst satt som må til om man skal ha et slikt oppsett. Så jeg tenker lyche som er ganske standalone uten om booking burde være i en app 😁
There was a problem hiding this comment.
Hva med en sulten modul innenfor samfundet?
There was a problem hiding this comment.
Mener sulten.views & models osv er best. altså sulten.views.menu . Blir litt feil med tanke på at sulten er uavhengig. samfundet.views.sulten.menu når det også blir samfundet.views.blog.blogpost i samme hovedkategori. Hadde vært finest med samfundet.sulten.views.menu som Doordash django, men da krever det litt mer restrukturering . Så min konklusjon da at det burde være: Lyche som egen app. Dele opp models og views i flere under mapper så vi får det mer logisk modul basert hvor man kan gjette seg til hva som kommer til å være i under modulene basert på parenten :)
There was a problem hiding this comment.
Det kan hende vi misforstår hverandre, men den blogposten du sendte støtter ikke saken din i det hele tatt. Den argumenterer det motsatte og styrker faktisk anbefalingen min om å ikke lage flere apper. Vi har ikke en gigantisk monolitt som skal skalere for hundre tusenvis av brukere.
Det er ikke vanskelig å refaktorere slik at sulten blir en modul med egen identitet slik du ønsker. Resten av koden trenger ikke sin egen identitet før det er et tydelig skille. Du skriver at sulten er uavhengig, det stemmer ikke. Den deler modeller med resten av samfundet.
backend/root/urls.py
Outdated
| @@ -12,6 +12,7 @@ | |||
| path('rest_framework/', include('rest_framework.urls')), | |||
| path('notifications/', include(notifications.urls, namespace='notifications')), | |||
| path('', include('samfundet.urls')), # Put last. | |||
| path('', include('sulten.urls')), # Put last. | |||
backend/sulten/models.py
Outdated
| from django.db import models | ||
|
|
||
| from root.utils.mixins import CustomBaseModel | ||
| from samfundet.models.general import Venue | ||
| from samfundet.models.model_choices import ReservationOccasion | ||
| from django.utils import timezone | ||
| from datetime import datetime, date, time, timedelta | ||
| from collections import defaultdict | ||
| from django.core.exceptions import ValidationError | ||
| from typing import Any | ||
|
|
||
|
|
backend/sulten/views.py
Outdated
| from rest_framework.permissions import DjangoModelPermissionsOrAnonReadOnly, AllowAny | ||
| from rest_framework.views import APIView | ||
| from rest_framework.viewsets import ModelViewSet | ||
|
|
||
| from samfundet.models.general import Venue | ||
| from sulten.models import Menu, MenuItem, FoodCategory, FoodPreference, Table, Reservation, Booking | ||
| from sulten.serializers import MenuItemSerializer, MenuSerializer, FoodCategorySerializer, FoodPreferenceSerializer, TableSerializer, \ | ||
| ReservationCheckSerializer, \ | ||
| BookingSerializer | ||
| from rest_framework.request import Request | ||
| from rest_framework.response import Response | ||
| from rest_framework import status | ||
|
|
||
| from django.utils import timezone |
backend/sulten/admin.py
Outdated
|
|
||
|
|
||
| @admin.register(Booking) | ||
| class BookingAdmin(CustomBaseAdmin): |
There was a problem hiding this comment.
Mye kode er ikke et stort problem, det øker ikke kompleksiteten. I Samf4 henger mye sammen, så en egen app hjelper ikke nødvendigvis så veldig mye. Foreløpig tror jeg det er best om man bare splitter ut når en fil blir for stor. Da har man det bare ved siden med ny fil. Her kunne man f.eks. bare hatt samfundet.models.sulten. Vi prøvde strukturen med flere apper i rekenett, og det var ikke noe særlig å vedlikeholde.
backend/samfundet/models/venue.py
Outdated
| from samfundet.models.model_choices import ReservationOccasion | ||
| from datetime import datetime, date, time, timedelta | ||
| from django.core.exceptions import ValidationError | ||
| from root.utils.mixins import CustomBaseModel | ||
| from collections import defaultdict | ||
| from django.utils import timezone | ||
| from django.db import models | ||
| from typing import Any | ||
|
|
Created app for sulten and moved views, models, admin and urls