contributte / menu-control

🍔 Menu and breadcrumb components for Nette framework (@nette)
MIT License
28 stars 20 forks source link

added suport for ACL #8

Closed whipsterCZ closed 10 years ago

whipsterCZ commented 10 years ago

@see http://doc.nette.org/en/2.1/access-control#toc-permission-acl

davidkudera commented 10 years ago

super, I was planning something like that :-)

davidkudera commented 10 years ago

I also wanted to add some "auto" option where resource is presenter name and permission is current presenter action. And than maybe some global option for that. Maybe some people using it this way, so it would be simplier for them. What do you think?

Edit: it is not like i want you to do that (unless you want of course). These are just my thoughts about acl and this menu ;-)

whipsterCZ commented 10 years ago

I do not think that the AUTO option is a good idea. It brings some magic and I don't have idea how to implemented it simply & clearly. It can throw exceptions in case that resource is not added to 'Nette\Security\Permission'

Thanks for your component, it is very usefull and well done..

davidkudera commented 10 years ago

Well, I thank you.

Also you are probably right about auto option. But now different aproach come to my mind. There https://github.com/sakren/nette-menu/issues/4 I've added support for <module> placeholder. Maybe I can add also for example <presenter> and <action> placeholders which could be used for example in this acl configuration.

I think that this would be much better than "auto magic". Don't you think? But this will be a bit more complicated because if I add it, it should work for all options, not just for acl.

Thanks for updates by the way. And please, can I ask you to squash all commits?

whipsterCZ commented 10 years ago

Ahoj

přiznám jsem že jsem asi zcela nepochopil k čemu je ta možnost Include Jsou to items které nejsou visual?

Můžeš mi prosím trochu osvětlit ten tvůj nápad s tím

díky Dan

2014-08-04 14:40 GMT+02:00 David Kudera notifications@github.com:

Well, I thank you.

Also you are probably right about auto option. But now different aproach come to my mind. There #4 https://github.com/sakren/nette-menu/issues/4 I've added support for placeholder. Maybe I can add also for example and placeholders which could be used for example in this acl configuration.

I think that this would be much better than "auto magic". Don't you think? But this will be a bit more complicated because if I add it, it should work for all options, not just for acl.

Thanks for updates by the way. And please, can I ask you to squash all commits?

— Reply to this email directly or view it on GitHub https://github.com/sakren/nette-menu/pull/8#issuecomment-51054310.


Ing.* Daniel Kouba* PHP & HTML5, iOS Developer, Contami Nation Manager +420 777 900 890, whipstercz@gmail.com , www.danielkouba.cz

davidkudera commented 10 years ago

Ahoj, možná bych to měl v dokumentaci lépe vysvětlit. Představ si, že máš v menu odkaz třeba na seznam všech článků. Když na něj kliknu, tak se zaktivní nějakou css clasou. Chci ale, aby zůstal ten odkaz aktivní i když kliknu na detail článku samotného. Takže si includnu třeba Article:detail.

No a z nějakýho důvodu jsem vytvořil placeholder <module>, který se nahradí za název modulu. Napadlo mě, že bych mohl udělat něco podobného pro presentery a akce. Je možný, že někteří používají acl pro akce presenterů tak, že se do startupu dá něco takovýho:

if (!$this->user->isAllowed($this->name, $this->action)) {
    // do something
}

Když je to takhle automaticky, tak by se hodilo mít i odkazy automaticky (pokud to člověk vyžaduje).

Takže konfigurace by mohla být třeba nějak takto:

Authors:
    target: Authors:default
    allow:
        acl:
            resource: <presenter>
            permission: <action>

Potom by se nemuselo totiž psát 2x v konfiguraci jednoho odkazu název presenteru i akce.

Ale tohle už je mimo tento PR a je to i na nějakou jinou dobu nebo i verzi. Jestli vůbec..

Edit: jo a jsou to items, které nejsou visual :-)

whipsterCZ commented 10 years ago

Líbí se mi to :) nicméně já bych to takto asi nikdy nepoužil :)

Dne 5. srpna 2014 9:13 David Kudera notifications@github.com napsal(a):

Ahoj, možná bych to měl v dokumentaci lépe vysvětlit. Představ si, že máš v menu odkaz třeba na seznam všech článků. Když na něj kliknu, tak se zaktivní nějakou css clasou. Chci ale, aby zůstal ten odkaz aktivní i když kliknu na detail článku samotného. Takže si includnu třeba Article:detail.

No a z nějakýho důvodu jsem vytvořil placeholder , který se nahradí za název modulu. Napadlo mě, že bych mohl udělat něco podobného pro presentery a akce. Je možný, že někteří používají acl pro akce presenterů tak, že se do startupu dá něco takovýho:

if (!$this->user->isAllowed($this->name, $this->action)) { // do something}

Když je to takhle automaticky, tak by se hodilo mít i odkazy automaticky (pokud to člověk vyžaduje).

Takže konfigurace by mohla být třeba nějak takto:

Authors: target: Authors:default allow: acl: resource: permission:

Potom by se nemuselo totiž psát 2x v konfiguraci jednoho odkazu název presenteru i akce.

Ale tohle už je mimo tento PR a je to i na nějakou jinou dobu nebo i verzi. Jestli vůbec..

— Reply to this email directly or view it on GitHub https://github.com/sakren/nette-menu/pull/8#issuecomment-51158545.


Ing.* Daniel Kouba* PHP & HTML5, iOS Developer, Contami Nation Manager +420 777 900 890, whipstercz@gmail.com , www.danielkouba.cz

whipsterCZ commented 10 years ago

chci říct, to psaní které ti to ušetří v configu, pak zdaleka převáží psaní kódu na straně ACL

budeš mít hrozně moc privilegií - pro každou akci jednu, přestože většinou stačí view/edit

Ale rozhodně třeba resource se mi líbí jen dotaz jak by se přeložil symbol Homepage / homepage / HomepagePresenter ?

Dne 5. srpna 2014 14:17 Daniel Kouba whipstercz@gmail.com napsal(a):

Líbí se mi to :) nicméně já bych to takto asi nikdy nepoužil :)

Dne 5. srpna 2014 9:13 David Kudera notifications@github.com napsal(a):

Ahoj, možná bych to měl v dokumentaci lépe vysvětlit. Představ si, že máš

v menu odkaz třeba na seznam všech článků. Když na něj kliknu, tak se zaktivní nějakou css clasou. Chci ale, aby zůstal ten odkaz aktivní i když kliknu na detail článku samotného. Takže si includnu třeba Article:detail .

No a z nějakýho důvodu jsem vytvořil placeholder , který se nahradí za název modulu. Napadlo mě, že bych mohl udělat něco podobného pro presentery a akce. Je možný, že někteří používají acl pro akce presenterů tak, že se do startupu dá něco takovýho:

if (!$this->user->isAllowed($this->name, $this->action)) { // do something}

Když je to takhle automaticky, tak by se hodilo mít i odkazy automaticky (pokud to člověk vyžaduje).

Takže konfigurace by mohla být třeba nějak takto:

Authors: target: Authors:default allow: acl: resource: permission:

Potom by se nemuselo totiž psát 2x v konfiguraci jednoho odkazu název presenteru i akce.

Ale tohle už je mimo tento PR a je to i na nějakou jinou dobu nebo i verzi. Jestli vůbec..

— Reply to this email directly or view it on GitHub https://github.com/sakren/nette-menu/pull/8#issuecomment-51158545.


Ing.* Daniel Kouba* PHP & HTML5, iOS Developer, Contami Nation Manager +420 777 900 890, whipstercz@gmail.com , www.danielkouba.cz


Ing.* Daniel Kouba* PHP & HTML5, iOS Developer, Contami Nation Manager +420 777 900 890, whipstercz@gmail.com , www.danielkouba.cz

davidkudera commented 10 years ago

Bylo by to jen Homepage nebo jakkoliv co vrací v presenteru getName(). Ještě bych ale musel promyslet, jestli to neoříznout o název modulu na začátku, protože k tomu už je vlastně vytvořený <module>.

Ale s tím množství privilegií máš určitě pravdu.

No a jinak, jestli se ti povede ten squash commitů, tak zítra večer bych to mohl určitě mergnout ;-)

whipsterCZ commented 10 years ago

jsem nováček co znamená squash comittů?

Dne 5. srpna 2014 14:57 David Kudera notifications@github.com napsal(a):

Bylo by to jen Homepage nebo jakkoliv co vrací v presenteru getName(). Ještě bych ale musel promyslet, jestli to neoříznout o název modulu na začátku, protože k tomu už je vlastně vytvořený .

Ale s tím množství privilegií máš určitě pravdu.

No a jinak, jestli se ti povede ten squash commitů, tak zítra večer bych to mohl určitě mergnout ;-)

— Reply to this email directly or view it on GitHub https://github.com/sakren/nette-menu/pull/8#issuecomment-51193206.


Ing.* Daniel Kouba* PHP & HTML5, iOS Developer, Contami Nation Manager +420 777 900 890, whipstercz@gmail.com , www.danielkouba.cz

davidkudera commented 10 years ago

V pohodě, chápu. Jinak je to ke sloučení commitů dohromady, takže to dává větší smysl. Jednoduše se přidává nějaká jedna funkce třeba právě přes PR a to se rovná jeden commit ;-) krátké počteníčko

whipsterCZ commented 10 years ago

Rozumím tomu co po mně chceš ale moc jsem nepcohopil jak se to dělá :( ani z toho článku

je to takto ok?

$ git fetch nette-menu $ git checkout omgpull $ git rebase -i nette-menu/master < choose squash for all of your commits, except the first one >< Edit the commit message to make sense, and describe all your changes >

$ git push origin omgpull -f

Dne 5. srpna 2014 15:38 David Kudera notifications@github.com napsal(a):

V pohodě, chápu. Jinak je to ke sloučení commitů dohromady, takže to dává větší smysl. Jednoduše se přidává nějaká jedna funkce třeba právě přes PR a to se rovná jeden commit ;-) krátké počteníčko http://blog.steveklabnik.com/posts/2012-11-08-how-to-squash-commits-in-a-github-pull-request

— Reply to this email directly or view it on GitHub https://github.com/sakren/nette-menu/pull/8#issuecomment-51197948.


Ing.* Daniel Kouba* PHP & HTML5, iOS Developer, Contami Nation Manager +420 777 900 890, whipstercz@gmail.com , www.danielkouba.cz

davidkudera commented 10 years ago

Tady jsem našel jednodušší postup: link. Je to až dole a začíná to s git rebase -i HEAD~10. Kdyžtak kdyby se nepodařilo, tak během zítřka bych mohl pomoct ;-)