My favorites | Sign in
Project Home Downloads Wiki Issues Source
READ-ONLY: This project has been archived. For more information see this post.
Search
for
  Advanced search   Search tips   Subscriptions
Issue 12: product, variation and item models can be refactored.
1 person starred this issue and may be notified of changes. Back to list
Status:  Fixed
Owner:  ----
Closed:  Apr 2008
Cc:  subim...@gmail.com


 
Reported by edmundo...@gmail.com, Feb 12, 2008
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.
item.rb
1.1 KB   View   Download
product.rb
4.8 KB   View   Download
variation.rb
992 bytes   View   Download
Feb 13, 2008
#1 edmundo...@gmail.com
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.
variation.rb
909 bytes   View   Download
Feb 14, 2008
#2 edmundo...@gmail.com
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
#3 edmundo...@gmail.com
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
Project Member #4 subim...@gmail.com
Edmundo - how are the tests coming? Did I miss a patch where they were?
Apr 8, 2008
#5 edmundo...@gmail.com
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
#6 edmundo...@gmail.com
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
Project Member #7 subim...@gmail.com
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

Powered by Google Project Hosting