Feature/package approve #3
Conversation
- pass the context to get popular and recently viewed datasets - update test to pass user name to context - update layout1 to pass username to helper - new test case to cehck only approved datasets are listed
- get rid of approved - correct role for readers
ckanext/ed/actions.py
Outdated
|
|
||
|
|
||
| @toolkit.side_effect_free | ||
| def package_search(context, data_dict): |
There was a problem hiding this comment.
This should be done in the IPackageController.before_search hook. In general is best to avoid overriding actions if possible
There was a problem hiding this comment.
We need to rethink the whole way pending datasets will be handled on the search. Probably using IPermissionLabels, as we will need to search for pending datasets at some point. But let's go with this for now.
There was a problem hiding this comment.
@amercader so is this fine? Or refactor to use before_search?
There was a problem hiding this comment.
Yes, please. Use before_search
ckanext/ed/helpers.py
Outdated
| @@ -24,26 +24,31 @@ def get_groups(): | |||
| return groups | |||
|
|
|||
|
|
|||
| def get_recently_updated_datasets(limit=5): | |||
| def get_recently_updated_datasets(limit=5, user=None): | |||
There was a problem hiding this comment.
Why was this needed? FYI, if you don't explicitly provide a user in the context CKAN will use the one logged in.
There was a problem hiding this comment.
Forget to mention in PR description. Did this cause we probably don't want pending datasets to appear in recently updated and the most popular dataset. Both get_recently_updated_datasets and get_most_popular_datasets use package_show that awaits user in context to check if one can views dataset.
There was a problem hiding this comment.
FYI, if you don't explicitly provide a user in the context CKAN will use the one logged in.
@amercader hmm... now I remember, the main reason was, cause tests started failing I think.
There was a problem hiding this comment.
The avoiding of pending datasets coming up should he handled at the search level. The implementation of get_recently_updated_datasets is wrong as you don't need to call package_show, you have all the info you need in the results returned by package_search.
Anyway if you relied on the check on package_show you would get a NotFound exception which is probably not what you want.
There was a problem hiding this comment.
@zelima can you revisit this and see if the change is actually needed?
get_validatorsfromIValidators) that unless the user is an admin/sysadmin sets the value toapproval_pending.statefield to the dataset schema, adding the previous validator validator to it. Hide it from the form for now (form_snippet: False)templates/package/new_package_form.html) and show "Request Dataset" instead of "Create Dataset".is_admin(user)helper function to determine editor -> If user is not admin + can create package means user is editorapproval_pendingshould not be displayed in the search results (/dataset or API)