Pull Request: Refactoring and Enhancing TimelinesController#489
Merged
Conversation
## Summary This pull request introduces multiple improvements to the `TimelinesController`, focusing on **code readability, maintainability, API response consistency, and enhanced Swagger documentation**. ## Key Changes ### 1. **Improved API Responses** - Introduced standardized **helper methods** to generate common responses (`NotFoundResponse`, `BadRequestResponse`, `SuccessResponse`). - Ensured all API endpoints return structured **JSON responses** (with `success` and `message` fields) for better API client consumption. - Added missing **error handling** for cases where resources are not found (`NotFound` response) instead of returning `null` values directly. ### 2. **Enhanced Swagger Documentation** - Ensured **consistent Swagger operation naming** using `nameof()` instead of hardcoded strings for maintainability. - Added missing XML documentation comments, ensuring all methods have clear descriptions of their purpose and parameters. - Fixed incorrect or redundant Swagger response types (e.g., removed `NoContent()` responses that caused issues with Swagger UI). ### 3. **Refactored Constructor and Dependency Injection** - Converted constructor from parameter-based field initialization to **direct private field assignment**, improving readability. - Improved dependency management by explicitly declaring **readonly fields** to prevent unintended modifications. ### 4. **Consistent Status Codes and Response Handling** - Updated `TimelinesCreate` and `TimelinesStop` to return **200 OK** with success messages instead of `204 NoContent`, ensuring all endpoints provide meaningful responses. - Introduced `ExistsAsync(machineId, timelineId, ct)` check in `TimelinesStop` to handle cases where the timeline does not exist before calling `StopAsync`. ## How to Test - Run the updated API and verify all endpoints return appropriate responses. - Use Swagger UI to check documentation and API responses. - Ensure API clients can handle improved error responses gracefully.
Contributor
Author
|
Please fully test yourself before merging, I have had issues with Building the API intermittently. |
Contributor
Read this as "Check yourself before you wreck yourself" ;^D |
Contributor
|
Great work, thank you for improving this endpoint! |
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
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.
Refactoring and Enhancing TimelinesController
Summary
This pull request introduces multiple improvements to the
TimelinesController, focusing on code readability, maintainability, API response consistency, and enhanced Swagger documentation.Key Changes
1. Improved API Responses
NotFoundResponse,BadRequestResponse,SuccessResponse).successandmessagefields) for better API client consumption.NotFoundresponse) instead of returningnullvalues directly.2. Enhanced Swagger Documentation
nameof()instead of hardcoded strings for maintainability.NoContent()responses that caused issues with Swagger UI).3. Refactored Constructor and Dependency Injection
4. Consistent Status Codes and Response Handling
TimelinesCreateandTimelinesStopto return 200 OK with success messages instead of204 NoContent, ensuring all endpoints provide meaningful responses.ExistsAsync(machineId, timelineId, ct)check inTimelinesStopto handle cases where the timeline does not exist before callingStopAsync.How to Test