-
Notifications
You must be signed in to change notification settings - Fork 43
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
Save and transition editions in one go #251
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
www.agileplannerapp.com/boards/173808/cards/5629 This partial contains many fields which are common across formats, hence a more descriptive name.
www.agileplannerapp.com/boards/173808/cards/5629 Editors have reported issues with the existing pattern of saving the edition form via AJAX before doing a transition. Due to these multiple submissions we suspect that on rare occations transition happens but edition doesn't get saved. Editors assume that when a transition happens, the edition form changes are saved, and don't think of it as a 2 step process. This change ensures that transition is performed using attributes within the edition form, which ensures that we save the edition and perform the transition in a single request.
www.agileplannerapp.com/boards/173808/cards/5680 couldn't find usages for that field in views.
www.agileplannerapp.com/boards/173808/cards/5629 Some scenarios need just the activity to be performed without saving edition changes. Hence accommodating that within the activity helper.
www.agileplannerapp.com/boards/173808/cards/5629 The earlier text was appropriate as button text, but not as modal title or modal button text. Also, adding a useful bit of help text to tell editors that their changes are not lost when they make transitions.
www.agileplannerapp.com/boards/173808/cards/5629 also, saving the states and their labels in the Edition model so they can be referred from the helper as well as controller. modeling events and its human readable format as a Hash, because that's more appropriate.
www.agileplannerapp.com/boards/173808/cards/5629 ... with the edition form. it needs to be submitted to editions#progress because submitting the edition will return error: Edition scheduled for publishing can't be edited. also, this action is triggered from a view where edition fields are locked, so we don't want to save edition changes while transitioning.
www.agileplannerapp.com/boards/173808/cards/5629 This was required earlier to save an Edition when making an edition transition. Now that transition parameters are submitted as edition fields, we do not need to save the edition form via AJAX before transitioning to another state.
www.agileplannerapp.com/boards/173808/cards/5629 This allows us to submit editions without using JS.
www.agileplannerapp.com/boards/173808/cards/5629 ... so that clicking them doesn't submit the edition form, instead opens a modal
www.agileplannerapp.com/boards/173808/cards/5629 Buttons with type=submit are being used within forms to submit the edition transition and edition form. Rectifying this issue.
www.agileplannerapp.com/boards/173808/cards/5629 ... to keep the naming consistent all across.
edition_form.trigger('submit'); | ||
} | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very happy to say goodbye to this JS 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happier to get rid of the 'fake' Save button.
www.agileplannerapp.com/boards/173808/cards/5629 ... and activity dialog copy.
fe2f633
to
4864e45
Compare
alext
added a commit
that referenced
this pull request
Sep 5, 2014
Save and transition editions in one go
vinayvinay
pushed a commit
that referenced
this pull request
Sep 10, 2014
www.agileplannerapp.com/boards/173808/cards/5629 ... with edition form. these were left out earlier, causing review activities to not save edition changes. part of a larger change: #251
vinayvinay
pushed a commit
that referenced
this pull request
Sep 10, 2014
www.agileplannerapp.com/boards/173808/cards/5629 ... with edition form. these were left out earlier, causing review activities to not save edition changes. part of a larger change: #251
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
www.agileplannerapp.com/boards/173808/cards/5629
we save the edition form via an AJAX call before performing any transition on editions. this proved to be flaky on some occasions where editors could see a transition happen, but without edition changes.
this is an attempt to achieve the same behaviour in a more robust way. instead of relying on JS to submit edition and transition forms, we're using the default form submit.
we've achieved this by inlining the activity fields like (comments, fact check email addresses) with the edition form fields, so that they get submitted along-with the edition, in a single request.
this required us to remove the fake 'Save' button that was triggering JS to submit an edition, and use the form's submit button instead,
input[type=submit]
on the edition form.also includes a decent amount of clean-up and refactoring: