Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure duplicate responses are not created by chained responses to the same question #328

Closed
rsutphin opened this issue May 22, 2012 · 2 comments
Assignees

Comments

@rsutphin
Copy link
Contributor

Since surveyor posts responses asynchronously, it's possible for several changes in a row to result in creating separate response records instead of creating and then updating the same record. E.g., say you have a pick-one question with a free-entry answer and a coded answer. You:

  • Select the coded answer
  • Change to the free-entry answer
  • Enter a value for the free-entry answer and tab away from (blur) the free-entry answer

Surveyor will make an AJAX call after each of those bullets. If all of them happen before the first AJAX call returns with the response ID, the server side code will create separate response records for each. This confuses analysis (which response is the real one?) since it is legal in general for a question to have multiple responses (either when it is a repeated question or a pick-many question). There is also code elsewhere in Surveyor (e.g., #297) which assumes that single-answer questions will only have one response per response set.

Proposed solution:

  1. Surveyor now associates a UUID with each response (the api_id column). Pregenerate UUIDs for each possible response when rendering the UI. Dynamically added responses (i.e., for repeaters) will generate the UUIDs on the client side.

  2. Submit the UUID with the user's response. Only create a new response record if there isn't one already with the UUID. Use a uniqueness constraint on responses.api_id to ensure that you don't get duplicates. If the constraint is violated during save, retry as an update.

Comments appreciated.

@rsutphin
Copy link
Contributor Author

@ybushmanova notes that the date picker in recent versions of surveyor is particularly prone to creating duplicate responses.

@ghost ghost assigned rsutphin Jun 15, 2012
@rsutphin
Copy link
Contributor Author

Another vulnerable situation in the current master is a repeater with a single free-text value question. A cucumber @javascript feature which:

  • Enters text for the first instance of the repeating question
  • Clicks the + add row button
  • Enters text for a second repetition
  • Blurs the second rep's field

... will consistently create three responses instead of two, duplicating the first one.

rsutphin added a commit that referenced this issue Jun 25, 2012
This is a longstanding Surveyor behavior, but the existing features (including the
selenium ones) mostly did not exercise it.

Some of the features are `@wip`, either because they are incomplete (I can't figure out
a good way to accurately simulate a user's use of a jQuery UI slider) or because they create duplicates per #328 and so fail.
rsutphin added a commit that referenced this issue Jul 23, 2012
rsutphin added a commit that referenced this issue Jul 23, 2012
rsutphin added a commit that referenced this issue Jul 23, 2012
It was added (AFAIK) only as a duplicate prevention bodge. With #328, it
should no longer be necessary.
rsutphin added a commit that referenced this issue Jul 23, 2012
rsutphin added a commit that referenced this issue Jul 23, 2012
Putting the input and the expected output side by side for each case clarifies
and documents the current behavior.
rsutphin added a commit that referenced this issue Jul 23, 2012
The solution for #328 relies on the database being the final arbiter of which
request gets to create the Response record for a given potential response.
Potential responses are identified by a UUID which is persisted by the first
request as the api_id.
rsutphin added a commit that referenced this issue Jul 23, 2012
This method relies on receiving api_ids for both new and existing responses to
eliminate the chance of duplicates. Still incomplete here: demonstrating what
exception is thrown when there's a race between to different attempts to
create the same response. Still incomplete everywhere else:
generating/rendering api_ids for responses in the UI.
rsutphin added a commit that referenced this issue Jul 23, 2012
There's a fair amount of behavior shared between async and normal PUTs. This
applies the specs for the common behaviors to both invocation modes. One of the
specs fails because it elicits a bug.

This commit also adds pending specs for new #328 behaviors to the shared
section.
rsutphin added a commit that referenced this issue Jul 23, 2012
This is the last conceptual part of #328. All the previously passing features
now pass again, as do some of the new duplicate-eliciting ones. There are still
some of those that don't pass, though. Needs more investigation.
rsutphin added a commit that referenced this issue Jul 23, 2012
This makes log archaelogy a bit more tractable for problems that arise from the
interaction of multiple tests.
rsutphin added a commit that referenced this issue Jul 23, 2012
This already works, so these are just new specs to verify.
rsutphin added a commit that referenced this issue Jul 23, 2012
As noted in env.rb, Firefox will not deliver focus/blur events when in the
OS background. This means that tests that rely on the change event being fired
for text inputs, e.g., will fail if Firefox is in the background.
rsutphin added a commit that referenced this issue Jul 23, 2012
rsutphin added a commit that referenced this issue Jul 23, 2012
See the comment on evict_capybara_session for reasoning.
rsutphin added a commit that referenced this issue Jul 24, 2012
These methods were exclusively used by the old implementation of
SurveyController#update.
rsutphin added a commit that referenced this issue Jul 24, 2012
In the extremely unlikely case that two update attempts both want to create
a response with the same API ID, one of them will fail with a unique constraint
violation on commit. This retry will allow the loser to get its updates applied.
rsutphin added a commit that referenced this issue Jul 24, 2012
@yoon yoon closed this as completed Aug 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants