-
Notifications
You must be signed in to change notification settings - Fork 140
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
PWFC extension changes #177
Conversation
@@ -82,6 +82,9 @@ doctrine_phpcr: | |||
- en | |||
- de | |||
|
|||
cmf_core: | |||
role: CAN_VIEW_NON_PUBLISHED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default role is "AUTHENTICATED_ANONOMOUSLY", so the PWFC status is effectively irrelevant unless we override this.
Also, we should change the name of this option: symfony-cmf/core-bundle#49
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't you need to define that role in security.yml? and to make this complete, we should have an unpublished content and a hint on the home page that after logging in you see more. we should probably finish #169 - i started that but never managed to get back to it to fix the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I guess we should add it to security.yml
- in this case I just
wanted that the non-published stuff was hidden. Will be cool to have the
login functionality.
On Wed, Jun 05, 2013 at 08:37:31AM -0700, David Buchmann wrote:
In app/config/config.yml:
@@ -82,6 +82,9 @@ doctrine_phpcr:
- en
- de+cmf_core:
- role: CAN_VIEW_NON_PUBLISHED
don't you need to define that role in security.yml? and to make this
complete, we should have an unpublished content and a hint on the home
page that after logging in you see more. we should probably finish [1]#169
i started that but never managed to get back to it to fix the tests.
—
Reply to this email directly or [2]view it on GitHub.References
Visible links
- enabled login for admin backend
enabled login for admin backend #169- https://github.com/symfony-cmf/cmf-sandbox/pull/177/files#r4548016
imho the CoreBundle should only load the publish workflow handling if its enabled. or do we need the service always present because controllers rely on it? could we put a dummy service then or does role anonymous make us never really run the logic? i am worried about overhead when the workflow is not needed at all. |
Yeah.. what if we set the PWFC via. a setter with a kernel.request event On Wed, Jun 05, 2013 at 08:35:15AM -0700, David Buchmann wrote:
|
well if we provide a default service by the core bundle that can be injected, it should be enough. otherwise we could tag them and have a compiler pass that adds the set pwfc method call to the definition. if pwf is disabled, we should provide/inject a noop-checker. the service should always be able to rely on the pwfc being injected, not assume null means do no check - otherwise setup errors would lead to data leaks. |
the required PRs are merged. |
ok, updated |
ok .. we will merge it when we have the next round of tags .. hopefully tomorrow. |
Depends on: