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
Background bulk add #262
Background bulk add #262
Commits on Apr 29, 2014
-
-
Validate Mapping http_status is in array
The constant was unused. Presumably it was once. I stringified the entries because we store them as strings in the database.
-
Add validations to MappingsBatch
These are mostly duplicates of things on Mapping, because MappingsBatch is a pseudo-mapping.
-
-
-
-
Had to bump mlanett-redis-lock because it had a conflicting dependency on redis.
-
-
MappingsController uses background worker
There is no longer any feedback to the user, which is lame. I'll add that back in next.
-
-
-
Record state for each MappingsBatchEntry
This will let us display progress to the user.
-
-
-
-
-
We don't want ephemeral data hanging around forever.
-
Sidekiq will perform 25 retries over approximately 21 days using an exponential backoff algorithm. We probably don't want mappings being changed days afterward so let's never retry. This is fairly drastic - we could have 6 retries which would be attempted over 6 minutes, but the MappingsBatch state field behaviour is incompatible with retries - it would mark them as failed after the first attempt. If we wanted to add retries back in, we'd need to change MappingsBatch's model of state to cope with it.
-
-
-
Fix bug in detecting hosts for another site
It wasn't deduplicating the hosts list, so when it compared with the (unique) list of hosts in the database, it gave the wrong answer.
-
Good UX, now that we have large batches of mappings.
-
Remove some redundant hidden fields
These values are already in the database
-
Only show a preview of the batch
Showing the whole of very large batches would cause very high view rendering times and hence timeout, plus it would have been unusable.
-
This makes much more sense when adding (possibly updating) a single mapping or a few mappings, which is definitely the normal case. This reinstates the modal confirmation dialogue for small batches. The specs for mappings_batch_outcome_presenter.rb were cribbed from the specs for BulkAdder. Reverted features changed earlier in this branch which run small batches to look for the modal, rather than the "x of y processed" flash message used for large batches. 20 was chosen as the upper limit for a "small" batch because it should be easily processable within the 15 second timeout we have for admin apps, but is just above the average batch size according to Google Analytics.
-
Create modal showing bulk add progress
Cribbed from @fofr's prototype. Still needs: * some tidying up * the ability to dismiss it * not to be shown after it is dismissed/the page is refreshed * Spinner images from http://www.mytreedb.com/view_blog/a-33-high_quality_ajax_loader_images.h tml * Shows an updating count and progress bar * Doesn’t gracefully handle the adding/updating situation, needs different copy in that situation
-
Move bulk progress script into a module
* Switch from inline JS to a module * Pull URL from data attribute * Switch from setInterval to a timeout that gets set once the previous request is complete * Reduce scope of jQuery selectors to just the element we’re looking at * Create a spec Not yet handling errors, updating aria values or handling completion
-
Clean up background bulk add modal styles
* Take out inline styles * Use default inherited styles where possible * Make some specific styles for this modal
-
Add an ok button to bulk add modal
* Refactor modal CSS that deals with absolutely positioned close buttons * Make the modal-saved-mappings CSS re-usable, and remove the specific font-size: 80% rule on the glyphicon
-
Show a modal success state when all mappings added
* When done matches total, stop making requests, show a success state and shut down the module * Use jasmine clock to avoid having to wait 1 second for each timeout test
-
-
-
-
-
-
-
Actually validate state on MappingsBatch
The `validate` method is for validating with custom methods, not built-in or custom validator classes. Unfortunately, Rails doesn't complain that a method doesn't exist for the validation you've defined.
-
Bulk add validation errors as per master
The messages on master performed well in user research, whereas the messages generated by ActiveModel::Errors#full_messages did not. This change means that, for example, the #full_messages method returns messages like: "paths Enter at least one valid path" hence the change to `batch.errors.values.flatten(1)` in the view.
-
-
Remove redundant parameter to validate
`validate` (as opposed to `validates`) doesn't take a field name argument, so doing so was redundant.
-
Find existing mappings on an indexed field
This should be much faster, especially on large sites.
-
Only show background modal once
The flash message should continue to show on subsequent page loads to give feedback.
-
-
Update the flash message with javascript
In addition to displaying the modal (until dismissed), let's update the flash message with AJAX. A separate flash key was needed to separate it from the normal flash messages so that the required markup could be included without over-complicating the existing flash message code.