Openfoodnetwork: Make OFN independent of spree_core

Created on 22 Feb 2020  路  2Comments  路  Source: openfoodfoundation/openfoodnetwork

What we should change and why (this is tech debt)

Quick summary of the reasons to do this:

  • A lot of OFN code is decorating Spree models (from my quick analysis we decorate around 50% of all spree models!), there would be a huge advantage in having the spree core merged with the decorations so we can increase quality, test coverage, etc.
  • On top of that spree 2.2, the next upgrade, includes a major refactoring of the spree adjustments logic, OFN has a lot of adaptations on top of the existing adjustments logic for example to include enterprise fees as adjustments. Adapting the OFN code to this new refactoring would be a nightmare. And not upgrading spree blocks us from the main issue which is upgrading rails.
  • One huge advantage will be to see all the unnecessary/unused complexity from spree go away, like promotions or stock movements/transfers.

In this epic we remove spree_core dependency and bring to OFN the spree models that we really need on our side. We will need to move a few 1000s of lines of code but the task is relatively simple to execute :tada:

We need to start with a detailed analysis of what decorators exist in OFN and what files from spree_core we will need to bring.

epic tech debt

Most helpful comment

This comment was where the initial list of 235 spree_core files that we needed were listed. These files were copied, merged or ignored.

These were the initial stats:
133 are live code classes:

  • 74 code classes will have to be copied to OFN
  • 40 code classes have to be merged with the existing decorators
  • 8 code classes will not be copied but need some adaptations in OFN
  • 11 code classes may not need to be copied

102 are test code classes (a lot of these will be ignored):

  • 42 specs (and spec helpers) will have to be copied to OFN (or ignored)
  • 22 specs have to merged with existing versions on OFN (or ignored)
  • 38 are the spree test factories that will have to be copied if they are used in OFN

This is the list of issues and PRs that were created gradually to move the files to OFN:

  • mailers #5686
  • calculators #5687
  • ControllerHelpers #5688
  • model payment and processing #5689
  • app/models/spree/stock #5690
  • model stock_item #5406

    • core/app/models/spree/order/checkout.rb #5824

  • core/app/models/spree/order_updater.rb and spec #5758
  • most files in core/lib/spree #5734
  • models country, zone, zone_member #5866
  • models stock_location and stock_movement #5867
  • models taxon and taxonomy #5868
  • models image and asset #5869
  • models app_config and preferences #5870
  • models shipping_category, ship_method, shipping_method_category, shipping_rates #5878
  • models payment_method, gateway and credit_card #5879
  • models tax_rate, adjustment, tax_category and calculator #5883
  • models variant, product and price, option_value and option_type #5885
  • models order, order_inventory, line_item, tokenized_permission, order_contents #5887
  • last bit of js and css #5928
  • basse_controller, base_helper and log entry #5924
  • Spree engine.rb #5929
  • models role and ability as well as user extensions #5925

    • helpers in core/lib/spree/testing_support #5941

  • spec factories #5943

And this is the list of files remaining:

Some DB default data:
core/db/default/spree/roles.rb COPY
core/db/default/spree/zones.rb MAYBE COPY

I am not sure we need these:
core/app/models/spree/product/scopes.rb MAYBE NO COPY - see product_fitlers, do we use this?
core/spec/models/spree/product/scopes_spec.rb see previous
core/app/models/spree/product_scope/scopes.rb MAYBE NO COPY same as above
core/app/models/spree/variant/scopes.rb MAYBE NO COPY same as above
core/spec/models/spree/variant/scopes_spec.rb see previous

All 2 comments

This comment was where the initial list of 235 spree_core files that we needed were listed. These files were copied, merged or ignored.

These were the initial stats:
133 are live code classes:

  • 74 code classes will have to be copied to OFN
  • 40 code classes have to be merged with the existing decorators
  • 8 code classes will not be copied but need some adaptations in OFN
  • 11 code classes may not need to be copied

102 are test code classes (a lot of these will be ignored):

  • 42 specs (and spec helpers) will have to be copied to OFN (or ignored)
  • 22 specs have to merged with existing versions on OFN (or ignored)
  • 38 are the spree test factories that will have to be copied if they are used in OFN

This is the list of issues and PRs that were created gradually to move the files to OFN:

  • mailers #5686
  • calculators #5687
  • ControllerHelpers #5688
  • model payment and processing #5689
  • app/models/spree/stock #5690
  • model stock_item #5406

    • core/app/models/spree/order/checkout.rb #5824

  • core/app/models/spree/order_updater.rb and spec #5758
  • most files in core/lib/spree #5734
  • models country, zone, zone_member #5866
  • models stock_location and stock_movement #5867
  • models taxon and taxonomy #5868
  • models image and asset #5869
  • models app_config and preferences #5870
  • models shipping_category, ship_method, shipping_method_category, shipping_rates #5878
  • models payment_method, gateway and credit_card #5879
  • models tax_rate, adjustment, tax_category and calculator #5883
  • models variant, product and price, option_value and option_type #5885
  • models order, order_inventory, line_item, tokenized_permission, order_contents #5887
  • last bit of js and css #5928
  • basse_controller, base_helper and log entry #5924
  • Spree engine.rb #5929
  • models role and ability as well as user extensions #5925

    • helpers in core/lib/spree/testing_support #5941

  • spec factories #5943

And this is the list of files remaining:

Some DB default data:
core/db/default/spree/roles.rb COPY
core/db/default/spree/zones.rb MAYBE COPY

I am not sure we need these:
core/app/models/spree/product/scopes.rb MAYBE NO COPY - see product_fitlers, do we use this?
core/spec/models/spree/product/scopes_spec.rb see previous
core/app/models/spree/product_scope/scopes.rb MAYBE NO COPY same as above
core/app/models/spree/variant/scopes.rb MAYBE NO COPY same as above
core/spec/models/spree/variant/scopes_spec.rb see previous

oh, this is done.

Was this page helpful?
0 / 5 - 0 ratings