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

Improvements to PR #246 (hiding the "Upload" tab) #247

Merged
merged 6 commits into from
May 5, 2015

Conversation

dairiki
Copy link
Contributor

@dairiki dairiki commented May 2, 2015

Lying in bed last night, it came to me that a better way to implement the goals of PR #246 was simply to disable the upload, upload-submit, and add_file views on contexts for which File resources are not sdi-addable. (PR #246 just hides the Upload tab, the views are still accessible, even though invoking these views would not be appropriate.)

This PR adds a new view predicate which checks the context to see whether a specific content type is sdi-addable. This predicate is then used to protect the previously mentioned views. (I'm not sure that the name of the predicate, sdi_addable, is the best choice. Suggestions welcome.)

I've also renamed the new function content_type_addable, introduced in #246, to is_sdi_addable to more closely match the name of the view predicate.

@cguardia
Copy link
Member

cguardia commented May 4, 2015

Hey, thanks for taking the time to improve this code. About the predicate name, how about tab_addable_content? That would be more in line with the current names for other management view predicates.

Other than that nit, this looks good to me.

@dairiki
Copy link
Contributor Author

dairiki commented May 4, 2015

I’m not sure about the "tab_" in tab_addable_content. The Add UI element is a dropdown button, not a tab. Maybe sdi_addable_content? Or just addable_content?

While we are working out API naming nits: where should the is_sdi_addable predicate function be located? I have initially put it in substanced.folder.util, but now I'm thinking a better location is one level up in substanced.folder (a la substanced.property.is_propertied, substanced.catalog.is_catalogable, and substanced.workflow.is_workflowed.)

@cguardia
Copy link
Member

cguardia commented May 4, 2015

My thinking was that this is like a specialized case of the tab_condition predicate, but I see your point. My vote at this time would be for addable_content.

I agree that substanced.folder might be a better location for the predicate.

cguardia added a commit that referenced this pull request May 5, 2015
Improvements to PR #246 (hiding the "Upload" tab)
@cguardia cguardia merged commit da42331 into Pylons:master May 5, 2015
@cguardia
Copy link
Member

cguardia commented May 5, 2015

Thanks!

@dairiki
Copy link
Contributor Author

dairiki commented May 5, 2015

No problem. Thank you!

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.

2 participants