feat: 266 New Contractor view #278
Conversation
There was a problem hiding this comment.
The code is well structured overall and is kept clean. In addition, TaskJson and TaskListJson have been reused so that there is no duplicate code. Due to the incorrect path, the frontend cannot currently work with the backend. The error 404 is returned. If the following errors have been corrected, the connection can be tried again.
Nevertheless, I still have some room for improvement (most of them are also marked as comments in the code):
- The file ContractorsEnpoint.java should be renamed to ContractorsEndpoint.java.
- The context and version need to be added to the path so that the path is “api/v1/contractor/...” in ContractorsEndpoint.
- Both endpoints should use the ownerId so that the path is consistent.
- Limit and offset can be introduced in the contractors endpoint so that pagination is taken into account. (If it is introduced, then it must also be implemented in the frontend.
- If only one task is queried, then no list needs to be returned, only the one task. (in ContractorsController.java)
- Tests missing: ContractorResource.java, ContractorsController.java
- Checkstyle throws an error in JDK 17 and 21: The indentation depth is incorrect in the ContractorsEndpoint file. Must be corrected. The error should then be gone.
|
|
||
| /** | ||
| * @author Quirt | ||
| */ |
There was a problem hiding this comment.
The complete path is missing. We had set the path as "api/v1/contractors/...", so api and v1 are missing here as context and version. My suggestion is the same implementation as in ProjectEndpoint.java.
| */ | |
| */ | |
| @Path(ContractorsEndpoint.CONTEXT + "/" + ContractorsEndpoint.VERSION + "/" + ContractorsEndpoint.SERVICE) |
| */ | ||
| public interface ContractorsEnpoint { | ||
| String SERVICE = "contractors"; | ||
|
|
There was a problem hiding this comment.
| String CONTEXT = "api"; | |
| String VERSION = "v1"; |
| String SERVICE = "contractors"; | ||
|
|
||
| @GET | ||
| @Path("/{contractorId}/tasks") |
There was a problem hiding this comment.
For readability and understanding, it would be better to rename the contractorId to ownerId. (Must also be changed in the frontend if it is changed here)
| /** | ||
| * @author Quirt | ||
| */ | ||
| public interface ContractorsEnpoint { |
There was a problem hiding this comment.
This method is misspelled. It could causes problems.
| public interface ContractorsEnpoint { | |
| public interface ContractorsEndpoint { |
| @@ -0,0 +1,26 @@ | |||
| package de.remsfal.service.boundary.project; | |||
|
|
|||
| import de.remsfal.core.api.ContractorsEnpoint; | |||
There was a problem hiding this comment.
Import must be corrected.
| import de.remsfal.core.api.ContractorsEnpoint; | |
| import de.remsfal.core.api.ContractorsEndpoint; |
| } | ||
| } | ||
|
|
||
| public List<? extends TaskModel> |
There was a problem hiding this comment.
If only one task is queried, then no list needs to be returned, only the one task.
| @Parameter(description = "Filter to return only tasks of a specific user") @PathParam("owner") @NotNull | ||
| @UUID String ownerId, | ||
| @Parameter(description = "Filter to return only tasks with a specific status") @QueryParam("status") | ||
| TaskModel.Status status); |
There was a problem hiding this comment.
The indentation is incorrect. Must be corrected. (Checkstyle Error)
| @Parameter(description = "Filter to return only tasks of a specific user") @QueryParam("owner") @NotNull | ||
| @UUID String ownerId, | ||
| @Parameter(description = "ID of the task", required = true) @PathParam("taskId") @NotNull @UUID String taskId); |
There was a problem hiding this comment.
The indentation is incorrect. Must be corrected. (Checkstyle Error)
| public TaskModel getTask(final String projectId, final String ownerId, final String taskId) { | ||
| logger.infov("Retrieving a task (projectId = {0}, ownerId={1}, taskId = {2})", projectId, ownerId, taskId); | ||
| return this.getTask(TaskType.TASK, projectId, taskId); | ||
| } |
There was a problem hiding this comment.
I don't think we need the projectId here either.
| public TaskModel getDefect(final String projectId, final String taskId) { | ||
| logger.infov("Retrieving a defect (projectId = {0}, defectId = {1})", projectId, taskId); |
There was a problem hiding this comment.
I don't think we need the projectId here either.
|
Due to personal Events i dont think I can complete this Pullrequest in time and I would therefore recommend to reject this PR. |
This PR should create a Contractors endpoint that allows contractors to get an overview over Task that are assigned to the owner.
The endpoint are defined as followed:
Get all Tasks assigned to the contractor
Get Informations about a specific Task that is assigned to the contractor