Skip to content

Conversation

ker0x
Copy link
Contributor

@ker0x ker0x commented Jul 1, 2025

Replace call to getReflectionProperty($field)->getValue($entity) and getReflectionProperty($field)->setValue($entity, $value) with getFieldValue() and setFieldValue() methods which are available in both ORM 2.x and 3.x

This makes it possible to use the new propertyAccessor property introduced in ORM 3.4 and fallback to reflFields with ORM < 3.4

PR contain patch #2945 and also fix #2965

@ker0x ker0x force-pushed the get-set-field-value branch from beb13b3 to 7f1cdf6 Compare July 1, 2025 22:24
@ker0x ker0x marked this pull request as ready for review July 1, 2025 22:24
@ker0x ker0x force-pushed the get-set-field-value branch from 7f1cdf6 to f12ac1e Compare July 1, 2025 22:34
@mbabker
Copy link
Contributor

mbabker commented Jul 1, 2025

Can you add a test fixture covering the "generate a slug using an embeddable object" scenario? It looks like all the tests only use fields directly on the model and nothing with a relation or embed, so it'd probably be good to have this covered as well.

@ker0x
Copy link
Contributor Author

ker0x commented Jul 2, 2025

@mbabker test added 👍 !

@ro0NL
Copy link

ro0NL commented Aug 18, 2025

cc @mbabker

@phansys phansys added Bug A confirmed bug in Extensions that needs fixing. Needs Changelog note Needs Rebase labels Sep 14, 2025
@phansys
Copy link
Collaborator

phansys commented Sep 14, 2025

Thank you so much for the contribution @ker0x! And, sorry for the delay.

Now, #2945 is merged. Could you please perform a rebase and add the corresponding changelog notes to confirm what kind of changes are being introduced in this PR?

@ker0x ker0x force-pushed the get-set-field-value branch from 9f586df to a523abd Compare September 14, 2025 00:46
@ker0x
Copy link
Contributor Author

ker0x commented Sep 14, 2025

@phansys CHANGELOG update and rebase done 👍!

@ker0x ker0x force-pushed the get-set-field-value branch from a523abd to 3153eed Compare September 14, 2025 01:17
@VincentLanglet
Copy link
Contributor

VincentLanglet commented Sep 14, 2025

Changelog diff seems weird since it doesn't show 3.20.1 lines and my recent merged PR
image
So I dunno if a new rebase is needed

Also you have the PHPStan build failing
https://github.com/doctrine-extensions/DoctrineExtensions/actions/runs/17704496457/job/50321200876?pr=2966
and PHPUnit ones

@ker0x ker0x force-pushed the get-set-field-value branch from 3153eed to 567df5b Compare September 15, 2025 01:26
Add sluggable test with embeddable

Update CHANGELOG
@ker0x ker0x force-pushed the get-set-field-value branch from 567df5b to 5907f23 Compare September 15, 2025 01:33
@VincentLanglet
Copy link
Contributor

VincentLanglet commented Sep 15, 2025

I think this is re-ready for a review @phansys :)

After the merge, I'll see if I can reduce the baseline

Copy link
Collaborator

@phansys phansys left a comment

Choose a reason for hiding this comment

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

Thanks @ker0x!

Please, check the test failures.

Copy link

codecov bot commented Sep 20, 2025

Codecov Report

❌ Patch coverage is 93.23308% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.56%. Comparing base (264c25a) to head (51dce33).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
.../ReferenceIntegrity/ReferenceIntegrityListener.php 66.66% 5 Missing ⚠️
src/References/Mapping/Event/Adapter/ORM.php 66.66% 1 Missing ⚠️
src/Tree/Hydrator/ORM/TreeObjectHydrator.php 50.00% 1 Missing ⚠️
src/Tree/Strategy/ODM/MongoDB/MaterializedPath.php 50.00% 1 Missing ⚠️
src/Tree/Strategy/ORM/Nested.php 95.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2966   +/-   ##
=======================================
  Coverage   78.55%   78.56%           
=======================================
  Files         169      169           
  Lines        8823     8793   -30     
=======================================
- Hits         6931     6908   -23     
+ Misses       1892     1885    -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@phansys
Copy link
Collaborator

phansys commented Sep 20, 2025

Thank you for your patience @ker0x!

@phansys phansys merged commit f6a5fcd into doctrine-extensions:main Sep 20, 2025
23 checks passed
@ker0x
Copy link
Contributor Author

ker0x commented Sep 20, 2025

@phansys You're welcome 😉!

@ker0x ker0x deleted the get-set-field-value branch September 20, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug A confirmed bug in Extensions that needs fixing. Pending Author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Sluggable] Type error when generating slug using embedded properties and ORM 3.4

5 participants