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

More available columns in folder_contents. #83

Merged
merged 5 commits into from Apr 8, 2016
Merged

Conversation

thet
Copy link
Member

@thet thet commented Apr 4, 2016

Follow up of #78 and obsoletes that. The other one contained the test isolation fixes branch, which failed through the jenkins test. Therefore this new branch with two more features/commits.

  • Add Creator, Description, end, start and location to the available columns and context attributes for folder_contents.
  • Show attributes from _unsafe_metadata if user has "Modify Portal Content" permissions.
  • Remove portal_type from available columns and use Type instead, which is meant to be read by humans.
    portal_type is now available on the attributes object.
  • Added most notably portal_type, review_state and Subject but also exclude_from_nav, is_folderish, last_comment_date, meta_type and total_comments to BaseVocabularyView translate_ignored list.
    Some of them are necessary for frontend logic and others cannot be translated.
    Fixes impossible to insert a picture already available on the website (Internal Image) #77

@jensens
Copy link
Sponsor Member

jensens commented Apr 5, 2016

hmm, this should go into 5.1 only, since the type -> portal_type change

@thet
Copy link
Member Author

thet commented Apr 5, 2016

a translated Types was once used here, where it was changed to portal_types:
cc0158e

Looks like an accident...

@thet thet closed this Apr 5, 2016
@thet thet reopened this Apr 5, 2016
@thet
Copy link
Member Author

thet commented Apr 5, 2016

Regarding "Extend translation ignore list for metadata values in vocabulary ", 1a45137
From the current set of portal catalog metadata, only Type is left to be translated.

@thet
Copy link
Member Author

thet commented Apr 5, 2016

@vangheem can you have a look at this PR, especially these commits?

  1. 5ac4636

Show attributes from _unsafe_metadata if user has "Modify Portal Content" permissions.

  1. 1a45137

Extend translation ignore list for metadata values in vocabulary
Added most notably portal_type, review_state and Subject but also exclude_from_nav, is_folderish, last_comment_date, meta_type and total_comments to BaseVocabularyView translate_ignored list.
Some of them are necessary for frontend logic and others cannot be translated.
Fixes #77

@thet
Copy link
Member Author

thet commented Apr 5, 2016

Sorry for packing again too much into one singe PR. At least it's different commits, so if it's necessary (controversy on individual commits), I can split that apart.

thet added 5 commits April 7, 2016 00:24
Add ``Creator``, ``Description``, ``end``, ``start`` and ``location`` to the available columns and context attributes for folder_contents.
Exclude ``Creator`` from the list of ``_unsafe_metadata``.
…``Type`` instead.

``portal_type`` is now available on the attributes object.
Added most notably `portal_type`, `review_state` and `Subject` but also `exclude_from_nav`, `is_folderish`, `last_comment_date`, `meta_type` and `total_comments` to ``BaseVocabularyView`` ``translate_ignored`` list.
Some of them are necessary for frontend logic and others cannot be translated.
Fixes #77
@jensens
Copy link
Sponsor Member

jensens commented Apr 7, 2016

lgtm, any reason to not merge?

@agitator
Copy link
Member

agitator commented Apr 7, 2016

yes please

@jensens jensens merged commit 59f1b26 into master Apr 8, 2016
@jensens jensens deleted the thet-fcmorecolumns branch April 8, 2016 06:57
@thet
Copy link
Member Author

thet commented Apr 8, 2016

@terapyon this commit 3b529a3 fixes an issue you introduced in f167266 and 92e9749
I understand the intention of your fix, but it broke things at other places.
Only Type is left from the catalog metadata to be translated by default (except you use other metdata, which make sense translating). I'm not sure, if my fix goes that much against the intention of your commits, that we could also avoid translating at all...

@terapyon
Copy link
Member

Thank you for fixing the bug. I will check it out again.

@@ -183,6 +184,8 @@ def __call__(self):
]
if attributes:
base_path = getNavigationRoot(context)
sm = getSecurityManager()
can_edit = sm.checkPermission('Modify portal content', context)
for vocab_item in results:
Copy link
Contributor

Choose a reason for hiding this comment

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

@thet Hi, I'm a bit confused. In folder_contents, the getVocabulary view is called on the portal. Therefore the context is the portal.
Why does a user need to have "Modify portal content" globally on the portal to be able to view the Creator column in any subfolder?
Why is Creator considered unsafe in the first place?

Copy link
Member Author

Choose a reason for hiding this comment

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

Having this info exposed could be considered unsafe in some use cases. What I did, was instead of never exposing those unsafe metadata, they are now exposed if the user has "Modify portal content" - on the context itself. Before, these metadata columns were just skipped.

I'm fine with these columns being unsafe, but maybe @vangheem can give some more insight. He added it in:

plone/plone.app.widgets@5055725#diff-83a9a1cb178e6b38717c2427eac08433R27
and
plone/plone.app.widgets@fd5c8cf#diff-cc8621ac4fb1caf4495921abbc02f895R33

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.

impossible to insert a picture already available on the website (Internal Image)
5 participants