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

HasNestedMany with nested fields in DependencyContainer #39

Open
wajdijurry opened this issue Oct 4, 2023 · 3 comments
Open

HasNestedMany with nested fields in DependencyContainer #39

wajdijurry opened this issue Oct 4, 2023 · 3 comments

Comments

@wajdijurry
Copy link
Contributor

I am using this package to nest a relation into other resource. In my case, I am nesting Variation resource into Product resource.

The Variation resource has the following fields:

// opt-out code
            Boolean::make('Enable Sale Price?', 'sale_price', function ($value) {
                return !is_null($value);
            }),

            DependencyContainer::make([
                Currency::make('Sale Price', 'sale_price')
                    ->currency('JOD'),
            ])->dependsOnNotEmpty('sale_price'),
// opt-out code

And the Product resource has this:

            HasManyNested::make('Variations', 'variations', Variation::class)
                ->propagate('id')
                ->hideFromIndex()
                ->useTabs(),

Something weird is going on, as illustrated in the attached video, when I check "Enable Sale Price?" for the second Variation, the first Variation got affected by this change.

This bug applies to all fields in DependencyContainer in the Variation resource.

screencast-admin.otomech.local-2023.10.04-19_01_26.webm
@wajdijurry
Copy link
Contributor Author

I figured out that the fields in the first Variation have the same IDs as the second Variation. I believe this is the case but I lack the experience to edit the fields templates in Vue.js

@Lupennat
Copy link

Lupennat commented Oct 5, 2023

HasNestedMany generate a "formUniqueId" for each nested form panel.
DependencyContainer Field does not use and propagate to child fields this prop.
The watcher recursively resolves all vue root nodes and assigns the value in "dependencyValues" based only on the field attribute, which in the context of hasManyNested is not unique.
Because of this, the dependency is shared among all Nested forms and not on the single form.

Probably it should be fixed with something like this on FormField.vue file

<template>
    <div v-if="dependenciesSatisfied">
        <div v-for="childField in field.fields">
            <component
               ...
                :form-unique-id="formUniqueId"
            />
        </div>
    </div>
</template>
<script>
export default {
      ...
      props: ['resourceName', 'resourceId', 'field', 'formUniqueId'],
      methods: {
           ...
           componentIsDependency(component) {
                if(component.formUniqueId !== this.formUniqueId) {
                    return false;
               }
               ...
            }
       }
}
</script>

@wajdijurry
Copy link
Contributor Author

HasNestedMany generate a "formUniqueId" for each nested form panel. DependencyContainer Field does not use and propagate to child fields this prop. The watcher recursively resolves all vue root nodes and assigns the value in "dependencyValues" based only on the field attribute, which in the context of hasManyNested is not unique. Because of this, the dependency is shared among all Nested forms and not on the single form.

Probably it should be fixed with something like this on FormField.vue file

<template>
    <div v-if="dependenciesSatisfied">
        <div v-for="childField in field.fields">
            <component
               ...
                :form-unique-id="formUniqueId"
            />
        </div>
    </div>
</template>
<script>
export default {
      ...
      props: ['resourceName', 'resourceId', 'field', 'formUniqueId'],
      methods: {
           ...
           componentIsDependency(component) {
                if(component.formUniqueId !== this.formUniqueId) {
                    return false;
               }
               ...
            }
       }
}
</script>

Thanks @Lupennat . I have forked this package and applied your fix. Now it is working like a charm!

@wajdijurry wajdijurry reopened this Oct 5, 2023
andrii-trush added a commit to isap-ou/nova-dependency-container that referenced this issue May 22, 2024
Fix bug in nested forms, as discussed in issue alexwenzel#39
alexwenzel pushed a commit that referenced this issue Jun 2, 2024
* Fix dependencies and remove debugging

* Fix formUniqueId in nested forms
alexwenzel added a commit that referenced this issue Jun 2, 2024
alexwenzel added a commit that referenced this issue Jun 2, 2024
alexwenzel added a commit that referenced this issue Jun 2, 2024
* Fix bug in nested forms, as discussed in issue #39 (#40)

* Fix dependencies and remove debugging

* Fix formUniqueId in nested forms

* Revert "Fix bug in nested forms, as discussed in issue #39 (#40)" (#44)

This reverts commit a2bcc77.

* Handle BackedEnum (#34)

* Handle BackedEnum
* Use ->value instead of tryFrom

* Get Correct Rules (#32)

Instead of trying to access the specific rule property and check if it is callable call the correct rule method instead so the individual field has a chance to handle the logic.

fixes #17

---------

Co-authored-by: Wajdi Jurry <[email protected]>
Co-authored-by: Lonny Kapelushnik <[email protected]>
alexwenzel added a commit that referenced this issue Jun 2, 2024
* added nvmrc file

* integrating various pull requests  (#45)

* Fix bug in nested forms, as discussed in issue #39 (#40)

* Fix dependencies and remove debugging

* Fix formUniqueId in nested forms

* Revert "Fix bug in nested forms, as discussed in issue #39 (#40)" (#44)

This reverts commit a2bcc77.

* Handle BackedEnum (#34)

* Handle BackedEnum
* Use ->value instead of tryFrom

* Get Correct Rules (#32)

Instead of trying to access the specific rule property and check if it is callable call the correct rule method instead so the individual field has a chance to handle the logic.

fixes #17

---------

Co-authored-by: Wajdi Jurry <[email protected]>
Co-authored-by: Lonny Kapelushnik <[email protected]>

---------

Co-authored-by: Wajdi Jurry <[email protected]>
Co-authored-by: Lonny Kapelushnik <[email protected]>
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

No branches or pull requests

2 participants