[v3.4] 494 import equipment #897
Conversation
…g it together" This reverts commit a5feecc.
Added :sort_order as an import parameter for categories; updated the relevant views (import and imported) as well as the import library and validation function.
added spec file for the CsvImport module and got working tests for the various module methods of EquipmentImport. Also added pending tests for functionality of both modules.
Conflicts: spec/spec_helper.rb
|
Ok, I believe that this is ready to go. It should allow for the importing of categories, equipment models, and equipment objects using appropriately formatted CSVs, with selective overwriting of the first two. The instructions on the import page are modelled after the user import page; this is due for an overwrite (see #520) but should suffice for now. As long as this is behaving as expected and is restricted to admin users, I think we can pull it in. |
| @@ -26,6 +26,9 @@ | |||
| get '/import_users/import' => 'import_users#import_page', :as => :csv_import_page | |||
| post '/import_users/imported' => 'import_users#import', :as => :csv_imported | |||
|
|
|||
| get '/import_equipment/import' => 'import_equipment#import_page', :as => :equip_import_page | |||
| post '/import_equipment/imported' => 'import_equipment#import', :as => :equip_imported | |||
There was a problem hiding this comment.
huh. I used a really questionable and slightly confusing naming convention when writing the import users routes two years ago. You adapted the structure to import equipment (import/import_page and imported/import), which is fine so long as we're consistent, but now I'm having second thoughts that what I wrote was really the best.
the imported route does the actual import_ing_, but it does render the successful-import/"imported" page at the end, so, I guess there was a logic.. :P
There was a problem hiding this comment.
Yes, this was very confusing when I was originally looking at it but I was basically copying the import users functionality so I stuck with it :-P.
|
This code looks right; I haven't started up a VM though to test. I'll try to remember to run tests on this tonight after work... I think ideally when implementing this for v4, we should try to abstract this to a generalized Import controller, and then shunt data to specialized or general methods in the model, and make sure we have thorough specs. :) |
|
This all works on the supplied data sets (in the example documentation), and correctly indicates why import fails e.g. if you try to import a model belonging to a category which doesn't exist in the db. It was really clever processing the cats first, and then the models, and finally the objects/items, as this makes it possible to import all three at the same time with no prior dependency on preexisting categories, etc. I like it! Ideally this should have specs, but it's almost a verbatim copy from the Import Users files (I noticed the exclamation marks and nomenclature are the same :P), and for v4 I think these should be abstracted to generalized methods as much as possible, so maybe we can wait on these specs in order to get this backported ASAP before the start of the school year. |
|
ability.rb correctly limits this to admins, going in for the merge... |
resolves #494 as requested by CSSI; this will need to be merged into
masteras well. I'm going to clean up the branch and remove the testing since it was barely set up; the import was working fine so in the interest of getting the feature implemented I think we should skip testing for now. Additionally, we probably want to refactor all of the import code at some point as well. I'll also verify that all of the changes to the category, equipment model, and equipment object models have been taken into account.