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

Fix routing parameters filled in incorrectly using mapping property names #775

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

c-v-c-v
Copy link

@c-v-c-v c-v-c-v commented May 8, 2024

Comment on lines +46 to +51
$properties[$name] = $this->resolveValue($dataProperty, $attribute, $parameter);

// keep the original property name
if ($name !== $dataProperty->name) {
$properties[$dataProperty->name] = $properties[$name];
}
Copy link
Contributor

@Tofandel Tofandel May 13, 2024

Choose a reason for hiding this comment

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

Should probably just be

Suggested change
$properties[$name] = $this->resolveValue($dataProperty, $attribute, $parameter);
// keep the original property name
if ($name !== $dataProperty->name) {
$properties[$dataProperty->name] = $properties[$name];
}
$properties[$dataProperty->outputMappedName ?? $dataProperty->name] = $this->resolveValue($dataProperty, $attribute, $parameter);

Instead of the next line

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that would seem the best solution, the problem is that validation now chooses the mapped name over the original name (this is a bug I need to fix but it is more complicated then I initially thought) while the data object is created from the original name ...

Something I need time to look at but is more complicated than you would expect since we need to know if a name was mapped for outputting errors.

@rubenvanassche
Copy link
Member

Thanks!

@rubenvanassche rubenvanassche merged commit b6b7ad5 into spatie:main Jun 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants