search

VOKO-Utrecht / voko

Administration and automation for VOKO
4 stars 8 forks source link

Improve file upload produt lists #329

Closed mgvd closed 6 months ago

mgvd commented 8 months ago

Wij hebben de laatste tijd wat problemen met de bestellijsten. We uploaden altijd een excelbestand in het systeem, maar die excelbestanden moeten wel heel precies ingevuld worden anders kleurt het rood op de lijst. Nu hebben we een aantal keer gehad dat er bij de bestanden van Nieuw Slagmaat een spatie stond in plaats van niks in een vakje en dan kleurt hij rood, of bij de bestanden van Zuylestein staat er een formule in om de prijs te berekenen in plaats van de prijs, dan gaat het ook mis. Als je vervolgens de hele lijst er weer uit haalt en opnieuw probeert up te loaden omdat je fouten hebt gemaakt met wel/ niet verwijderen, neemt hij toch de oude lijst weer over. Denk jij dat hier nog wat in te verbeteren valt?

Pagina: /ordering/admin/supplier/1/

bveldkamp commented 8 months ago

De formules kunnen kennelijk worden omzeild met data_only (openpyxl.load_workbook(filename, data_only=True). Voor de spaties zouden we denk ik wel wat intelligentere parsing kunnen bouwen.

Dat ie het oude bestand neemt is wel vreemd, ik vraag me af of er iets fout gaat bij die NamedTemporaryFile als een bestand met dezelfde suffix al bestaat. Sws wordt het tijdelijke bestand nu nooit verwijderd lijkt het, dus misschien is dat de eerste stap.

Heb je een voorbeeld van een foutieve Excel?

Relevante code in webapp\ordering\admin_views.py : create_draft_products_from_spreadsheet()

mgvd commented 8 months ago

Ik heb om de bestanden gevraagd. Zelf dacht ik dat de handmatige correcties worden overgeschreven als het bestand opnieuw wordt geüpload. Niet dat een of bestand wordt gebruikt. Ik vraag het na

Op ma 6 nov 2023 12:05 schreef Berend Veldkamp @.***>:

De formules kunnen kennelijk worden omzeild met data_only (openpyxl.load_workbook(filename, data_only=True). Voor de spaties zouden we denk ik wel wat intelligentere parsing kunnen bouwen.

Dat ie het oude bestand neemt is wel vreemd, ik vraag me af of er iets fout gaat bij die NamedTemporaryFile als een bestand met dezelfde suffix al bestaat. Sws wordt het tijdelijke bestand nu nooit verwijderd lijkt het, dus misschien is dat de eerste stap.

Heb je een voorbeeld van een foutieve Excel?

Relevante code in webapp\ordering\admin_views.py : create_draft_products_from_spreadsheet()

— Reply to this email directly, view it on GitHub https://github.com/VOKO-Utrecht/voko/issues/329#issuecomment-1794573544, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARKEMVNRV2USLRUXHIFAVDYDDABLAVCNFSM6AAAAAA67HXU6KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJUGU3TGNJUGQ . You are receiving this because you authored the thread.Message ID: @.***>

bveldkamp commented 8 months ago

Ik heb een paar verbeteringen aangebracht, zie branch:

Wat er denk ik fout gaat bij heruploaden, is dat men producten uit de lijst verwijderd, en meteen daarna opnieuw een gewijzigde Excel uploadt. Maar in dat geval worden de eerder geüploade drafts niet verwijderd, dat gebeurt pas als je op Valideren klikt. Na de 2e upload zie je dan bovenaan de oude drafts, incl. degene die je had geprobeerd te verwijderen, en onderaan nog een keer de aangepaste.

Wat de beste oplossing is weet ik niet, misschien moeten bij elke upload automatisch de drafts van de huidige ronde worden verwijderd? Je krijgt anders in elk geval duplicaten zolang je niet op Valideren hebt gedrukt. Maar je kunt dan niet meerdere Excel sheets gebruiken (geen idee of dat voorkomt). Alternatief is een extra vinkje bij de upload: 'Overschrijf bestaande' oid.

Nog een paar andere verbeterpunten die we misschien meteen mee kunnen nemen:

  1. Een aantal van 0 wordt nu als ongeldige invoer beschouwd, maar ik vraag me af of dat wel zou moeten. Een boer zou 1 Excel kunnen hebben voor alle producten die hij in een jaar heeft, en voor alles buiten het seizoen een aantal van 0 kunnen opgeven. Nu moet je die 0 waarden er alsnog handmatig uithalen. We zouden ze wel in het upload overzicht kunnen tonen (misschien in een afwijkende kleur), maar ze eruit halen bij Resultaat Opslaan
  2. Je ziet niet wat er fout is als de rij rood is
  3. Misschien zouden we de prijzen in deze view nog op 2 decimalen moeten afronden
  4. Het maximum aantal parameters in een POST is default 1000. Bij 2× uploaden van de voorbeeld Excels (beide ~134 rijen, maal 6 waarden) en daarna Valideren, krijg je een foutmelding daarover. Bij valideren van meer dan 166 producten zal het ook niet goed gaan
mgvd commented 8 months ago

Mooi werk al! Ik denk dat we met iemand die die lijsten uploadt moeten overleggen en je suggesties bespreken. Ik regel een call

bveldkamp commented 7 months ago

TODO