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

add some body tags to determine permission needed and roles needed to view #50

Merged
merged 3 commits into from
Sep 10, 2015

Conversation

djay
Copy link
Member

@djay djay commented Jul 18, 2015

The goal is plone/Products.CMFPlone#465, which is make it easy for a themer to theme the backend separate from the frontend while still using the toolbar.

I'm not sure the viewrole- ones are needed (or desired for security reasons). Maybe it should just say be restricted to viewrole-anonymous?

I think with this change a seperated theme could be as simple as

Also, need some advice on the best way to implement this. Seems a bit hacky.

@djay
Copy link
Member Author

djay commented Jul 18, 2015

@vangheem @jcerjak advice?

@vangheem
Copy link
Member

Sorry, about to get on plane. If you only need to check a few permissions,
maybe you just use checkPermission against the context object for each
permission you care about?
On Jul 18, 2015 9:47 AM, "Dylan Jay" [email protected] wrote:

@vangheem https://github.com/vangheem @jcerjak
https://github.com/jcerjak advice?


Reply to this email directly or view it on GitHub
#50 (comment)
.

@jensens jensens changed the title [WIP] add some body tags to determine permission needed and roles needed to view add some body tags to determine permission needed and roles needed to view Jul 21, 2015
@djay
Copy link
Member Author

djay commented Sep 8, 2015

@tomgross I've removed the tags about roles required. Seems permission required for a view is normally enough to differentiate backend vs frontend.
@vangheem so you think instead of revealing all permissions needed it should just be a binary tag? It would make the diazo rules clearer if it was backend maybe?

I guess I was thinking that something that requires an additional permission could then be themed differently. Perhaps thats where the roles required tags I deleted might have been useful, where you could theme in elements specific to reviewers for example?

@vangheem
Copy link
Member

vangheem commented Sep 8, 2015

Simple is often better. All these things related to permissions might be a bit difficult for themers to fully get.

I am a bit worried about revealing this data publicly. Is this something we should worry about at all?

@djay
Copy link
Member Author

djay commented Sep 9, 2015

You have to have the permission to see it so its not revealing much. The
public would only see view-permission-view or view-permission-none

On Tue, 8 Sep 2015 20:39 Nathan Van Gheem [email protected] wrote:

Simple is often better. All these things related to permissions might be a
bit difficult for themers to fully get.

I am a bit worried about revealing this data publicly. Is this something
we should worry about at all?


Reply to this email directly or view it on GitHub
#50 (comment)
.

@djay
Copy link
Member Author

djay commented Sep 9, 2015

@vangheem so based on the other thread you seem to be proposing I change it so you would add the following in your custom there.

  <!-- Import Barceloneta rules -->
  <xi:include href="++theme++barceloneta/backend.xml" />

  <rules css:if-content="body.frontend" css:if-content="#visual-portal-wrapper">
               <theme href="index.html" />
    ....

or should it be css:if-not-content="body.backend"?

@djay
Copy link
Member Author

djay commented Sep 10, 2015

@vangheem @esteele I've updated it so you can use either the body.frontend or body.viewpermission-view tags. Is there anything else I have to do?

@vangheem
Copy link
Member

Looks fine.

vangheem added a commit that referenced this pull request Sep 10, 2015
add some body tags to determine permission needed and roles needed to view
@vangheem vangheem merged commit bddc8c4 into master Sep 10, 2015
@vangheem vangheem deleted the backend-tag branch September 10, 2015 12:28
@vangheem
Copy link
Member

Can you add a changelog please? Don't bother with a PR for it.

@vangheem
Copy link
Member

Oops, this didn't have a test run attached with it. I'll watch jenkins and revert if there is a problem.

@mauritsvanrees
Copy link
Member

@mauritsvanrees
Copy link
Member

oh, I may see the problem, giving it a quick try now

@vangheem
Copy link
Member

yes, "getattr(view, 'ac_permissions')" should be "getattr(view, 'ac_permissions', None)"

Want me to make the change?

@mauritsvanrees
Copy link
Member

Done. c29e1cb

@vangheem
Copy link
Member

thank you.

talarias pushed a commit that referenced this pull request Dec 9, 2021
Add preview for thumbnail on @@theming-controlpanel.  Fixes #49.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants