Conversation
fixup
| FacilityId = table.Column<int>(nullable: false), | ||
| WorkerId = table.Column<int>(nullable: false), | ||
| Score = table.Column<int>(nullable: false) | ||
| }, |
There was a problem hiding this comment.
there's a missing property to add an optional comment (nullable).
|
|
||
| [HttpGet] | ||
| public IActionResult GetFacilities() => Ok(_dbContext.Facilities.ToList()); | ||
| public FacilitiesController(AppDbContext dbContext, RateWorkerHandler rateWorkerHandler, BlockWorkerHandler blockWorkerHandler) |
There was a problem hiding this comment.
we should not expose a dbContext here. It should live within a Repository class.
| } | ||
|
|
||
| [HttpGet("facilities/list")] | ||
| public async Task<IActionResult> GetFacilities([FromBody] RateWorkerRequest request) |
There was a problem hiding this comment.
GET methods should not use Body according to conventions.
| _blockWorkerHandler = blockWorkerHandler; | ||
| } | ||
|
|
||
| [HttpGet("facilities/list")] |
There was a problem hiding this comment.
we should use relative paths, otherwise we could end up having duplicate segments.
| [HttpGet("facilities/list")] | ||
| public async Task<IActionResult> GetFacilities([FromBody] RateWorkerRequest request) | ||
| { | ||
| return await _dbContext.Facilities.ToListAsync(); |
There was a problem hiding this comment.
dbContext should not be exposed here. There should be a handler and a repository containing the dbContext and queries. We should not return full domain entities, use DTOs instead and return a proper status code (Ok, BadRequest, NotFound, etc).
|
|
||
| [HttpGet] | ||
| public IActionResult GetFacilities() => Ok(_dbContext.Facilities.ToList()); | ||
| public FacilitiesController(AppDbContext dbContext, RateWorkerHandler rateWorkerHandler, BlockWorkerHandler blockWorkerHandler) |
There was a problem hiding this comment.
this class is not following dependency inversion principle. We should use abstractions (interfaces) instead of implementations. Delegate instance creation to the framework by registering dependencies in the DI container to the proper lifetime.
| { | ||
| try | ||
| { | ||
| _workerRepository.BlockWorker(request.WorkerId, request.FacilityId); |
There was a problem hiding this comment.
calling a blocking operation like requests to DB, files, other API synchronously will block the current thread and can lead to thread pool starvation. Add await for this kind of operations as a good practice to release the thread so it can.
| } | ||
| catch (Exception e) | ||
| { | ||
| return new JsonResult(new { error = e.Message }); |
There was a problem hiding this comment.
there should be a log for this exception and we could return a general message to the client.
2 participants
Feature: Rate & Block Workers
Healthcare facilities using our shift management platform have requested functionality to:
This pull request introduces new endpoints to enable those actions.
Acceptance Criteria
✅ A facility can submit a rating (1–5 stars, optional comment) for a worker after a shift.
✅ A facility can block a worker from being assigned to future shifts.
✅ Blocked workers must not be assignable to future shifts at the blocking facility.