OFN v2 - Account Invoices

Product Development
1

From spree upgrade phase 2 issue #2918, I realized this feature has not been used in the last 2 years. Last invoices on live databases are from 2016.
There’s some effort involved in making this work in v2. I estimate 3 dev days.
This estimate is based on two problems that need to be addressed:

  • account invoices are based on orders without line items which is no longer possible in spree v2, I believe we would need to create some sort of product to include in these orders…
  • account invoices have code to move the order workflow and that code needs to be adapted to spree v2 (basically, setting shipping method only when order.state is delivery)
  • I am not sure this list is complete, maybe there are more things that need to be fixed, including verifying and validating the feature is currently working in v1 (the most recent invoices in UK live DB do not have a linked order id which indicates there’s a problem at least in this instance, this can be just a configuration problem).

Architecture

This part of the code makes use of OFN orders in an un-ortodox way where we have an order without products and with a custom distributor, shipping method and payment method. It’s a bad use of the spree model for other means…
I believe this part of the code should be external to OFN, not part of the OFN code base.
I recommend we delete this code until we decide to use this feature again, when we decide that, this feature should be moved to a separate engine at least, if not to an external software component all together.
This PR deletes the feature: https://github.com/openfoodfoundation/openfoodnetwork/pull/3321

This is a significant part of the OFN complexity, 4000 lines of smart code, so deleting this and forcing it out of the code base into an engine is a great step towards making OFN more simple and clean.

Alternatives

Releasing v2 with these specs commented out is not an option, I think we either fix it or delete it. This PR can be reverted easily later and the existing code fully re-used when/if we decide to make it work (in v2.1 for example).

So, the only alternative to deleting the feature is to fix it before releasing v2.
Releasing OFN v2 with this feature broken and commented specs is not a valid option.

Opinion

From what I read in slack this is really a dead feature, I think we should delete this and any other feature that is not being used and is not at the top of our priorities.

Postponing the deleting of waste is wasteful itself.

.

Thoughts? Votes? Concerns? :slight_smile:

2

For reference:

1 Like
3

I agree this should be removed. UK now use totally different systems to bill. We adapted this feature for UK needs and still it was more complex to use than automating an external system with Zapier and Quickbooks.

Australia are working out their business model and hence are interested in potentially using this again. However I would suggest that all instances explore options for automatic billing outside of OFN as it allows faster business model iterations than updating OFN code will.

I can share how I do it with Zapier if anyone is interested. And perhaps if many instances use this we can create an endpoint for cleaner integrations.

2 Likes
4

I"m just confirming - here we are talking about orders, accounts, invoices for OFN enterprise USERS. So systems we use to bill our users. Deleting this (which I agree with) has no impact on invoices or accounts for customers (people who buy from our users). Right?

5

Yes that is correct @tschumilas

6

I would like to quote Kirsten’s opinion here as well:

deleting makes me nervous. It hasn’t been used since 2016 because australia stopped charging people - not because it wasn’t working. We are currently going through a process of working out our business model for australia, and i would like to decide that and then decide the best way to implement it, before we take this implementation option off the table please. Thanks :slightly_smiling_face: so that’s a ‘please wait’?

I still agree with Luis though that we should remove the code.

  • Un-deleting is even easier than deleting.
  • It does hold up the Spree upgrade.
  • We don’t even know with which business model we will end up with.
1 Like
7

I also agree with the general opinion of removing it. We saw as a way to implement our business model now that is going to be soon put in place but all the arguments seem reasonable. Besides, I’m specially concerned with its maintenance cost.

@lin_d_hop could you doc how you solve it with Zapier and others? I’d like to know what options we have or I may end up writting a custom ruby script :trollface:

8

Here is the solution to the problem with Zapier:

1 Like
9

The PR to delete acct invoices is ready to be approved and merged:

10

This feature has now been deleted. I will archive the thread to keep discourse threads relevant :+1: