search

SEL-Columbia / formhub

Mobile Data Collection made easy.
http://formhub.org
BSD 2-Clause "Simplified" License
259 stars 163 forks source link

Add an "edit" button to the web-data-view #363

Closed prabhasp closed 12 years ago

prabhasp commented 12 years ago

that should (0) be per-row, and treat each row as an instance/parsedinstance/mongo-data-row (1) get the current instance's submission-xml as well as form-xml (2) create a new xml file that pre-fills the form-xml with submission-xml data (3) passes that xml file over to touchforms for editing and (4) makes sure that some entity (instanceID?) is carried over in the process so that when touchforms re-submits data back into formhub, we can know that this was an edit of a previous datapoint that was already in formhub.

again, this is a big one.

larryweya commented 12 years ago

@prabhasp @MartijnR can enketo be used for this, discussed it with @ukanga and we have the idea of using versioning to keep versions of the xmls and treat duplicates as edits.

MartijnR commented 12 years ago

enketo-dev.formhub.org now has the following scaffolding in place that can load a edit-form view with the instance. (the instance is visible in the source code at the end of the <head>, one var called jrDataStr (the original) and one var called jrDataStrToEdit (passed by Formhub). Nothing else is working yet (the data is not yet loaded into the form)).

To load this view, use the following 2-step process:

  1. use the exact same function as the one called with the Enter Web Form button, but this time look for the JSON response variable called 'edit_url' (instead of 'url').
  2. post the XML instance (named: instance) to this edit_url. This should load the form and the jrDataStrToEdit var in <head> should show the instance you posted.

How shall we redirect the user back to formhub after a successful submission of the edit data? A way would be for the formhub POST to included both the instance as well as a 'return_url' that enketo could redirect to upon successful submission, but I am not sure if that's the best way. The edit-form view should also be 'iframeable' if that is desired, in which case formhub could do the redirect perhaps?

prabhasp commented 12 years ago

On Mon, Oct 1, 2012 at 7:28 PM, Martijn van de Rijdt < notifications@github.com> wrote:

enketo-dev.formhub.org now has the following scaffolding in place that can load a edit-form view with the instance. (the instance is visible in the source code at the end of the , one var called jrDataStr (the original) and one var called jrDataStrToEdit (passed by Formhub). Nothing else is working yet (the data is not yet loaded into the form)).

To load this view, use the following 2-step process:

1.

use the exact same function as the one called with the Enter Web Form button, but this time look for the JSON response variable called 'edit_url' (instead of 'url'). 2.

post the XML instance to this edit_url. This should load the form and the jrDataStrToEdit var in should show the instance you posted.

How shall we redirect the user back to formhub after a successful submission of the edit data? A way would be for the formhub POST to included both the instance as well as a 'return_url' that enketo could redirect to upon successful submission, but I am not sure if that's the best way.

This sounds good, pending substantial reasons to not do so.

The edit-form view should also be 'iframeable' if that is desired, in which case formhub could do the redirect perhaps?

— Reply to this email directly or view it on GitHubhttps://github.com/modilabs/formhub/issues/363#issuecomment-9053966.

Prabhas Pokharel http://twitter.com/prabhasp US mobile: +1 347 948 7654 skype/facebook/whatever: prabhasp

MartijnR commented 12 years ago

Ok, the functionality is now, largely untested but, complete on the enketo side. It is deployed on enketo-dev.formhub.org.

I've added the return_url redirect-upon-successful-submission but can easily remove it if you'd prefer a different solution. Having the immediate redirect means some kind of 'succes!' message should be displayed in formhub or I could delay the re-direct for a few seconds and show a dialog (submission failures are of course handled by enketo).

I am not sure if formhub adds any nodes to the instances after they are received from a client and stored in the db. It seems that every xml form served by formhub already has the uuid node (or doesn't it?). If formhub does add nodes that are not in the <instance> of the xml form, please let me know where they are added (inside <data><formhub>......</formhub></data>?) because I'd have to make sure those new nodes are copied when loading formhub-POSTed instances in enketo.

ukanga commented 12 years ago

We have made some changes for this and dev.formhub.org has this changes at the moment. We seem to be getting a 404 with the error message "An error occurred during transformation or processing instances. ".

We are also making two http requests, one to enketo-dev.formhub.org and the other to the edit_url could we just make one request to enketo-dev.formhub.org and get back a redirect_url that has the data in place? Seems as you explained earlier we need to do a post to the edit_url which we are also to redirect to so as to do the editing, redirecting with post data seems tricky at the moment.

MartijnR commented 12 years ago

regarding the error, I just don't see any requests to .....enketo-dev.formhub.org/webform/edit ,so I'm sure that 2nd step is not working in the branch currently on dev.formhub.org. EDIT: If you do get that error again (which would be great actually) it is likely caused by the fact that that form is not hosted on a publicly accessible server (localhost e.g.?).

Regarding your proposal. Yeah, I see this would make sense. The only issue is that (apart from wanting enketo to stay very lightweight and never store data/forms, always pull everything form the source), we would have to implement some kind of server-side session storage that I haven't done before.

MartijnR commented 12 years ago

I noticed in Ukang'a curl tests that <meta><instanceID/></meta> is added to instances. I have assumed that those are the only nodes that formhub is adding to instances that are NOT in the original XML form.

I added the following to enketo's instance-loading feature with regards to dealing with instance nodes that do not exist in the form's default instance: Any direct children of [ROOT]->meta are cloned.

Please let me know if formhub is adding any nodes other than children of <meta>.

MartijnR commented 12 years ago

oh, and just in case, please use the full return url including 'http(s)://formhub.org'

larryweya commented 12 years ago

@MartijnR On the first request, we get this json

{u'url': u'http://x4ivn.enketo-dev.formhub.org/webform', u'reason': u'existing', u'edit_url': u'http://x4ivn-.enketo-dev.formhub.org/webform/edit', u'success': False}

Notice the edit_url has a dash, this is leads to the url being un-resolvable.

We then took out the dashes and did the post and got back a 404 with the message, edit's can "The edit view can only be launched in offline mode"

MartijnR commented 12 years ago

The url is correct and resolvable for me. The dash definitely needs to be there. I mean when I go to http://x4ivn-.enketo-dev.formhub.org/webform/edit in the browser, it resolves. The error message I'm seeing is 'No instance provided to edit and/or no return url provided to return to.', which is provided by the enketo controller and correct. EDIT: now http://x4ivn-0.enketo-dev.formhub.org/webform/edit

MartijnR commented 12 years ago

I'm at a loss here because this works fine too: curl -d "return_url=/bob/forms/tutorial/instance&instance=<?xml+version%3D%221.0%22+?><tutorial+id%3D%22tutorial%22><formhub><uuid>feaf64c2582a4184bedd57059a28d5cf</uuid></formhub><name/><age/><picture/><gps/><meta><instanceID>uuid:ef0f40d039a24103beecc13ae526b98a</instanceID></meta></tutorial>&format=json" http://x4ivn-.enketo-dev.formhub.org/webform/edit

MartijnR commented 12 years ago

Thanks guys! See https://github.com/MartijnR/enketo/pull/20#issuecomment-9177630. I merged it in the modilabs/enketo master branch and deployed it to enketo-dev.formhub.org.

Wil add the instance delete functionality and if you tell me which instance node I need to copy, will do that too.

dev.formhub.org is not up-to-date maybe? because the edit functionality is not working there

MartijnR commented 12 years ago

We may have to use <deprecatedID> when sending data to enketo for editing. Enketo would then have to create a new <instanceID>.

See: https://bitbucket.org/javarosa/javarosa/wiki/OpenRosaMetaDataSchema.

MartijnR commented 12 years ago

after our phone conversation, follow the discussion here: https://groups.google.com/forum/?fromgroups=#!topic/javarosa-developers/1JCOWALv3nI and https://groups.google.com/forum/?fromgroups=#!topic/opendatakit-developers/5BrxIPOLfos

ukanga commented 12 years ago

@MartijnR I have updated submission code to look for deprecatedID for edits, so edits should now only happen when we have a deprecatedID that matches an existing instanceID.

MartijnR commented 12 years ago

Great. Finished the work on the enketo side too I think and deployed it to enketo-dev. Is your code deployed somewhere?

ukanga commented 12 years ago

I think dev.formhub.org is on demo today, so you can give ukanga.com:8000 a try, just updated it.

MartijnR commented 12 years ago

Cool. I did and it seems to work!

MartijnR commented 12 years ago

At one stage I did manage to submit a duplicate somehow while editing. See http://ukanga.com:8000/bob/forms/data_types/instance#/6 number 1 and 2 were duplicates (but later on I edited 1 again).

I think it may be related to a double <meta><instanceID></meta> presence. However, I could not replicate a situation in which these nodes are added twice. I tested with a form that already had <meta><instanceID/></meta>and the doubling did not occur. Odd. It may have been caused by an old submission with two instanceIDs (remember the old issue formhub had that you resolved).

Probably best to do any testing on new submissions.

prabhasp commented 12 years ago

Sorry, I meant, whooooo hoooo, lets go ahead and merge!

On Fri, Oct 19, 2012 at 5:40 PM, Prabhas Pokharel < prabhas.pokharel@gmail.com> wrote:

Great, lets go ahead and merge then!

On Fri, Oct 19, 2012 at 5:02 PM, Martijn van de Rijdt < notifications@github.com> wrote:

Oh, yes, I did and it seems to work!

— Reply to this email directly or view it on GitHubhttps://github.com/modilabs/formhub/issues/363#issuecomment-9620346.

Prabhas Pokharel http://twitter.com/prabhasp US mobile: +1 347 948 7654 skype/facebook/whatever: prabhasp

Prabhas Pokharel http://twitter.com/prabhasp US mobile: +1 347 948 7654 skype/facebook/whatever: prabhasp

prabhasp commented 12 years ago

Great, lets go ahead and merge then!

On Fri, Oct 19, 2012 at 5:02 PM, Martijn van de Rijdt < notifications@github.com> wrote:

Oh, yes, I did and it seems to work!

— Reply to this email directly or view it on GitHubhttps://github.com/modilabs/formhub/issues/363#issuecomment-9620346.

Prabhas Pokharel http://twitter.com/prabhasp US mobile: +1 347 948 7654 skype/facebook/whatever: prabhasp

ukanga commented 12 years ago

I would ask for more tests, at the moment enketo-dev seems to be unresponsive on myside (firefox,chrome). Also the double instance might be a residual effect from previous testing. I have cleaned up the db at the moment so we should have a clean slate for tests.

MartijnR commented 12 years ago

Looking good now on dev.fh with some ad-hoc testing. @prabhasp this seems like a cool end-to-end test development opportunity, doesn't it?

MartijnR commented 12 years ago

Still issue. Fh has been observed to receive the edited data successfully, but enketo does not receive a response back before the 10sec timeout. Something we've noticed before and blamed the dev server for. To be continued.

MartijnR commented 12 years ago

Attempted to troubleshoot this by adjusting the cURL code in https://github.com/modilabs/enketo/blob/master/Code_Igniter/application/controllers/data.php (submission function) but could not test it properly ( I think somebody was working on dev.formhub.org)

MartijnR commented 12 years ago

@ukanga Now, definitely an issue with the <meta><instanceID/></meta> being added again in dev.formhub.org when it already exists. This now happens every time, I submit a new record for this form: http://dev.formhub.org/martijnr/forms/data_types and then try to edit it. See the source html (jrDataStrToEdit) in the <head> element of the enketo webform/edit page or look in the database in the instances table. One way to know for sure it happens in formhub is the fact that one of the <instanceID>s does not contain hyphens.

MartijnR commented 12 years ago

Any news?

I noticed that the duplicate instanceID nodes are no longer created.

However, there are still situations where the submission fails according to enketo, but in actual fact, it hasn't failed, because it shows up in formhub (under browse data). That's one mysterious issue. Any luck writing tests for this @ukanga?

The advantage of the issue above is that it highlights another issue. When a submission for whatever reason is submitted twice (and has the same instanceID), which I think is something we should expect to happen, both are counted as separate submissions (instead of only the latest submission).

ukanga commented 12 years ago

@MartijnR the double instanceID would happen on edits that were done earlier before we resolved the issue so I'll say that aspect has been resolved.

The issue about changes being applied on formhub side while enketo is saying offline, may have to do with the timeouts that enketo is using as oppossed formhub not giving response. From logs, all submissions have status code, and mostly now its a 201 and for duplicate submission is 202. My theory is that enketo did not wait long enough for the response, if it timeout it needs to resend. If the previous request was successful, then the submission will be marked as a duplicate. From https://github.com/modilabs/enketo/blob/master/Code_Igniter/application/controllers/data.php#L111, i see we have a 30 second timeout, hence the JS code should get a timeout and try to resend.

So far today I only got the offline message once, @katembu once, @larryweya none.

ukanga commented 12 years ago

deployed