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

Filter data nested forms #68

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

DAGpro
Copy link

@DAGpro DAGpro commented Sep 21, 2024

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

Copy link

codecov bot commented Sep 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (5f56ed6) to head (fe1628f).
Report is 32 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##              master       #68    +/-   ##
============================================
  Coverage     100.00%   100.00%            
- Complexity       257       282    +25     
============================================
  Files             13        15     +2     
  Lines            598       717   +119     
============================================
+ Hits             598       717   +119     

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

Copy link
Contributor

@arogachev arogachev left a comment

Choose a reason for hiding this comment

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

Hydration of nested form models works for both one-to-one and one-to-many relations - #69. Also the tests are needed. What's the purpose of these changes exactly?

@DAGpro
Copy link
Author

DAGpro commented Sep 30, 2024

@arogachev The purpose of the changes, when working with nested form fields, is for the hydrator to receive the correct data structure.

Field::text($formModel->nestedForm, 'text'); 
Field::text($formModel->nestedForm->seconNestedForm, 'desctioption'); 

@samdark samdark requested a review from vjik October 1, 2024 10:28
src/FormHydrator.php Outdated Show resolved Hide resolved
src/FormHydrator.php Show resolved Hide resolved
Copy link
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.

  1. Nested forms must use map loggic (see FormHydrator::createMap()).
  2. Nested forms may be in properties with any variable scope (private/protected/public).

@DAGpro
Copy link
Author

DAGpro commented Oct 4, 2024

  1. Вложенные формы могут находиться в свойствах с любой областью действия (частная/защищенная/публичная).

With dot-notation and array it will work without filtering.
Will Field be used with methods?

Field::text($form->nestedForm(), 'text');

@vjik
Copy link
Member

vjik commented Oct 4, 2024

With dot-notation and array it will work without filtering.

I think, get a form is developer task. Field::text($form->nestedForm(), 'text'); is ok.

@DAGpro
Copy link
Author

DAGpro commented Oct 6, 2024

  1. Nested forms must use map loggic (see FormHydrator::createMap()).

Map logic for nested forms awaits implementation yiisoft/hydrator#97 ?

@vjik
Copy link
Member

vjik commented Oct 7, 2024

Map logic for nested forms awaits implementation yiisoft/hydrator#97 ?

Yes, it's need to create nested mapping for hydrator.

@DAGpro
Copy link
Author

DAGpro commented Oct 27, 2024

[
    'firstForm' => Yiisoft\Hydrator\ObjectMap([
        'map' => [
            'secondForm' => Yiisoft\Hydrator\ObjectMap([
                'map' =>
                    [
                        'value' =>
                            [
                                0 => 'firstForm',
                                1 => 'secondForm',
                                2 => 'value',
                            ],
                    ],
            ]),
        ],
    ]),
    'value' => 'value',
];
[
    'firstForm' => Yiisoft\Hydrator\ObjectMap([
        'map' => [
            'secondForm.value' => [
                0 => 'firstForm',
                1 => 'secondForm',
                2 => 'value',
            ],
        ],
    ]),
    'value' => 'value',
];

There is a question of resolving such cases.

When specifying a Nested rule with a null argument on a parent form, ignore the property with the embedded form and the rules from that form?

@vjik
Copy link
Member

vjik commented Oct 27, 2024

When specifying a Nested rule with a null argument on a parent form, ignore the property with the embedded form and the rules from that form?

When Nested without rules, then rules from nested object should be used.

@DAGpro DAGpro requested a review from vjik November 16, 2024 18:07
@DAGpro DAGpro requested a review from arogachev November 16, 2024 18:07
@DAGpro
Copy link
Author

DAGpro commented Dec 4, 2024

Review?

@@ -9,6 +9,13 @@ use Yiisoft\FormModel\FormModel;

/** @var FormModel $formModel */
$field = Field::text($formModel, 'login');

/** Display fields nested forms */
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/** Display fields nested forms */
/** Display nested form */

Does that display whole form?

Copy link
Author

Choose a reason for hiding this comment

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

Whole?

nested form field or nested form 🤷‍♂️

@samdark
Copy link
Member

samdark commented Dec 6, 2024

@vjik would you please take a look at it?

@vjik
Copy link
Member

vjik commented Dec 8, 2024

@DAGpro

Do yiisoft/form widgets support nested forms?
Can you show example of form field HTML for property from nested form?

@DAGpro
Copy link
Author

DAGpro commented Dec 8, 2024

@DAGpro

Поддерживают ли yiisoft/formвиджеты вложенные формы? Можете ли вы показать пример поля формы HTML для свойства из вложенной формы?

Are you interested in using widgets directly from "yiisoft/form" or via the "formModel/Field" helper?

Filling out nested forms is allowed in the filterNestedForms method, error collection for nested forms is allowed in the validator package via the Nested rule class.

When using nested forms directly with the output of errors there are no problems, when using dot-notation and an array, all errors of nested fields are output in the parent field.

@DAGpro
Copy link
Author

DAGpro commented Dec 10, 2024

echo \Yiisoft\FormModel\Field::text($mainForm, 'value');
echo \Yiisoft\FormModel\Field::text($mainForm->firstNestedForm(), 'value');
echo \Yiisoft\FormModel\Field::number($mainForm->firstNestedForm()->secondForm(), 'value');
echo \Yiisoft\FormModel\Field::number($mainForm->firstNestedForm()->secondForm(), 'string');
<div>
    <label for="mainform-value">Value</label>
    <input type="text" id="mainform-value" name="MainForm[value]" value="v">
    <div>Value must contain at least 3 characters.</div>
</div>
<div>
    <label for="firstnestedform-value">Value</label>
    <input type="text" id="firstnestedform-value" name="FirstNestedForm[value]" value="va">
    <div>Value must contain at least 3 characters.</div>
</div>
<div>
    <label for="secondnestedform-value">Value</label>
    <input type="number" id="secondnestedform-value" name="SecondNestedForm[value]">
    <div>
        Value cannot be blank.
        <br>
        The allowed types for value are integer, float and string. null given.
    </div>
</div>
<div>
    <label for="secondnestedform-string">String</label>
    <input type="number" id="secondnestedform-string" name="SecondNestedForm[string]" value>
    <div>
        String cannot be blank.
        <br>
        String must contain at least 4 characters.
    </div>
</div>
echo Text::widget()->inputData(new \Yiisoft\Form\PureField\InputData(
    "MainForm[value]",
    $mainForm->value,
    validationErrors: $mainForm->firstNestedForm()->secondForm()->getValidationResult()->getErrorMessages(),
));
echo Text::widget()->inputData(new \Yiisoft\Form\PureField\InputData(
    "FirstNestedForm[value]",
    $mainForm->firstNestedForm()->value,
    validationErrors: $mainForm->firstNestedForm()->secondForm()->getValidationResult()->getErrorMessages(),
));

echo Text::widget()->inputData(new \Yiisoft\Form\PureField\InputData(
    "SecondNestedForm[value]",
    $mainForm->firstNestedForm()->secondForm()->value,
    validationErrors: $mainForm->firstNestedForm()->secondForm()->getValidationResult()->getErrorMessages(),
));
<div>
    <input type="text" name="MainForm[value]" value="v">
    <div>
        Value cannot be blank.
        <br>
        The allowed types for value are integer, float and string. null given.
        <br>
        String cannot be blank.
        <br>
        String must contain at least 4 characters.
    </div>
</div>
<div>
    <input type="text" name="FirstNestedForm[value]" value="va">
    <div>
        Value cannot be blank.
        <br>
        The allowed types for value are integer, float and string. null given.
        <br>
        String cannot be blank.
        <br>
        String must contain at least 4 characters.
    </div>
</div>
<div>
    <input type="text" name="SecondNestedForm[value]">
    <div>
        Value cannot be blank.
        <br>
        The allowed types for value are integer, float and string. null given.
        <br>
        String cannot be blank.
        <br>
        String must contain at least 4 characters.
    </div>
</div>

@vjik
Copy link
Member

vjik commented Dec 13, 2024

This PR required more time for review. Give me more time, I detailed review as soon as possible.

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.

4 participants