search

indiehd / web-api

GNU Affero General Public License v3.0
6 stars 4 forks source link

Velkart integration #183

Closed cbj4074 closed 4 years ago

cbj4074 commented 4 years ago

The essence of this PR is to eliminate the Order and OrderItem models in this repository in favor of integrating the separate Velkart package's Cart, Order and Product models. Velkart leverages a third-party shopping cart implementation, which spares us lots of work and enables us to avoid "reinventing the wheel".

The Velkart package is designed to be fairly generic, and perfectly fills this repository's need for a basic e-commerce implementation that it can build upon where the more complicated business logic diverges from what Velkart provides.

This project's e-commerce needs are pretty specific and unique, so to "stitch together" this repository and Velkart, I added a DigitalAsset model that serves as a bridge between a Song and a Velkart Product. In essence, a Song or an Album can be a DigitalAsset, and so, by extension, a Product that can be added to a Cart, and subsequently (upon successful checkout), an Order.

codecov-commenter commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@d9a25b7). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #183   +/-   ##
=========================================
  Coverage          ?   63.35%           
  Complexity        ?      401           
=========================================
  Files             ?      110           
  Lines             ?     1056           
  Branches          ?        0           
=========================================
  Hits              ?      669           
  Misses            ?      387           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d9a25b7...a58cbf6. Read the comment docs.

cbj4074 commented 4 years ago

@mblarsen I realize that this is a challenging change-set to review. Ultimately, I'm just looking for a "second set of eyes" to make sure I didn't overlook anything significant or do anything too egregious.

I should mention specifically that the old OrderItem and the new DigitalAsset models are not exactly equivalent, but the files/classes/tests for them can be almost the same, in terms of their functionality. Hence, you will see quite a bit of renaming from the former to the latter, with a couple of adjustments here-and-there.

cbj4074 commented 4 years ago

@mblarsen I should mention also that a handful of tests have been marked as ignored. My preference is to merge these changes in and then worry about those tests subsequently. I don't want the scope of these changes to grow to the point that it's unmanageable. I/we can address those tests in a separate PR.