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

IBX-6315: Added LocationArgumentResolver #270

Merged
merged 7 commits into from
Oct 19, 2023

Conversation

ciastektk
Copy link
Contributor

@ciastektk ciastektk commented Sep 1, 2023

Question Answer
JIRA issue IBX-6315
Type improvement
Target Ibexa version v4.6
BC breaks no

This PR adds ArgumentResolver for locationId passed as query parameter. Already added LocationParamConverter works only for locationId passed as route attribute.

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ibexa/engineering).

@ciastektk ciastektk changed the title Added location argument resolver Added LocationArgumentResolver Sep 1, 2023
@ciastektk ciastektk force-pushed the added-location-argument-resolver branch from 81272b3 to d0df5b2 Compare September 1, 2023 15:26
@konradoboza konradoboza requested a review from a team September 4, 2023 06:44
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

@ciastektk is there a JIRA issue connected with this change?

src/bundle/Core/Converter/LocationArgumentResolver.php Outdated Show resolved Hide resolved
@alongosz alongosz requested a review from a team September 4, 2023 07:29
@ciastektk ciastektk changed the title Added LocationArgumentResolver IBX-6315: Added LocationArgumentResolver Sep 4, 2023
@ciastektk ciastektk force-pushed the added-location-argument-resolver branch from 8db4946 to 9c53b7f Compare September 4, 2023 10:43
Copy link
Contributor

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

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

+1 apart from red PHPStan analysis.

@ciastektk ciastektk force-pushed the added-location-argument-resolver branch from e091b4d to f932081 Compare September 6, 2023 09:14
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@micszo micszo self-assigned this Sep 11, 2023
@micszo micszo removed their assignment Sep 12, 2023
@micszo micszo self-assigned this Oct 10, 2023
@ciastektk ciastektk force-pushed the added-location-argument-resolver branch from f932081 to 8a315d5 Compare October 10, 2023 12:52
@micszo micszo removed their assignment Oct 19, 2023
@ciastektk ciastektk force-pushed the added-location-argument-resolver branch from 8a315d5 to 25ace1c Compare October 19, 2023 08:04
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@alongosz alongosz merged commit 3a2be98 into main Oct 19, 2023
22 checks passed
@alongosz alongosz deleted the added-location-argument-resolver branch October 19, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants