Laravel 8

[cbj4074]

To address any lingering Laravel 8 upgrade-related issues.

It seems like we still have some issues with seeders, which probably need to be updated to reflect the factory-related changes we already made.

[codecov[bot]]

Codecov Report

Merging #196 (1a3fb38) into master (918262a) will decrease coverage by 0.20%. The diff coverage is 54.54%.

@@             Coverage Diff              @@
##             master     #196      +/-   ##
============================================
- Coverage     70.95%   70.74%   -0.21%     
  Complexity      409      409              
============================================
  Files           113      112       -1     
  Lines          1105     1104       -1     
============================================
- Hits            784      781       -3     
- Misses          321      323       +2     

Impacted Files	Coverage Δ	Complexity Δ
app/Artist.php	100.00% <ø> (ø)	8.00 <0.00> (ø)
app/Console/Commands/Feature.php	100.00% <ø> (ø)	2.00 <0.00> (ø)
app/DigitalAsset.php	100.00% <ø> (ø)	2.00 <0.00> (ø)
app/Featured.php	100.00% <ø> (ø)	2.00 <0.00> (ø)
app/Http/Controllers/Api/AlbumController.php	100.00% <ø> (ø)	5.00 <0.00> (ø)
app/Http/Controllers/Api/ArtistController.php	100.00% <ø> (ø)	5.00 <0.00> (ø)
app/Http/Controllers/Api/FeaturedController.php	71.42% <ø> (ø)	7.00 <0.00> (ø)
app/Http/Controllers/Api/GenreController.php	40.00% <ø> (ø)	5.00 <0.00> (ø)
app/Http/Controllers/Api/LabelController.php	80.00% <ø> (ø)	5.00 <0.00> (ø)
app/Http/Controllers/Api/OrderController.php	0.00% <ø> (ø)	18.00 <0.00> (ø)
... and 42 more

---

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 be28f4d...1a3fb38. Read the comment docs.

[mblarsen]

Please see commit messages and TODOs added to OrderSeeder @cbj4074

[cbj4074]

@mblarsen Sorry for the delayed response!

Your confusion is entirely well-founded, and you're not overlooking anything. I had started reworking the Order domain object and the associated model, repository, and controller, and started this Laravel 8 upgrade before those changes were completely finished.

I'm just getting my head back into this, and it looks like there is an as yet unmerged branch on the docs repo that explains the nature of these changes. Let me re-orient myself and I'll post another update late today.

[cbj4074]

Actually, the aforementioned change to docs was indeed merged already, and may be viewed at:

https://github.com/indiehd/docs/tree/cc6ecac678943c009943aa2403e942bc2bc4a268/velkart/domain-knowledge

I still haven't had a chance to examine that cart/order TODO and look into the price issue you mentioned, but I'll do that tomorrow.

FWIW, it seems like all tests are passing, as of my most recent commit.

[cbj4074]

@mblarsen Actually, let's merge this, because the scope of these changes is primarily about upgrading to Laravel 8, and the Cart, Order, and Price issues fall outside of that scope. I will open another Issue for those and link it here.

[cbj4074]

@mblarsen I just committed a few changes that address my previous comment.

With this PR in its current state, do you see any issues remaining? Or do you have any follow-up questions regarding any Velkart integration issues that remain unanswered?

[mblarsen]

The reason for copying the factories is that we cannot access those from velkart, but we still want factories for cart and order.

For long term we can move the factories out of dev so that they can be used in the client code. That way any other code that uses the velkart package will be able to use the factories as well.

[mblarsen]

Hmm, it seems like they have already been moved out of autoload-dev so it could be that we can use the factories from velkart

[cbj4074]

The reason for copying the factories is that we cannot access those from velkart

Do you recall the reason for that?

And do you think we're all set, and can merge this now?

FWIW, I reviewed all of the upstream changes to core Laravel files, and fixed the couple of small things that I had merged inadvertently in previous commits, and at this point, it all looks good to me.

[mblarsen]

I want to try first if we can use the factories from Velkart. Normally you cannot because they are in auto load-dev which is omitted when the package is used as a dependency. But I'm velkart you have included the factories in autoloadso it should work. Will give it a try.