mam/mamweb
11
0
Fork 0
Code Issues Pull requests 8 Projects Wiki Activity Actions

verejny kontaktnicek #71

Merged
ticvac merged 10 commits from kontaktnicek_pro_vsecny into master 2024-11-12 21:15:25 +01:00
Conversation 1 Commits 10 Files changed 7 +4 -3
1 changed files with 4 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 1b755ad1f7 - Show all commits

7
soustredeni/views.py
View file

@ -121,9 +121,6 @@ def soustredeniKontaktnicekView(request, soustredeni, typ):
# nebyl jsi tam, nebo nejsi org # nebyl jsi tam, nebo nejsi org
if (not request.user in [u.osoba.user for u in soustredeni.ucastnici.all()]) and not request.user.je_org: if (not request.user in [u.osoba.user for u in soustredeni.ucastnici.all()]) and not request.user.je_org:
zelvuska commented 2024-11-06 18:16:03 +01:00
Outdated
Review
Copy link

Asi bych napsal popisnější hlášku, vynechal bych tu část z orgem, a naopak bych tam připsal, že nejsi přihlášený (to se stane asi častěji než že bych se dostal na špatný kontaktníček).

Také si nejsem jistý, jestli HttpResponse je to správné. (Jakože vrátí to něco, co moc nevypadá jako M&M stránky, ne?)

Asi bych napsal popisnější hlášku, vynechal bych tu část z orgem, a naopak bych tam připsal, že nejsi přihlášený (to se stane asi častěji než že bych se dostal na špatný kontaktníček). Také si nejsem jistý, jestli HttpResponse je to správné. (Jakože vrátí to něco, co moc nevypadá jako M&M stránky, ne?)
ledoian commented 2024-11-12 19:29:15 +01:00
Outdated
Review
Copy link

Uh, je to ekvivalentní s if request.user not in […] and not request.user.is_org:?

Uh, je to ekvivalentní s `if request.user not in […] and not request.user.is_org:`?
raise PermissionDenied() raise PermissionDenied()
# není k dispozici
if (not soustredeni.kontaktnicek_pdf and typ == "pdf") or (not soustredeni.kontaktnicek_vcf and typ == "vcf"):
raise Http404()
kontaktnicky = { kontaktnicky = {
ledoian commented 2024-11-12 19:30:30 +01:00
Outdated
Review
Copy link

404 není správný návratový kód (kontaktníček zjevně existuje, ale nemám k němu přístup), má být 403 (PermissionDenied se myslím jmenuje ta Djangová výjimka).

404 není správný návratový kód (kontaktníček zjevně existuje, ale nemám k němu přístup), má být 403 (`PermissionDenied` se myslím jmenuje ta Djangová výjimka).
'pdf': (soustredeni.kontaktnicek_pdf, 'applcation/pdf', 'rb'), 'pdf': (soustredeni.kontaktnicek_pdf, 'applcation/pdf', 'rb'),
ledoian commented 2024-11-12 19:35:13 +01:00
Outdated
Review
Copy link

Poněkud se mi nelíbí místní duplikace kódu, když se tam mění dohromady tři věci (až přidáme další, tak někde něco zapomeneme přepsat a bude to bug…). Co třeba:

# typ -> (field, mime_type, otevreni)
kontaktnicky = {
|   'pdf': (soustredeni.kontaktnicek_pdf, 'applcation/pdf', 'rb'),
|   'vcf': (soustredeni.kontaktnicek_vcf, 'text/vcard', 'r'), # vcf je texťák, nevím, jestli je potřeba ho otevítat binárně.
}
try:
	field, mime, otevreni = kontaktnicky[typ]
except KeyError as e:
	raise ValueError("Neznámý typ kontaktníčku") from e
if not field:
|   ...
with open(field.path, otevreni) as soubor:
|   return HttpResponse(soubor.read(), content_type=mime)
Poněkud se mi nelíbí místní duplikace kódu, když se tam mění dohromady tři věci (až přidáme další, tak někde něco zapomeneme přepsat a bude to bug…). Co třeba: ```python3 # typ -> (field, mime_type, otevreni) kontaktnicky = { | 'pdf': (soustredeni.kontaktnicek_pdf, 'applcation/pdf', 'rb'), | 'vcf': (soustredeni.kontaktnicek_vcf, 'text/vcard', 'r'), # vcf je texťák, nevím, jestli je potřeba ho otevítat binárně. } try: field, mime, otevreni = kontaktnicky[typ] except KeyError as e: raise ValueError("Neznámý typ kontaktníčku") from e if not field: | ... with open(field.path, otevreni) as soubor: | return HttpResponse(soubor.read(), content_type=mime) ```
ledoian commented 2024-11-12 20:33:54 +01:00
Outdated
Review
Copy link

Uh, ten if not field: ... byl náznak toho, že není potřeba výčet typů v if (not soustredeni.kontaktnicek_pdf and typ == "pdf") or (not soustredeni.kontaktnicek_vcf and typ == "vcf"):, sorry za neexplicitnost.

Uh, ten `if not field: ...` byl náznak toho, že není potřeba výčet typů v `if (not soustredeni.kontaktnicek_pdf and typ == "pdf") or (not soustredeni.kontaktnicek_vcf and typ == "vcf"):`, sorry za neexplicitnost.
@ -135,6 +132,10 @@ def soustredeniKontaktnicekView(request, soustredeni, typ):
except KeyError as e: except KeyError as e:
ledoian commented 2024-11-12 19:36:23 +01:00
Outdated
Review
Copy link

Možno rovnou:

with open(…) as whatever:
	return HttpResponse(…)
Možno rovnou: ```python3 with open(…) as whatever: return HttpResponse(…) ```
raise ValueError("Neznámý typ kontaktníčku") from e raise ValueError("Neznámý typ kontaktníčku") from e
# není k dispozici
if not field:
raise Http404()
with open(field.path, otevreni) as kontaktnicek: with open(field.path, otevreni) as kontaktnicek:
response = HttpResponse(kontaktnicek.read(), content_type=mime) response = HttpResponse(kontaktnicek.read(), content_type=mime)
response['Content-Disposition'] = 'attachment; filename="kontaktnicek.{}"'.format(typ) response['Content-Disposition'] = 'attachment; filename="kontaktnicek.{}"'.format(typ)