Skip to content

Fix incorrect operation Each (new Nested) rule for iterable of objects#768

Merged
samdark merged 9 commits into
yiisoft:masterfrom
Enjoyzz:nested-for-iterable-of-objects
May 31, 2025
Merged

Fix incorrect operation Each (new Nested) rule for iterable of objects#768
samdark merged 9 commits into
yiisoft:masterfrom
Enjoyzz:nested-for-iterable-of-objects

Conversation

@Enjoyzz
Copy link
Copy Markdown
Contributor

@Enjoyzz Enjoyzz commented May 29, 2025

Q A
Is bugfix? ✔️
New feature?
Breaks BC?

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.18%. Comparing base (fe63447) to head (d2cad7c).
Report is 77 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #768      +/-   ##
============================================
+ Coverage     94.40%   96.18%   +1.78%     
- Complexity      953     1035      +82     
============================================
  Files           108      122      +14     
  Lines          3018     3274     +256     
============================================
+ Hits           2849     3149     +300     
+ Misses          169      125      -44     

☔ 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.

Copy link
Copy Markdown
Member

@vjik vjik left a comment

Choose a reason for hiding this comment

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

Currently this case can be resolved by new Each(new Nested()), but it's not available as attribute. And it requires implement RulesProviderInterface and place rule in getRules() method, it's not convenient.

Whole, I like this suggestion.

@arogachev @samdark WDYT?

Comment thread src/Rule/NestedHandler.php Outdated
@samdark
Copy link
Copy Markdown
Member

samdark commented May 29, 2025

The idea is very good 👍 Need a bit more for it to be merged:

  1. Docs.
  2. CHANGELOG.
  3. What @vjik suggested

@Enjoyzz
Copy link
Copy Markdown
Contributor Author

Enjoyzz commented May 30, 2025

Отказался от Nested в пользу Each(new Nested()).
Мне кажется это более правильно?
Такая реализация закрывает #751
Что скажете?

@vjik
Copy link
Copy Markdown
Member

vjik commented May 30, 2025

Oh, creating objects for attribute parameters is available from PHP 8.1.

Yes. Each(new Nested()) is best way.

@vjik vjik closed this May 30, 2025
@vjik vjik reopened this May 30, 2025
@Enjoyzz
Copy link
Copy Markdown
Contributor Author

Enjoyzz commented May 30, 2025

И еще, теперь думаю это не New feature , а скорее bugfix, который фиксит то, что по логике должно было работать ранее. Соответственно даже можно документацию не трогать.
Впрочем как скажете.

Copy link
Copy Markdown
Member

@vjik vjik left a comment

Choose a reason for hiding this comment

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

Yes, now this is bugfix. Add line to changelog.

Comment thread tests/Rule/NestedTest.php Outdated
@Enjoyzz Enjoyzz changed the title Nested for iterable of objects Fix incorrect operation Each (new Nested) rule for iterable of objects May 30, 2025
@Enjoyzz Enjoyzz requested a review from vjik May 30, 2025 07:10
@vjik vjik requested a review from samdark May 30, 2025 07:35
@samdark samdark merged commit 1a706eb into yiisoft:master May 31, 2025
31 of 32 checks passed
@samdark
Copy link
Copy Markdown
Member

samdark commented May 31, 2025

Thank you!

@Enjoyzz Enjoyzz deleted the nested-for-iterable-of-objects branch May 31, 2025 10:51
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