Podezřelé semináře (#1465) #65
Merged
zelvuska
merged 27 commits from podezrele-seminare into master 2 weeks ago
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'podezrele-seminare'
Deleting a branch is permanent. It CANNOT be undone. Continue?
Oproti #1465 zbývají už jen:
WIP: Podezřelé semináře (Fix: #1465)to WIP: Podezřelé semináře (#1465) 3 weeks agoZatím vypadá dobře :-)
(i.e. zajímá mě, jak moc je tohle relevantní pro bugy vzniknuvší v !66 a když to vidím, tak si sem dávám značku ať ty viděné změny nemusím kontrolovat znovu :-D)
<div class="mam-org-only"><ul><li><a href="/admin/seminar/novinky/{{novinka.pk}}">Upravit novinku</a><li><a href="/admin/novinky/novinky/{{novinka.pk}}">Upravit novinku</a>Možná radši
{% url 'admin:novinky_novinky_edit' pk=novinka.pk %}? (resp. aproximuji syntaxi, nekopírovat :-D)Po mergi oddělení tvorby a po commitu
8fd582d192zbývá:Zbývá
WIP: Podezřelé semináře (#1465)to Podezřelé semináře (#1465) 3 weeks agoUlohaVzorakNode,TextNode,CastNode,OrgTextNode,Já vím, že chybělo už předtím, ale chybí tu
ReseniNode. (i později v deklaracích jednotlivých adminů)'probehlo/',views.SoustredeniListView.as_view(),name='seminar_seznam_soustredeni'name='soustredeni_probehlo'S tímhle přejmenováním spíš nesouhlasím. To, že se na webu seznam soustředění vyskytuje v „Soustředění → Proběhlo“ je podle mě UI detail a z pohledu vývojáře chci spíš vědět, že adresa vede na seznam soustředění (nebo přehled soustředění).
from django import templatefrom enum import Enumimport seminar.models as mfrom .models import RocnikNode, CisloNode, CastNode, TextNode, TemaVCisleNode, UlohaVzorakNode, UlohaZadaniNode, PohadkaNode(Těchto TreeNodů je tu málo, ale nechce se mi teď zjišťovat, které tři chybí…)
import seminar.models as simport seminar.models as mfrom .models import TemaVCisleNode, RocnikNode, CisloNode, UlohaVzorakNode, UlohaZadaniNode, TreeNode, CastNode, TextNode, ReseniNode, PohadkaNode, OrgTextNode(Tu chybí
MeziCisloNode, to je vcelku podezřelé…)Asi nejsem schopen opravit (aniž bych se snažil pochopit treenode).
if isinstance(value,KonferaNode): return "Konfera"if isinstance(value,ClanekNode): return "Článek"# if isinstance(value,KonferaNode): return "Konfera" # FIXME neexistuje# if isinstance(value,ClanekNode): return "Článek" # FIXME neexistujeTyhle komentáře způsobí tiché spadnutí. Byl by to
NameError, nejspíš i při validním volání, teď je toreturn None, díky čemuž se bude chyba špatně lovit.NameErrorje za mě fajn, ale klidně to jde vyřešit nějakým globálnímraise ValueError('tenhle typ TreeNode neexistuje').To je asi otázka spíš na to, proč KonferaNode a ClanekNode přestalo existovat a proč s tím nepřestal existovat i tento kód…
def isKonfera(value):return isinstance(value, m.KonferaNode)# FIXME neexistuje# return isinstance(value, KonferaNode)Prosím aspoň zalogovat, neumřít tiše.
Stejně jako výše. KonferaNode prostě neexistuje. (Přestala existovat v migraci 0080, které jsi autor…)
from django.test import TestCaseimport treenode.treelib as tlimport seminar.models as mfrom .models import CastNodeV principu myslím, že tady je lepší řešení spíš
import treenode.models as tn, ale až někdo někdy bude pokračovat ve psaní těchto testů, tak si to tam nejspíš dopíše, tak je to teď asi jedno…Proč si myslíš, že je lepší náhodná zkratka, než importování přímo věcí (když je to lokální věc).
A celkově nad tímhle (jak má vypadat importování toho všeho, co předtím bylo
import seminar.models as m) trochu dumám.class ProfileEditForm(UdajeForm):err_logger = logging.getLogger('seminar.edit.problem')err_logger = logging.getLogger('personalni.edit.problem')Heh, tohle nikde není deklarované, i když se to od pohledu má chovat (zvlášť co se týče ochrany dat) stejně jako
personalni.prihlaska.problem. Myslím, že by mělo stačit tady (jinde to stejně není) přejmenovat napersonalni.prihlaska.problem.edit(ale teď nekontroluji, jestli si pamatuji chování pythoního logování správně…)To nebylo ani předtím… Myslíš,
že personalni.prihlaska.problem.editpomůže?Mám za to, že Pythoní logování drží hierarchii podle teček, takže
personalni.prihlaska.problem.editse chová by default stejně jakopersonalni.prihlaska.problem. Ale nevěřím tomu úplně, tak to asi na webschůzce vyzkouším na testwebu (ze shellu)…Asi OK (až na to, co píšu výš). Věci kolem TreeNodů spíš prosím někam poznamenat, chápu, že se nikomu nechce je implementovat teď hned…
(A já se jdu pokusit to někam mergenout, ze cviku…)
Nějak to mergenout šlo (konflikt byl myslím jen v
treenode/models.pya bylo potřeba předělat pár views), ukázkový merge (neotestovaný) je ve větvibez_treenodu_i_bez_podezrelych_seminaru.Nevím, jestli chceme tuhle větev přímo vzít a testovat, spíš si imho chceme vzít inspiraci a nejdřív zamergeovat jeden PR, pak nový
mastermergenout do druhého PR (velmi podobně jako na té větvi) a pak zamergeovat druhý PR. (Technicky je to jedno, ale půjde se líp vyznat v tom, co se v gitu dělo, a navíc gitea nebude úplně zmatená z toho, co těm pull requestům děláme…)Viděno až po
ac6c41cc88, LGTM. (TreeNody mi tu klidně zatím nech, domlátím během schůzky asi…)Kouzelnická poznámka:
git grep seminar | sed '\|/migrations/| d; \|deploy_v2/| d; \|split-apps-meta/| d'popisuje posledních 53 seminářů:SeminarModelBaseaOverwriteStorage(možná je chceme dát dovarious?)seminarexistuje)Navíc v
mamweb.settings_commonmámeSEMINAR_RESENI_DIRaSEMINAR_KONFERY_DIR, využívají se na jednom místě v kóduSkoro se mi chce tomu říkat úspěch, nebo aspoň 98% úspěchu. (Přesouvání
SeminarModelBasebude možná trošku zábava, DB tabulky by měly jít přejmenovat, dokumentaci aktualizujme…)A kdyžtak navrhuji tenhle komentář prostě zkopírovat do Kance (včetně příkazu) a dořešit jindy, separátně, později,
možná, …'probehlo/',views.SoustredeniListView.as_view(),name='seminar_seznam_soustredeni'name='soustredeni_seznam'good for me… (ještě ten
printdoseminar/models.py)024f8e0a80into master 2 weeks ago024f8e0a80.