[ckan-dev] Datapreview

Mon Sep 24 09:07:02 UTC 2012

Hi Toby,

very good suggestions. I can look into them. Now that we have three previews it makes sense to refactor since we can see where flexibility is necessary and where we can merge code.

On Sep 24, 2012, at 9:44 , Toby Dacre <toby.okfn at gmail.com> wrote:

> Dominik,
> 
> As discussed briefly on irc on friday we need to make the datapreviewer
> code tidier now that it is expanding in complexity to stop it becoming
> spagetti.  My suggestions are
> 
> a) move all the templates into their own directory.
> /templates/package/dataviewer seems good enough for me.  This includes the
> data_preview snippet.
> 
> b) create a new data_preview_base.html template something like this to use
> as the base for any preview pages to keep them simpler
> 
>    {% extends "base.html" %}
> 
>    {% block subtitle %}{{ h.dataset_display_name(c.package) }} - {{
> h.resource_display_name(c.resource }}{% endblock %}
> 
>    {# remove any scripts #}
>    {% block scripts %}
>    {% endblock %}
> 
>    {# remove any ckan styles #}
>    {% block styles %}
>    {% endblock %}
> 
>    {% block custom_styles %}
>    {% endblock %}
> 
> c) move to using a single route say
> /dataset/{pkg_id}/resource/{res_id}/preview{style} where style could be
> things like .recline, .json, .pdf etc we could use a regex in the route but
> that seems overkill.  This means that we just have a single controller
> action.
> 
> d) merge the separate actions into a single one so they share logic and we
> don't end up with situations like caring that a resource is in a package if
> viewed by recline but not is json data etc
> 
> 
> 
> Once that is achieved then I would next look at
> 
> e) cleaning the templates formatting etc {{ _('...') }} not {{_('...')}}
> etc minor but important and removing things like
>  {% resource 'base/main' %}
>  {% resource 'vendor/vendor' %}

I just added them to get the new js module logic. Would you rather put the needed files in a group only for the previews or create a shared group that in also included in page.html.

> 
> f) consolidating the javascript we could probably just have a single
> datapreview bundle
> g) look at the _parse_recline_state() stuff first to get a requirements for
> it and decide if some of these are not recline specific - also make a bit
> more robust
> h) look at stuff like do we need javascript to show the json? can we not
> just <code>{{ c.resource_json }}</code> maybe some js for syntax
> highlighting

As ar as I know, c.resource_json is not the actual resource but the metadata. 

> 
> What do you think of this as a plan going forward

+1

> Toby