Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #248: Refactored handlers #269

Merged
merged 25 commits into from
Jun 3, 2024
Merged

Issue #248: Refactored handlers #269

merged 25 commits into from
Jun 3, 2024

Conversation

alexmerlin
Copy link
Member

@alexmerlin alexmerlin commented May 28, 2024

This PR does the following:

  • simlify handler actions by removing responsability of handling exceptions
  • enhances handler response status codes based on the response / exception
  • handlers return HTTP status code based on response / exception
  • creates/uses case-specific exceptions instead of the generic ones
  • updates method docblocks with the correct exceptions thrown
  • capture code-level exception messages, log them to the daily log and rethrow the same type of exception with a safe message that can be shown in a UI
  • fixes user ResetPassword retrieval based on the hash from the path (vs always using the latest one)
  • minor code cleanups
  • replaced UUIDv1 usage with UUIDv4
  • restored #[ORM\HasLifecycleCallbacks] attribute to all entities extending our AbstractEntity because without that, the updatedAt field does not get populated
  • breaks ResponseTrait into HandlerTrait and ResponseTrait so that we can access *Response methods without including the handle method in a class
  • return 204 No Content on successful DELETE operations

Tasks remaining:

  • fix any issues reported by the reviewers
  • fix existing unit tests
  • add more unit tests

alexmerlin added 10 commits May 24, 2024 06:57
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
@alexmerlin alexmerlin added enhancement New feature or request 5.x labels May 28, 2024
@alexmerlin alexmerlin self-assigned this May 28, 2024
@alexmerlin alexmerlin marked this pull request as draft May 28, 2024 13:43
Copy link

github-actions bot commented May 28, 2024

Qodana for PHP

497 new problems were found

Inspection name Severity Problems
Undefined class 🔶 Warning 225
Undefined method 🔶 Warning 183
Undefined namespace 🔶 Warning 84
Type mismatch in property assignment 🔶 Warning 3
Incompatible return type 🔶 Warning 2

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

@alexmerlin alexmerlin linked an issue May 28, 2024 that may be closed by this pull request
@alexmerlin alexmerlin requested a review from MarioRadu May 29, 2024 13:48
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
@alexmerlin
Copy link
Member Author

@MarioRadu @cPintiuta When you have time, take another look at this PR and leave your feedback with any observations you have

Copy link
Member

@arhimede arhimede left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add 204 to DELETE
201 when a resource is uploaded for the first time

@alexmerlin
Copy link
Member Author

add 204 to DELETE 201 when a resource is uploaded for the first time

Implemented.

@alexmerlin alexmerlin marked this pull request as ready for review June 1, 2024 06:29
@arhimede arhimede self-requested a review June 3, 2024 09:44
@arhimede arhimede merged commit b4e6908 into 4.0 Jun 3, 2024
21 checks passed
@alexmerlin alexmerlin deleted the issue-248 branch June 5, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor handler
3 participants