Logging#198
Open
RobertDuras wants to merge 15 commits intomasterfrom
Open
Conversation
…ent via ajax to the api
…rd api logic is in one place with the same type of validation.
…eader. This was apperently removed at some point and caused no errors because the routes in api.php don't actually check the csrf-token. Now that it's in web.php we need it.
… rows, added before the private function call in card api
Owner
|
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.. :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: