| Issue 12: | product, variation and item models can be refactored. | |
| 1 person starred this issue and may be notified of changes. | Back to list |
Here comes a test about refactoring code using the unit provided in th earlier issue. It was made using substruct_rel_1-0-a2. product.rb refactored. * after_create callback was erased (not needed even for not yet saved products). * before_save was needed then, but was put in item.rb. * Deprecated tags= in flavor of tag_ids= (a placeholder tag_id= was left in place for now to not create an infinite looping). * Deprecated related_product_ids= if flavor of related_product_suggestion_names= that is a more descriptive name its not the same code it doesn't verify if the object is new or not, it appears to not be needed. item.rb refactored. * before_save was put here, product and variation inherit from item and needs it. * suggestion_name is an instance method not a class method, added a new comment box, (not very usefull). variation.rb refactored. * before_save was erased from here, its already in item.rb. * erased commented method name=, it wasnt beeing used of course. As you can see, controllers and views was not touched, and I could not make it give any errors, so it should make what it meant for and don't break anything. It will issue warnings about the deprecated methods and it will be usefull to change the views and controllers and see if something was forgotten. Take a look at the code (I didn't made a patch, I am uploading the full models) regards.
Feb 14, 2008
Hold the last change, its another basic attribute redefined to mean something else, changed, it mess with how validations behave, and I will not touch controllers and views nor simply fix it changing its meaning.
Feb 20, 2008
Just to let you know, the files on the first comment can already be proved that works and don't change the behavior of the system using tests.
Apr 8, 2008
Edmundo - how are the tests coming? Did I miss a patch where they were?
Apr 8, 2008
I didn't made them available yet as Im working on it and it will change all the time until be complete. As I said in IRC I can put a sample, I will compress the files in a package (tar.gz) and attach to the issue 8 (its not representable using svn diff).
Apr 8, 2008
This is just a sample too, it works, simplify some things, uses deprecation code and tests ( issue 8 ) can be used to prove that the state of the system doesn't brake. I don't have sure about formatting (spacing, tabs, blank lines, etc). Theres other places that are in worst conditions, (the order model that I was looking some days ago send e-mails for example). This issue is more a question about if these kind of things can be accepted (are wanted).
Apr 9, 2008
Rolled in the first set of patches with revision 75. Closing this now...I like the direction you're headed in with the refactoring + testing. Thanks again...
Status:
Fixed
Cc: subimage |
One more change in variation.rb # Display name...includes product name as well def name if self.product return "#{self.product.name} - #{self.attributes['name']}" else return self.attributes['name'] end end Doesn't make sense test if a variation has a product, I think it MUST have. At contrary this one whould be wrong and return a nil.images error sometimes: def images self.product.images end So changed to: # Display name...includes product name as well def name "#{self.product.name} - #{self.attributes['name']}" end Again, its not broken, it just isn't needed.909 bytes View Download