[ckan-dev] convert_from_extras and extras_as_string

[Sean Hammond]

Hey, what shall we do about this ticket?

https://github.com/okfn/ckan/issues/376

convert_from_extras is a little confusing. For example lets say I
have an IDatasetForm plugin that adds a simple string as a custom field.
The user types foobar into the field in the dataset, the dataset read
template will get a top-level key in c.pkg_dict: 'my_custom_field': u'foobar'.

So far, so good.

But if a user does a package_show via the API, the new top-level key in
the package dict looks like this: "my_custom_field": "\"foobar\""

It seems to come down to these two lines that do an extra, apparently
unnecessary, json.dumps() on the value if the value is a string and
extras_as_string is not in the context:

https://github.com/okfn/ckan/blob/master/ckan/lib/dictization/model_dictize.py#L89

https://github.com/okfn/ckan/blob/master/ckan/lib/dictization/model_dictize.py#L103

When viewing a package in the web frontend extras_as_string is in the
context, when viewing it over the api it's not, hence the differrent
values.

If I just delete those two lines (so now the extras_as_string behaviour
becomes the default) and then fix a few other lines that would get json
decoding errors as a result, then it works. I can enter foobar into my
custom field and get foobar back in the web ui and foobar back from the
api with no extra quotes. The tests pass too, apart from 23 failing
asserts that look like they can probably be fixed by simple updates to
the tests.

https://github.com/okfn/ckan/pull/383

But I've got a feeling this isn't the right thing to do as
extras_as_string was explicitly added as an option and not the default
behaviour for a reason. Do we actually want to let people store
non-string json objects as values using convert_to/from_extras?

I think we either need to make this change, or if there's a good reason
to keep the current behaviour then keep it but document it clearly
somewhere (maybe convert_to/from_extras need docstrings, and we can also
add an example usage into the example_idatasetform extension that I'm
working on) because it's confusing and looks like a bug.