Enable Policies

[mblarsen]

• [x] enable existing policies
• [x] add viewAny
• [x] policies (for each controller):
  • [x] Album
  • [x] Artist
  • [x] Song
  • [x] Genre
  • [x] Label
• [x] enable in ApiController
  • [x] AlbumContrller
  • [x] ArtistController
  • [x] UserController
  • [x] LabelController
  • [x] GenreController
  • [x] .. these are the existing policies, should we enable for others? (which)
  • [x] Didn't create on OrderController and FeaturedController since no user should be able to change that is that correct? @cbj4074
• [x] fix tests (all/most controllers tests are broken because of this change)

[mblarsen]

@cbj4074 I've added and activated more policies, however, some parts of the model I need a better understanding of. I've asked here on Discord.

[mblarsen]

ps: @cbj4074 no need to review until "Ready for review"

[cbj4074]

@mblarsen To answer your questions in the TODO list:

• Re: which additional policies are needed and which controllers should have them enabled, I think you've hit them all, with the possible exception of OrderController (see next item).
• Re: the OrderController, any site visitor should be permitted to call any of those methods, whether authenticated or not. When a visitor adds an item to the shopping cart on the front-end, these API methods are called on the back-end. Does this effectively mean that it doesn't even require a policy?
• Re: the FeaturedController, generally speaking, that's correct; nobody is authorized to call these methods, with the exception of the artists() method, which returns the list of currently-featured Artists. (The list of featured Artists is generated automatically with an Artisan command.)

[mblarsen]

* Does this effectively mean that it doesn't even require a policy?

Yes, but it is better to be explicit about it and have an OrderPolicy that just has return true. I'll add this + Featured policy.

[mblarsen]

I've updated with your proposed changes.

I'm at bit unclear about delete, restore, and forceDelete in many cases.

[mblarsen]

I think we need to tweak this a bit. My recollection is that $song->users actually refers to users who have purchased the song, per the song_user table.

@cbj4074 I suggest renaming $song->users then to something more clear. E.g. buyers, customers, etc.

Easy to mistake to make.

[mblarsen]

@cbj4074 I had to refactor IndexRouteTest to use album instead of users as we have now blocked users.index.

[cbj4074]

I'm at bit unclear about delete, restore, and forceDelete in many cases.

I looked at those while reviewing and I think in the vast majority of cases, if not all of them, they are correct. I will double-check them once we get this merged-in (I don't think it's critical to verify them now, nor should doing so hold-up this PR).

@cbj4074 I suggest renaming $song->users then to something more clear. E.g. buyers, customers, etc.

I agree, and will do that once this is merged.

I'm taking another look at catalogable/profile issue and will respond to that shortly...

[cbj4074]

@mblarsen I took the liberty of merging your other PR into this one, and reconciling the handful of conflicts. Hopefully, I preserved all of the changes from both PRs, but please double-check my work.