Assetcontroller cleanup #5858
Assetcontroller cleanup #5858
Conversation
This is a giant diff without many functional changes, mostly cosmetic. I've pulled a number of methods out of assetscontroller, preferring instead to create some more targetted controllers for related actions. I think this cleans up the file some, and suggests some places for future targetted improvement. Fix weird missing things.
dmeltzer
force-pushed
the
assetcontroller-cleanup
branch
from
d57ba66
to
a653e47
Compare
|
A few other things fixed while poking at this:
|
|
Should we move the API controllers into their own business as well? |
|
I would think so... I'd love to readdress at some point the idea of moving everywhere to just using api controllers and parsing it in the frontend, it seems like we could get rid of a lot of code that way. |
| @@ -420,7 +369,7 @@ public function destroy($id = null) | |||
|
|
|||
| // Redirect to the user management page | |||
| return redirect()->route('users.index')->with('success', $success); | |||
| } catch (UserNotFoundException $e) { | |||
| } catch (ModelNotFoundException $e) { | |||
There was a problem hiding this comment.
As far as I can tell, UserNotFoundException doesn't exist anywhere... Maybe it was a laravel 4 relic?
There was a problem hiding this comment.
I can't remember exactly what led to this change--It was a few months ago, I remember that I didn't hit the expected error until changing it to this approach.
There are a number of other places in UsersController that uses UserNotFoundException, I haven't looked into porting those.
|
Yeah, absolutely. That's the eventual plan, though I don't know whether I want to use Vue for that or just jQuery. I hate having to compile assets every time there's a tiny change. :-/ Trait-ifying and consuming the API for writes/updates are one of the biggest priorities for v5. |
|
I'm going to go ahead and leave codacy where it is. Definitely cleaned up a lot of complexity, but AssetCheckinController@store() is going to be fixed when I extract a checkin method again, and the other ones are just codacy being picky :) |
Been sitting on this one for a little while, getting tired of trying to keep it up to date :) This looks much bigger than it is, I think. Primarily just code reorganization, moving Asset file operations to their own controller as well as bulk asset operations. Trying to keep things a bit more CRUDY as well as the files a bit less gigantic. If the direction looks good, I'll keep poking at some of the other controllers, if you feel like it's just noisy to make these changes feel free to reject :)