3022 IDatasetForm fixes #233
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Routes have the form `dataset-type_action`, eg `harvest_source_read` or `mydatasettype_edit`. This allows to build the url with url_for or nav_named_link from the controllers or templates.
This makes search queries much easier
When visiting the search page of a custom dataset type (eg /<my-type>), only datasets from this type are shown. On the main search page (/dataset), only standard datasets are shown, unless the config option `ckan.search.show_all_types` is set to True.
Also don't pass id on new action
This allows custom URLs like the ones from dataset types. Private function _url_with_params was duplicated on helpers.
This allows the Pager to generate URLs that point to the relevant dataset type page, and not /dataset.
This allows dataset type pages to show their own facets and have links pointing to the right direction.
| @@ -273,7 +273,8 @@ def package_dictize(pkg, context): | |||
| result_dict['isopen'] = pkg.isopen if isinstance(pkg.isopen,bool) else pkg.isopen() | |||
|
|
|||
| # type | |||
| result_dict['type']= pkg.type | |||
| # if null assign the default value to make searching easier | |||
| result_dict['type']= pkg.type if pkg.type else u'dataset' | |||
There was a problem hiding this comment.
how about this result_dict['type'] = pkg.type or u'dataset'
|
Apart from the minor fixes this looks good to me. Thanks for removing that duplicate code in lib/helpers. I'll merge if you make the changes or tell me why they are a bad idea ;p |
| else: | ||
| # Unless changed via config options, don't show non standard | ||
| # dataset types on the default search page | ||
| if not asbool(config.get('ckan.search.show_all_types','False')): |
There was a problem hiding this comment.
pep8 if not asbool(config.get('ckan.search.show_all_types', 'False')): space after comma
ghost
assigned 
|
@tobes I fixed the issues you mentioned, let me know if you find something else |
tobes
added a commit
that referenced
this pull request
Jan 9, 2013
|
I had to rename your migration file due to a conflict. now merged |
amercader
referenced
this pull request
Feb 14, 2013
This is because package_type should not influence the page that is redirected to. The previous implementation had the problem that package_type is `package` by default which broke the redirection after editing the slug of a dataset.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds fixes and new functionality to properly support dataset types on 2.0.
Some highlights:
type_action(eg my-type_new, my-type_edit, ...) to allow generating the urls from extensions, etc. as a call to url_for with control, action etc did not generate a url for a non deafult typeAs reference, this branch of ckanext-harvest is using these changes to handle dataset types for harvest sources, and has custom templates, etc:
https://github.com/okfn/ckanext-harvest/tree/2.0-dataset-sources