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

Use the package_search API for the count of datasets #298

Merged
merged 1 commit into from
Nov 2, 2017

Conversation

metaodi
Copy link
Member

@metaodi metaodi commented Oct 26, 2017

Rather than relying on the 'current' flag on the HarvestObject it's more
accuarte to use the package_search API to display the current number of
datasets, as this is actually the number of datasets displayed in the
list.
If for whatever reason the 'current' flag is not up-to-date, this leads
to a mismatch between the displayed number of datasets and the acutal
number of datasets returned by CKAN.

@metaodi metaodi changed the title WIP: Use the package_search API for the count of datasets Use the package_search API for the count of datasets Oct 26, 2017
@metaodi metaodi requested a review from amercader October 26, 2017 14:43
@metaodi
Copy link
Member Author

metaodi commented Oct 31, 2017

@amercader what do you think? I'm unsure where/if the total_dataset calculations are used somewhere else. Alternatively to this I could create a new template helper function to display the number on the harvest source page and leave the API untouched.

@amercader
Copy link
Member

That sounds good @metaodi.
Can you explain the changes in the tests? Shouldn't total_datasets remain the same with the changes? (it's fine if not, just want to double check)

@metaodi metaodi force-pushed the use-api-to-get-dataset-count branch from b2b642d to 0cbb941 Compare October 31, 2017 14:59
@metaodi metaodi force-pushed the use-api-to-get-dataset-count branch from 0cbb941 to 9bfd586 Compare November 1, 2017 07:24
@metaodi
Copy link
Member Author

metaodi commented Nov 1, 2017

@amercader I now changed the template to use a template helper and keep the original total_datasets field as-is.

@amercader amercader merged commit c166144 into master Nov 2, 2017
@metaodi metaodi deleted the use-api-to-get-dataset-count branch November 2, 2017 13:01
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