Skip to content

[HttpKernel] Support Uid in #[MapQueryParameter] #58717

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 7, 2025
Merged

[HttpKernel] Support Uid in #[MapQueryParameter] #58717

merged 1 commit into from
Feb 7, 2025

Conversation

seb-jean
Copy link
Contributor

@seb-jean seb-jean commented Oct 30, 2024 •
edited by OskarStark
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #51618
License MIT

I added support Uid in #[MapQueryParameter]. To use it, you must do this:

#[Route(path: '/', name: 'user_show')]
public function indexAction(
    #[MapQueryParameter] ?Ulid $groupId = null,
)

@carsonbot carsonbot added this to the 7.2 milestone Oct 30, 2024
@alexandre-daubois
Copy link
Member

I guess unit tests covering this new feature will be required for it to be accepted.

@seb-jean
Copy link
Contributor Author

I guess unit tests covering this new feature will be required for it to be accepted.

Ok, but I don't know how to do it :/.

@OskarStark
Copy link
Contributor

Maybe take a look at the current tests? Mostly it's a good starting point

@seb-jean
Copy link
Contributor Author

seb-jean commented Nov 1, 2024 •
edited
Loading

The test I wrote failed because assertSame is used. It should be assertEquals. Do I replace assertSame with assertEquals on this line?

@seb-jean
Copy link
Contributor Author

seb-jean commented Nov 6, 2024

I changed it to assertEquals and it works now.

@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
@OskarStark
Copy link
Contributor

Please add a changelog entry

@seb-jean
Copy link
Contributor Author

seb-jean commented Jan 6, 2025

Please add a changelog entry

It's done !

@fabpot
Copy link
Member

fabpot commented Jan 6, 2025

@seb-jean You need to rebase as we don't merge PRs with a merge commit.

@seb-jean seb-jean force-pushed the uid-mapqueryparameter branch from 576d856 to ee695b0 Compare January 18, 2025 09:06
@seb-jean
Copy link
Contributor Author

@seb-jean You need to rebase as we don't merge PRs with a merge commit.

I just rebased.

@seb-jean
Copy link
Contributor Author

Hello, are there any changes I still need to make?

@seb-jean seb-jean force-pushed the uid-mapqueryparameter branch from ee695b0 to 9366905 Compare February 1, 2025 17:41
@fabpot fabpot force-pushed the uid-mapqueryparameter branch from 9366905 to 9fd614a Compare February 7, 2025 08:14
@fabpot
Copy link
Member

fabpot commented Feb 7, 2025

Thank you @seb-jean.

@fabpot fabpot merged commit c4ee8fd into symfony:7.3 Feb 7, 2025
6 of 7 checks passed
@fabpot fabpot mentioned this pull request Feb 7, 2025
Closed
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@fabpot fabpot fabpot approved these changes

@OskarStark OskarStark OskarStark approved these changes

Assignees
No one assigned
Labels
Feature HttpKernel Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

Work attribute MapQueryParameter with UidValueResolver
6 participants
@seb-jean @alexandre-daubois @OskarStark @fabpot @nicolas-grekas @carsonbot