jamfluencer / api

2 stars 1 forks source link

DRY Jam checking logic #2

Open marshall-davis opened 1 month ago

marshall-davis commented 1 month ago

The logic in each Jam related route that checks is a Jam is active should move to a middleware or otherwise be made less repetitive.

pechtelt1 commented 1 month ago

Created an PR on this issue. Few suggestions for the code base:

Hope this helps you!

marshall-davis commented 1 month ago

Thanks! I appreciate the PR and the additional comments.

As you've seen the Spotify Token primary uses that of the Jam "host", and I've debated moving that to a common function (like middleware) but figured I'd cross that bridge when I decided on exactly how I want to handle the other user's authentication to Spotify. So you'll likely see that idea or similar implemented within a few weeks.

Route groups are starting to be a must. I 100% agree. This was "down and dirty" at first and it's become a mess.

However, I'm not sure I'm sold on controllers for this level of complexity. I think moving some of this functionality into services or shared actions will make the controllers so lean that they may as well be closures. That one is TBD.

pechtelt1 commented 1 month ago

@marshall-davis thanks for your reply. I understand your thinkings! :)