Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Přidán přístup k plabám pro hospodáře a vedoucí VzA #2619

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

marekdedic
Copy link
Collaborator

@marekdedic marekdedic commented Feb 7, 2024

S tímhle PR mají hospodáři VzA přístup ke všem platbám dané jednotky - nejsou tam víc granulární oprávnění, ale mně to asi přijde v pohodě, hospodáři VzA jsou typicky lidi, které ta jednotka nějakým způsobem stejně zná a hospodář střediska s nimi nutně interaguje...

Stejně tak jsem rovnou přidal vedoucí VzA, ty dokonce schvaluje střediskovka...

Closes #2617

@marekdedic marekdedic changed the title Přidán přístup k plabám pro hospodáře VzA Přidán přístup k plabám pro hospodáře a vedoucí VzA Feb 7, 2024
@marekdedic marekdedic marked this pull request as ready for review February 7, 2024 15:42
@@ -236,7 +236,7 @@ private function assertCanViewBankAccount(int $id): void
if (
$role->getUnitId() === $account->getUnitId()
&& $role->isBasicUnit()
&& ($role->isAccountant() || $role->isOfficer() || $role->isEventManager())
&& ($role->isAccountant() || $role->isOfficer() || $role->isEventManager() || $role->isEducationAccountant())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a proc to nepripojit pod funkci role->isAccountant() a dělat na to samostatnou?

A nenastane v obou případech, že hospodář VzA dostane přístup k uctu i jednotky jako takové?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jo, určitě by to šlo připojit pod isAccountant(), ale přišlo mi lepší nemíchat 2 různé role do jedné funkce, zvlášť když ta vnitřní logika těch rolí je úplně jiná.

Ano, hospodář VzA tím dostane přístup i k platbám jednotky. Mně to přijde asi v pohodě, z důvodů popsaných v těle PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Za mě je to stále Accountant, tak bych to spíš spojil. Protože stejně jsme udělali Officer pres více úrovní a rolí.

S tím přístupem jsem si toho nejdřív nevšiml. Každopádně bych přidal i hlášku do plateb, že přístup k platbám mají i role VzA pod danou jednotkou.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, sloučil jsem ty funkce. Co myslíš hláškou? Permanentní baner mi přijde jako overkill...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jako hospodar, co si zadá ucet do h.skautingu neocekávám, že se k tomu automaticky dostane i vedoucí vzdělávačky. Pokud existuje nějaký precedens pro přístup lidí z VzA k jednotce, tak to udělat podle toho @manulekonecna

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tak jako precedens je podle mě trochu v cesťácích - tam taky s rolí vzdělávačky vidím i cesťáky a auta střediska...

Napadá mě ještě tam ten baner dát a třeba za 3 měsíce ho zase zrušit. Klidně na to připravím druhé PR...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jeste bych rád pohled @manulekonecna nebo @jerrysohn

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jen ještě pro kontext doplním, jaké je typické řešení tohohle problému ze strany pořádajících jednotek - hospodář dostane v ISu roli správce akcí, potažmo vedoucí/admin... :D

@marekdedic marekdedic requested a review from sinacek February 7, 2024 16:22
@marekdedic marekdedic force-pushed the education-economist-payment-access branch from 7e6004d to b81a64b Compare February 8, 2024 09:00
@marekdedic
Copy link
Collaborator Author

@sinacek @jerrysohn @manulekonecna Ahoj, potřeboval bych nějaké vyjádření k tomuto půl roku starému PR - otevírám druhý ročník naší VzA, ve kterém mě tohle blokuje. Stejný problém pak v zápětí budu řešit i pro cesťáky, kde bych jako hospodář VzA potřeboval taky větší přístup - prostě teď některé důležité části h.skautingu jsou nepoužitelné bez role u jednotky, kterou podle mě nemám mít. Díky.

@marekdedic marekdedic force-pushed the education-economist-payment-access branch from b81a64b to 3d5d811 Compare October 30, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

S rolí "VzA: hospodář" nemám právo zobrazit platby
2 participants