Bye bye Spree :-) (or Spree v2.1 and beyond)

Tech Exploration
21

@luisramos0 @maikel @Matt-Yorkley @paco @sauloperez - I just added the tag gathering-19 to this post so that it comes up when looking for notes from all sessions. If you can do so for others would be great :slight_smile: Thanks

2 Likes
22

Now that upgrade to spree 2.1 is almost done, I spent a few weekend hours starting the upgrade to spree 2.2:

I think this would be a straight forward upgrade if it wasnā€™t the ā€œAdjustments refactoringā€ in spree 2.2.

This refactoring is probably very good in terms of spree BUT for OFN it will be a nightmare (and obviously a lot of time of dev and of scratching our heads).
To be more specific, I invite you to look (keep scrolling down!) at the post Ryan Biggs wrote at the time about the refactoring, itā€™s an essay with 20 pages!!!
https://ryanbigg.com/2013/09/order-adjustments

This materializes in mystic migrations like this one that completely break OFN. And I havent even thought about EnterpriseFees which are extra adjustments we do to orders in OFN.

Currently I am estimating this upgrade in so many headachesā€¦ and so I am thinking this is it.
On the other side, I believe we can do the copy/paste exercise (bye bye spree) pretty quickly and get the rails upgrades to rails 6 at a rather quick pace.

What do you devs think? I think we should make this decision as soon as rails 4 is live.

23

This completely terrifies me. I get that you are saying ā€œon the other side things will be greatā€ and I believe you, but this looks like a chasm that we are going to continue losing large chunk of our dev capacity in for a long time. Itā€™s hard for me to see the other side :frowning:

There is no way to do the bye bye spree before embarking on breaking all the adjustments and fees @luisramos0?

24

I can be more clear and less dramatic :slight_smile:
Option 1 - upgrade to spree 2.2 and adapt to the new adjustments and stuff - after this we would have to upgrade spree again and again and say byebye spree at some later stage (this painful path includes the good stuff from spree later versions)
Option 2 - say byebye spree now (copy paste what we need from spree to OFN) and start upgrading Rails on our side. Here, painful spree upgrades become the (a lot less painful) rails upgrades. We could simply focus on rails 4.2, 5 and 6.

My question is really: do we all agree itā€™s time for option 2?

25

ah yes, then I think the answer is clear :slight_smile: I agree!

26

Iā€™m missing the crucial part for this decision: What do we gain with Spree 2.2 and what do we lose?

I had a quick look at Ryan Biggā€™s article but didnā€™t read it all. I found one section about the future changes which mostly mentions reducing call backs. This is something we want, a win.

What are the changes that donā€™t make sense to us which would be losses?

I think we need more information to do this decision.

27

Hello Maikel,

pros of keeping Spree:

  • the adjustments changes are good, we get that benefit :+1:
  • we get the spree code upgraded to rails 6 if we go up to Spree 4
  • we get all the benefits of the improvements in the spree code

cons of keeping with Spree:

  • compared to the huge investment of time these spree upgrades represent, the benefits of the PROs above are ridiculously small (we still have to upgrade rails on the OFN code anyway)

  • in terms of investment past and future:

    • past - I have spent 9 months working on the spree 2.0 upgrade and spree 2.1 took a month for me and maybe a few weeks for Matt
    • future - we have 13 spree upgrades to rails 6 (I think itā€™s a good assumption they will be between 1 month and 9 months in size): spree 2.2, 2.3 (rails 4.1), 2.4, 3.0 (rails 4.2), 3.1, 3.2 (rails 5.0), 3.3 (rails 5.1), 3.4, 3.5, 3.6(rails 5.2), 3.7 (rails 5.2), 4.0 (rails 6) and 4.1

  • if we say byebye spree now (I think itā€™s a fairly quick task similar to removing spree_backend #4050) we can then focus on upgrading rails: 4.1, 4.2, 5.0, 5.1, 5.2, 6.0

  • in terms of investment on rails upgrades - Iā€™d say these 6 rails upgrades to rails 6 are at the very least half the (very large) size of the 13 spree upgrades above. By leaving Spree now we are cutting the upgrade to rails 6 time in half - I think loosing the improvements of the spree code from 2.2. to 4.1 is a comparatively low price we should pay.

  • also, after we leave Spree, we will immediately have a much better code base without decorators that we can refactor and improve, unlike the current core code that is difficult to refactor due to being dependent on Spree.

In a way this is a project management decision to get to rails 6 faster (or just to make it possible). Seems pretty obvious to me now that the faster we jump off spree, the faster we get to rails 6.

28

I believe I would choose also option 2. But I think @maikelā€™s approach is right and I wonder how many Spree improvements are hidden inside each Spree upgrade.
But not only around technical changes, maybe also for the general UI? If I remember correctly the big backend UI improvement comes with Spree 3. Would this be something worth to copy-paste or it is totally not worth it?

29

Rachel we only depend on spree_core that means we would only benefit from the changes to the core of spree.
Fortunately we have already left spree_backend and our messy deface customizations of it so we will not have to handle the adaptation to that :tada:

30

I did a short analysis of the work to remove spree in https://github.com/openfoodfoundation/openfoodnetwork/issues/4826
I think this is slightly bigger than removing spree_backend. I think spree_backend was a month of work, I think removing spree_core will take 1.5 months.
I also analyzed how difficult the upgrades to rails 4.1 and rails 4.2 will be, they look rather simple. Iā€™d say rails 4.1 is less than a month and rails 4.2 maybe a month (Iā€™m a bit unsure how much trouble will ā€œnils on serialized attributesā€ and ā€œorder of testsā€ cause us).
This is just to give us some timescales to manage this work:

  • reference - upgrade to spree 2.0 - 12 months
  • reference - remove spree_backend - 4 weeks
  • reference - stripeSCA work done so far - 4 weeks
  • reference - upgrade spree 2.0 to spree 2.1 - 8 weeks
  • remove spree core - 6 weeks
  • upgrade from rails 4.0 to rails 4.1 - 3 weeks
  • upgrade from rails 4.1 to rails 4.2 - 4 weeks

Disclaimer: #guesstimates

31

I am adding here something that I think is important. Itā€™s important to explain why the jump from rails 4.0 to 4.2 is important and in a way ends the massive tech debt problem we have right now:

  • Rails 4.0 was last patched in Jan2015
  • Rails 4.1 was last patched in July 2016
  • Rails 4.2 was last patched in March 2019 (EDIT: security patch rails 4.2.11.3 was released in May 2020)
32

@danielle asked me to chime in here so Iā€™m doing so. Iā€™ll preface this by acknowledging that Iā€™m not a regular contributor to this code base so feel free to take or leave my opinions :slight_smile:

IMO, a code base should not use external libraries for its core business logic. It makes development seem quicker initially, but is a maintainability nightmare. There will inevitably be customisations required which deviate from the feature set of the external library, and that will lead to the kind of monkey-patching (ā€œdecoratorsā€) present in OFNā€™s code base. OFN is an online store, so the functionality in Spree is very much part of its core business logic. This logic belongs directly in the code base for reasons of maintainability and clarity.

(I think there are strong parallels with the trade-offs discussed here: https://dashbit.co/blog/a-new-authentication-solution-for-phoenix)

So to me, the obvious choice is to vendor the Spree code in the OFN codebase and take ownership of it. You can remove all the bits of Spree that arenā€™t used by OFN, and get rid of all the monkey-patching which will make it a lot easier to navigate the code base. Yes, youā€™ll miss out on any refactoring or improvements done in future Spree versions, but who is to say that those things are actually relevant to OFN anyway? You can focus future refactorings on those which provide the most value to OFNā€™s code base rather than Spreeā€™s.

2 Likes
33

Itā€™s great to see you agree with this approach Jon! Thanks a lot for taking the time to comment.

34

Quick progress update here. This Tuesday I went for the rails 4.1 upgrade on top of spree 2.1. The build looks good and I made some progress fixing some initial issues.

To do this I relaxed the rails 4.0 requirement in spree 2.1 so that we could bump it to rails 4.1 in OFN.

The main point is that I realized we donā€™t need to do/finish ā€œbye bye spreeā€ BEFORE the upgrade to rails 4.2. We can go for the rails 4.1 and rails 4.2 upgrade without moving all spree code to our side. We can move the spree code to OFN gradually and meanwhile get the rails upgrades done :tada:
This is much better because of the time to market (bye bye spree estimate is 6 weeks), this means we are probably 3 dev weeks away from rails 4.1 and another 4 weeks away from rails 4.2.

3 Likes
35

A quick update on this project, meanwhile this July and August I took something like two weeks of my time and I managed to finish the bye bye spree project :heart: The PRs have been going through the pipe and there are now 6 PRs to be reviews/tested and merged https://github.com/openfoodfoundation/openfoodnetwork/issues/4826
After this we will no longer depend on spree all together.

As a result we have copied quite a bit of Spree v2.1 code to our code base.
This is Spreeā€™s license: https://github.com/spree/spree/blob/2-1-stable/license.md
Can anyone advise on what exactly we should now do about this legal aspect of the project?

1 Like
36

Just a quick (historical) update to share how amazed we are about how easy rails upgrades are without Spree.
6 months after removing Spree and we have already upgraded to rails 4.1, 4.2, 5.0 and now 5.2 is almost done.
This was definitely a hugely beneficial decision for OFN!

5 Likes
37

hey @luisramos0 do you know if this was this dealt with?

1 Like
38

Hello Rachel!
No, it was not addressed. I still dont know what we need to doā€¦

39

Thanks @luisramos0 Apparently we just need to add the notice. After chatting with the team yesterday at delivery circle, Iā€™ve created this: Add Spree's copyright notice to source code and documentation Ā· Issue #7619 Ā· openfoodfoundation/openfoodnetwork Ā· GitHub

1 Like