Roslund
commented
6 years ago Well, it works, but lets keep this in a seperate branch for now.. I don't think it will the project course in any way except giving us more we have to document and wright about.. :)
Important to read before reviewing:
The decision on whether to merge this will have to wait until the next group meeting. It can still be reviewed, but don't merge it. (Also, if bugs or problems are discovered do voice them in the comments)
Pros vs. cons:
The main pros of merging are:
The main cons of merging are:
It might also be better to merge this after the presentation as it’ll lead to less potential of discovering new bugs during the presentation.
Description:
This pull request adds simple logging functionallity to the board. Currently only an administrator can view the log, but this could easily be changed by moving the route outside of the admin route group. The log is currently limited to the 50 latest movements to keep it somewhat short. It's only accessible via a direct link: admin/log.
Do Note: This pull request contains a change in the database structure, so you'll have to re-migrate and re-seed to test it.
The pull request also fixes a validation bug arising from the two separate methods of validating api requests. (Api routes in api.php having a different middleware and validation structure from web.api) CSRF Protection also wasn't used when requests were made to routes in api.php, which lead to a possibility for cross-site request forgery. While this might not be a problem right now it could lead to future problems if not fixed. Having the same validation for all API routes is also a good thing.
The implentation of this means that we'll have have timestamps for all movements of cards, addressing this part of the feedback from the client: