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

'$this' as $sourcePropertyName means complete source object #60

Merged

Conversation

mike4git
Copy link
Collaborator

@mike4git mike4git commented Oct 6, 2023

As mentioned in the issue:
Sometimes you want to populate the complete (source) object in a certain target attribute/property.
With that PR it is quite simple possible.

Closes #58

@mike4git mike4git linked an issue Oct 6, 2023 that may be closed by this pull request
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
src/Populator/PropertyMappingPopulator.php Outdated Show resolved Hide resolved
tests/Populator/PropertyMappingPopulatorTest.php Outdated Show resolved Hide resolved
@jdreesen
Copy link
Member

This PR would also mean, that with the following config:

# config/packages/neusta_converter.yaml
neusta_converter:
  converter:
    person.converter:
      # ...
      properties:
        email: ~
        phone: ''

the email would be populated from the target field email to the source field email
while the for phone, the source object would be populated into the target field phone.

Would that be correct, and is this what we want?

I'd rather suggest that both ~ and '' should do the same here.
What do you think?

@jdreesen jdreesen changed the title Empty $sourcePropertyName means complete source object Empty $sourcePropertyName means complete source object Oct 10, 2023
@mike4git
Copy link
Collaborator Author

This PR would also mean, that with the following config:

# config/packages/neusta_converter.yaml
neusta_converter:
  converter:
    person.converter:
      # ...
      properties:
        email: ~
        phone: ''

the email would be populated from the target field email to the source field email while the for phone, the source object would be populated into the target field phone.

Would that be correct, and is this what we want?

I'd rather suggest that both ~ and '' should do the same here. What do you think?

I see your point but for some circumstances it is necessary that the "complete" source object should be populated into a certain target property.
Should we write a new config for that?

What about a certain Populator type?!

@jdreesen
Copy link
Member

I see your point but for some circumstances it is necessary that the "complete" source object should be populated into a certain target property.

But do you need it at this place? If I understood correctly, you wanted to use this especially together with the ConvertingPopulator?

@mike4git
Copy link
Collaborator Author

mike4git commented Apr 2, 2024

What would you think about the following syntax:

# config/packages/neusta_converter.yaml
neusta_converter:
  converter:
    person.converter:
      # ...
      properties:
        email: ~
        phone: this

So, as you mentioned:

The complete source object will be converted into phone target property.

@jdreesen
Copy link
Member

jdreesen commented Apr 2, 2024

I really like this idea!

But I would suggest using $this as the keyword. Then it is easier to recognize that it is not a normal field name.

@mike4git mike4git force-pushed the 58-allow-converting-object-itself-by-convertingpopulator branch from 1430109 to 3807bf5 Compare June 13, 2024 14:13
@mike4git mike4git requested a review from jdreesen June 13, 2024 14:22
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
src/Populator/PropertyMappingPopulator.php Outdated Show resolved Hide resolved
@jdreesen jdreesen changed the title Empty $sourcePropertyName means complete source object '$this' as $sourcePropertyName means complete source object Jun 13, 2024
@jdreesen jdreesen merged commit 7eea357 into main Jun 13, 2024
5 checks passed
@jdreesen jdreesen deleted the 58-allow-converting-object-itself-by-convertingpopulator branch June 13, 2024 15:19
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.

Allow converting object itself by ConvertingPopulator
3 participants