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

Refactor: Transfer responsibility from handlers to services. #272

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

alexmerlin
Copy link
Member

@alexmerlin alexmerlin commented Jun 4, 2024

This PR does the following:

  • provides cleaner handlers, by moving find/throw exeption logic to services
  • removes duplicate checks across the code by having the above checks inside services
  • find* service methods will do one of the following:
    • find the requested entity and return it, allowing the request to continue processing
    • not find the requested entity, throw a NotFoundException, which will pass through the handler and get caught by the HandlerTrait's handle method that returns a 404 Not Found response
  • replaces old /** @ var Class $variable */ annotations with asserts, helping both the IDE and Psalm on inferring types

@alexmerlin alexmerlin added the enhancement New feature or request label Jun 4, 2024
@alexmerlin alexmerlin self-assigned this Jun 4, 2024
Copy link

github-actions bot commented Jun 4, 2024

Qodana for PHP

It seems all right 👌

No new problems were found according to the checks applied

💡 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]

@arhimede arhimede merged commit b48285d into 5.0 Jun 5, 2024
21 checks passed
@alexmerlin alexmerlin deleted the refactor-responsibility-from-handler-to-service branch June 5, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants