arti/Estonian-ID-card-mobile-authenticator-POC
1
0
Fork 0
mirror of https://github.com/TanelOrumaa/Estonian-ID-card-mobile-authenticator-POC.git synced 2024-11-23 06:10:59 +02:00
Code Packages Releases Wiki Activity

Updated Peer review for "STELLA app" project (markdown)

TanelOrumaa 2021-11-22 22:13:13 +02:00
parent 97c10ee5ec
commit b047b36d99
1 changed files with 2 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
Peer-review-for-"STELLA-app"-project.md

@ -19,7 +19,8 @@ Also, having this kind of data in a resource file ensures that once the client n
Another small issue for backend is that some of the methods are a bit too long and some parts of code are repeating. In this case it would be beneficial to extract the repeating code. An example of this is in the `DatabaseService.java` and `ProductStatusService.java` files, where methods `deleteDiscount()`, `generateDiscountCode()`, `returnStatus()` and `updateClientsProductStatus()` all create a `RestTemplate` and add an URI and headers to it. This could be done in a separate method with all relevant data passed as parameters, which would also simplify unit testing. Another small issue for backend is that some of the methods are a bit too long and some parts of code are repeating. In this case it would be beneficial to extract the repeating code. An example of this is in the `DatabaseService.java` and `ProductStatusService.java` files, where methods `deleteDiscount()`, `generateDiscountCode()`, `returnStatus()` and `updateClientsProductStatus()` all create a `RestTemplate` and add an URI and headers to it. This could be done in a separate method with all relevant data passed as parameters, which would also simplify unit testing.
Another comment (not an issue) is that many of the `Model` classes have unused getter and setter methods. Since this is a WIP project, it’s likely that those methods will find use at a later date, but if not, there’s no point creating a bunch of unused methods. Creating getters and especially setters is something that’s best done on a need-to basis, so that you won’t accidentally create a setter for something that should be a read-only value (this is assuming those exist, as we are not familiar with the business logic, only guesses can be made here). Another comment (not an issue) is that many of the `Model` classes have unused getter and setter methods. Since this is a WIP project, it’s likely that those methods will find use at a later date, but if not, there’s no point creating a bunch of unused methods. Creating getters and especially setters is something that’s best done on a need-to basis, so that you won’t accidentally create a setter for something that should be a read-only value (this is assuming those exist, as we are not familiar with the business logic, only guesses can be made here).
On positive sides, the code is mostly very well commented and where it isn’t, self-documenting code practises are used. On positive sides, the code is mostly very well commented and where it isn’t, self-documenting code practises are used.
###Acceptance tests
### Acceptance tests
The functionality was checked according to the release notes in the wiki. Only the functionalities that were marked as delivered or mentioned in the project plan scopes for iterations 2 and 3 were checked. The bugs mentioned in the release notes were ignored because it would be pointless to point out something that the development team is already aware of and also things that belong under the iteration 4 tasks because the deadline has not passed yet. The functionality was checked according to the release notes in the wiki. Only the functionalities that were marked as delivered or mentioned in the project plan scopes for iterations 2 and 3 were checked. The bugs mentioned in the release notes were ignored because it would be pointless to point out something that the development team is already aware of and also things that belong under the iteration 4 tasks because the deadline has not passed yet.
In general the functionalities mentioned in the release notes are implemented and they seem to be working as intended (minus editing the existing table rows part, which according to the project plan seems to be a task for iteration 4 because the creation of update endpoints is mentioned there). In general the functionalities mentioned in the release notes are implemented and they seem to be working as intended (minus editing the existing table rows part, which according to the project plan seems to be a task for iteration 4 because the creation of update endpoints is mentioned there).
* The clients page of the admin panel works as described in the user story 4. All the necessary fields are present and it is possible to delete existing clients. * The clients page of the admin panel works as described in the user story 4. All the necessary fields are present and it is possible to delete existing clients.