Create demo dataset add related page

[aron.carroll]

Discussion:

​https://okfn.basecamphq.com/projects/9558659-demo-ckan-front-end/posts/62821386/comments

Implementation:

​http://s031.okserver.org:2375/dataset/adur_district_spending/related/new

Needs a new endpoint at /dataset/DATASET/related/new this should have a form containing the following fields.

• Title (required)
• Type of item (Application|Visualisation)
• Description
• URL (required)
• Image URL

When submitted if an item is created it should redirect (303) back to /dataset/DATASET/related with a flash message saying "Related item was successfully created".

If failed to create it should redirect (303) back to the form and populate the error messages, see add dataset for examples (​https://github.com/okfn/ckan/blob/feature-2375-demo-theme/ckan/templates/package/new_package_form.html)

[ross]

Yup. Will look at this Thurs when I'm back in.

[toby]

@Ross you will need to do this based off the feature-2375-demo-theme branch - you can use that branch or one split from it.

new templates want to be in templates/... the old templates live in templates_legacy if you need to copy them across that's fine

talk to aron or I if you need to know anything

[ross]

Okay, the 303 on validation failure seems a little excessive though given that we're posting to that URL. Is a 200 okay?

[ross]

Am working in feature-2445-add-related-new

[toby]

@ross

the validation error seems correct to use 200

for the created the 303 seems ok but i'd just use pylons redirect myself

Also did aron say we want this as a javascript free add item form

[ross]

He didn't say but I assumed yes given current impl uses dodgy JS->API.

[aron.carroll]

I generally take the approach that a form submitted via POST should always redirect to a page. This prevents accidental user resubmissions when hitting refresh as well as the annoying popup saying "are you sure you want to re-submit".

​http://en.wikipedia.org/wiki/Post/Redirect/Get

Looking at the rest of CKAN it doesn't do this so I'll leave it up to you.

[ross]

redirects are fine, of course, but redirect on validation failure seems painful ?

Also templates in templates_legacy?

[toby]

@aron I'm fine with the redirect thinking it's just the specific 303 rather than just whatever pylons does.

@ross, templates_legacy are there for ckan users who have their own templates as we are completely rewriting all the existing templates

there is a lovely .ini setting to switch between the new and legacy modes

[ross]

I *think* this is done in feature-2445-add-related-new although I may have missed something, if you could take a look and let me know if it is what you wanted.

[aron.carroll]

Thanks Ross this is exactly what I wanted.

Just wondering about the use of the c.form variable to insert the form into the page. I know we do this with the dataset and groups to support legacy templates, do we need to do this going forward? I'd rather each action just rendered one template and leaves it up to that to render the rest.

Toby, there's a couple of TODO statements in the code where the breadcrumbs need fixing. I think the templates just need access to the package dict.

​https://github.com/okfn/ckan/blob/feature-2375-demo-theme/ckan/templates/related/pages/form_page.html#L6 ​https://github.com/okfn/ckan/blob/feature-2375-demo-theme/ckan/templates/related/edit.html#L5

Also if you generate an error message on one of the forms (leave the title blank) the error string comes out wrapped in quotes. eg. u'title is missing'

Also might be of interest, I've converted the templates to use Jinja, and this is a good example of the conversion.

​https://github.com/okfn/ckan/commit/d779addf6e48be67695b35c031393e2d90d8c22f

[aron.carroll]

Oh forgot to mention I've merged the code into feature-2375-demo-theme. We should delete the feature-2445-add-related-new branch.

[toby]

@aron,

this is as good a place as anywhere to answer your question about the c.form thing.

a) some of this we are sort of stuck with. datasets for example can be of different types and have different forms for different datasets. so they need to be rendered separately. I have no idea why or how much this is used but it exists as an option to complicate everything, not much we can do about it.

b) I think trying to do pages as a single rendering is not going to be easy because of the way ckan has been created maybe we can change this over time. One major issue for me is that ckan does lots of crazy stuff for some reasons but these reasons are not apparent from the code. Maybe there is some documentation somewhere but I'm not convinced.

anyhow we should maybe close this ticket now. You've added some stuff in the comment for me to do maybe that should be added as a new ticket?

[toby]

closing as todos moved to other tickets and also completed