Conversation
fixup
| }, | ||
| constraints: table => | ||
| { | ||
| table.PrimaryKey("PK_ShiftRatings", x => new { x.ShiftId, x.WorkerId }); |
There was a problem hiding this comment.
Should the FacilityId be included as a part of the PK?
| [HttpGet("facilities/list")] | ||
| public async Task<IActionResult> GetFacilities([FromBody] RateWorkerRequest request) | ||
| { | ||
| return await _dbContext.Facilities.ToListAsync(); |
There was a problem hiding this comment.
To follow the standard as the rest of the code, this should be moved to the repository, a handler and return the appropriate type.
| } | ||
|
|
||
| [HttpGet("facilities/list")] | ||
| public async Task<IActionResult> GetFacilities([FromBody] RateWorkerRequest request) |
There was a problem hiding this comment.
This is a get method. Request should not have a body.
| { | ||
| try | ||
| { | ||
| _workerRepository.BlockWorker(request.WorkerId, request.FacilityId); |
There was a problem hiding this comment.
A return needs to be added in the try statement.
if the BlockWorker is async, an await needs to be added.
| [HttpPost("facilities/rate-worker")] | ||
| public async Task<IActionResult> RateWorker([FromBody] RateWorkerRequest request) | ||
| { | ||
| return await _rateWorkerHandler.HandleAsync(request); |
There was a problem hiding this comment.
No validations in any part of the code to make sure the data will be properly saved.
| [HttpPost("facilities/block-worker")] | ||
| public async Task<IActionResult> BlockWorker([FromBody] BlockWorkerRequest request) | ||
| { | ||
| return await _blockWorkerHandler.HandleAsync(request); |
There was a problem hiding this comment.
No validations in any part of the code to make sure the data will be properly saved.
| Score = request.Score | ||
| }; | ||
|
|
||
| _dbContext.ShiftRatings.Add(shiftRating); |
There was a problem hiding this comment.
this is calling the DB directly instead following the structure of the BlockWorkerHandler that uses a repository.
| { | ||
| try | ||
| { | ||
| _workerRepository.BlockWorker(request.WorkerId, request.FacilityId); |
There was a problem hiding this comment.
A validation needs to be added to avoid duplicate records.
| Score = request.Score | ||
| }; | ||
|
|
||
| _dbContext.ShiftRatings.Add(shiftRating); |
There was a problem hiding this comment.
A validation needs to be added to avoid duplicate records.
| } | ||
| catch (Exception e) | ||
| { | ||
| return new JsonResult(new { error = e.Message }); |
There was a problem hiding this comment.
Logs need to be added for debugging purposes.
| } | ||
| catch (Exception e) | ||
| { | ||
| return new JsonResult(new { error = e.Message }); |
There was a problem hiding this comment.
Logs need to be added for debugging purposes.
| constraints: table => | ||
| { | ||
| table.PrimaryKey("PK_ShiftRatings", x => new { x.ShiftId, x.WorkerId }); | ||
| table.ForeignKey("FK_ShiftRatings_Shifts_ShiftId", x => x.ShiftId, "Shifts", "Id", onDelete: ReferentialAction.Cascade); |
| { | ||
| table.PrimaryKey("PK_ShiftRatings", x => new { x.ShiftId, x.WorkerId }); | ||
| table.ForeignKey("FK_ShiftRatings_Shifts_ShiftId", x => x.ShiftId, "Shifts", "Id", onDelete: ReferentialAction.Cascade); | ||
| table.ForeignKey("FK_ShiftRatings_Facilities_FacilityId", x => x.FacilityId, "Facilities", "Id", onDelete: ReferentialAction.Cascade); |
| _blockWorkerHandler = blockWorkerHandler; | ||
| } | ||
|
|
||
| [HttpGet("facilities/list")] |
There was a problem hiding this comment.
Route is duplicating the "facilities" part.
| return await _dbContext.Facilities.ToListAsync(); | ||
| } | ||
|
|
||
| [HttpPost("facilities/rate-worker")] |
There was a problem hiding this comment.
Route is duplicating the "facilities" part.
| return await _rateWorkerHandler.HandleAsync(request); | ||
| } | ||
|
|
||
| [HttpPost("facilities/block-worker")] |
There was a problem hiding this comment.
Route is duplicating the "facilities" part.
| } | ||
| catch (Exception e) | ||
| { | ||
| return new JsonResult(new { error = e.Message }); |
There was a problem hiding this comment.
Depends on the complexity of the API and how public it is, this error message should not be returned.
A GUID or an identifier should be returned and saved anywhere using the identifier to search it.
| } | ||
| catch (Exception e) | ||
| { | ||
| return new JsonResult(new { error = e.Message }); |
There was a problem hiding this comment.
Depends on the complexity of the API and how public it is, this error message should not be returned.
A GUID or an identifier should be returned and saved anywhere using the identifier to search it.
2 participants
Implementing requested feature.