[ckan-changes] [okfn/ckan] 10afee: [#362][s]: remove controller, routing and test cod...

Wed Feb 20 16:14:39 UTC 2013

  Branch: refs/heads/release-v2.0
  Home:   https://github.com/okfn/ckan
  Commit: 10afeece4cde4364e5b446a74a6039d8726648b0
      https://github.com/okfn/ckan/commit/10afeece4cde4364e5b446a74a6039d8726648b0
  Author: Rufus Pollock <rufus.pollock at okfn.org>
  Date:   2013-02-20 (Wed, 20 Feb 2013)

  Changed paths:
    M ckan/config/routing.py
    R ckan/controllers/datastore.py
    R ckan/tests/functional/test_datastore.py

  Log Message:
  -----------
  [#362][s]: remove controller, routing and test code related to old (ES) datastore.

Note, have not removed (and will not remove) references to old datastore in:

* templates_legacy (assume these will get removed anyway)
* i18n (assume this will get rebuilt prior to release)

  Commit: 34981d2b9b87be43e0350fdf75492323e2ac872f
      https://github.com/okfn/ckan/commit/34981d2b9b87be43e0350fdf75492323e2ac872f
  Author: Rufus Pollock <rufus.pollock at okfn.org>
  Date:   2013-02-20 (Wed, 20 Feb 2013)

  Changed paths:
    M ckan/controllers/package.py
    R ckan/templates/package/resource_api_data.html
    M ckan/templates/package/snippets/data_api_button.html

  Log Message:
  -----------
  [#362,bugfix][s]: remove remaining reference to routing for old datastore (by removing api_data method in package controller).

The ultimate change was reasonably substantial - removal of an entire method on
a controller and associated template. It began simply with the need to
remove/update reference this routing call in api_data method on package
controller

    url = h.url_for('datastore_read', id=id, qualified=True)

I spent about half an hour trying to understand if this method was used at all.

I ultimately determined that we had a setup where clicking on the Data API
button in the resource view had 2 possible effect routes:

* Via href={url} => package.py/api_data method =>
  template/package/resource_api_data.html =>
  template/ajax_snippets/api_info.html
* Via JS intercept of click => api/snippets controller =>
  template/ajax_snippets/api_info.html

But there was a subtle difference: the first route (which is *not* normally
followed since JS is usally enabled) still uses the old datastore URL (this is
why I'd started this investigation!).

(Aside: this was hard to debug through the interface as JS is enabled and I
note we do not have resource view tests so the non AJAX route is not tested).

I first tried to correct this by setting the URL to the correct datastore API
but then discovered other parameters were missing - it seems the signature of
the call to ajax_snippets template has changed but that the relevant code in
package/resource_data_api.html has not been updated:

    {% snippet 'ajax_snippets/api_info.html', datastore_root_url=datastore_root_url, embedded=true %}

Rather than continue fixing I decided at this point to remove this non-AJAX
fallback code in its entirety.

Comment: this was a subtle bug that has crept in because there are now 2 ways to do
something and one of these routes (the non-AJAX route) is almost never used ...

I wonder whether it is necessary always to have both options (AJAX and
non-AJAX) as it likely to multiply code complexity and, hence, these sort of bugs.

  Commit: cf0c4fe351d7649142663c96de469bed9a5e8f1c
      https://github.com/okfn/ckan/commit/cf0c4fe351d7649142663c96de469bed9a5e8f1c
  Author: tobes <toby.junk at gmail.com>
  Date:   2013-02-20 (Wed, 20 Feb 2013)

  Changed paths:
    M ckan/templates/package/snippets/data_api_button.html

  Log Message:
  -----------
  [#362] Fix api info for non-js browsing

  Commit: 52be376ede9a6ab0ee0775c4d042a057b5e2babc
      https://github.com/okfn/ckan/commit/52be376ede9a6ab0ee0775c4d042a057b5e2babc
  Author: tobes <toby.junk at gmail.com>
  Date:   2013-02-20 (Wed, 20 Feb 2013)

  Changed paths:
    M ckan/templates/package/snippets/data_api_button.html

  Log Message:
  -----------
  [#362] Fix broken test code

  Commit: 30e8d8046559a5e7967efb510c225f3f761da9a3
      https://github.com/okfn/ckan/commit/30e8d8046559a5e7967efb510c225f3f761da9a3
  Author: amercader <amercadero at gmail.com>
  Date:   2013-02-20 (Wed, 20 Feb 2013)

  Changed paths:
    M ckan/templates/package/snippets/data_api_button.html

  Log Message:
  -----------
  [#362] Fix datastore active check and misleading message

Compare: https://github.com/okfn/ckan/compare/07ec688893ba...30e8d8046559