Skip to content
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

Make OFN independent of spree_core #4826

Closed
luisramos0 opened this issue Feb 22, 2020 · 2 comments
Closed

Make OFN independent of spree_core #4826

luisramos0 opened this issue Feb 22, 2020 · 2 comments
Assignees
Labels
epic Group of issues tech debt

Comments

@luisramos0
Copy link
Contributor

luisramos0 commented Feb 22, 2020

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 🎉

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.

@luisramos0 luisramos0 added tech debt epic Group of issues labels Feb 22, 2020
@luisramos0
Copy link
Contributor Author

luisramos0 commented Apr 9, 2020

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:

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

@luisramos0
Copy link
Contributor Author

oh, this is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Group of issues tech debt
Projects
None yet
Development

No branches or pull requests

1 participant