nova-inline-relationship icon indicating copy to clipboard operation
nova-inline-relationship copied to clipboard

Fields not resolving / ThirdParty integration.

Open wize-wiz opened this issue 5 years ago • 9 comments

Good day,

I'm sorry for this ridiculously long issue, but it's quite complicated ;)

I've commented on this #11 issue and some days later I noticed the ThirdParty Integration section in the readme, so I guess this kinda answers the question on how to make things compatible.

So first of all, I'm the maintainer of the nova-dependency-container package and I was working on some compatibility issues between the two packages. Some major problems were related to NDC not respecting the mutation of field attributes when trying to find dependency fields and the lack of setting an attribute of its own (which till now is null or empty).

The problems I found using this package are kind of complex and I probably haven't seen or understood everything yet concerning the overall code. So here are some questions and stuff I don't quite understand. I'll try to be as clear as possible :)

I only tested it with a BelongsTo relationship.

1) Resource fields are not resolving.

I've tried to get a single Select field to be resolved and I just could not get displayUsingLabels (which sets the display callback Field::$displayCallback) to work, using

Select('Some field', 'someattr')
    ->options([
        'value1' => 'Value 1',
        'value2' => 'Value 2'
    ])
    ->displayUsingLabels();

would always resolve in value1 to be displayed instead of Value 2.

Same goes for a simple Text field setting the resolve callback using resolveUsing which sets the Field::$resolveCallback.

Text::make('Some field', 'someattr')
    ->resolveUsing(function() {
        return 'Resolved value';
    }); 

The method displayUsingLabels or resolveUsing is called when availableFields is called on the resource, but the callbacks (Field::$displayCallback, Field::$resolveCallback) are only called if the fields resolve or resolveForDisplay are called. So I guess this never happens.

source

    public function getPropertiesWithMetaForDisplay($resource, $attribute): Collection
    {
        $fields = $this->getFieldsFromResource($resource, $attribute)
            ->filter->authorize(app(NovaRequest::class))
            ->filter(function ($field) {
                // should call $field->resolveForDisplay();
                return $field->showOnDetail;
            });

         /*
         // verifies fields are resolved
         foreach($fields as $field) {
              // shows 'Value 1' or 'Resolved value'
              Log::info($field->value);
         }
        */        

        return $this->getPropertiesFromFields($fields)
            ->keyBy('attribute')
            ->map(function ($value, $key) {
                return $this->setMetaFromClass($value, $key);
            });

nor are they resolved in getFieldsFromResource.

2) Resolved fields retrieved from related resource are never used.

Here we come to the part where I don't fully understand what (or more the why) is going on here. Basically the setMetaFromClass is where everything goes wrong. The call stack is:

  • Fields are retrieved by getFieldsFromResource which is called by getPropertiesWithMetaForDisplay or getPropertiesWithMetaForForms, depending on what type of request (Detail/Update/Create) was made.
  • getPropertiesWithMetaForDisplay or getPropertiesWithMetaForForms returns a generated array with newly created fields (components) returned by setMetaFromClass. It literally creates a new field $class = app($item['component'], $attrs); and sets the value later on $class->value = $value !== null ? $value : ''; where it is serialzed $item['meta'] = $field->jsonSerialize();.
  • This procedure is repeated when resolveResourceFields is called in resolve and resolveForDisplay, where updateFieldValue does the thing with setMetaFromClass all over again. The only difference I see is where this->value is iterated on to retrieve all resources in, for example, a many to many relationship.

The problem is that the "original" fields retrieved by getFieldsFromResource are never used anywhere. The nova dependency container relies on being resolved by itself. By calling resolve or resolveForDisplay, it resolves all fields nested in the container, also multiple nested nova dependency containers.

My first problem was that the related resource created in getFieldsFromResource never actually created the correct resource, but the resource being requested (not the resource related to the requested resource).

        $resource =
            ! empty($this->resourceClass)
            ? new $this->resourceClass($model) :
            Nova::newResourceFromModel($model->{$attribute}()->getRelated());

Here the $resource becomes the "viewed" resource, not the related resource. In a BelongsTo relation this line should be something like new $this->resourceClass($model->{attribute}) in order to successfully pass the related resource to its fields resolve and resolveForDisplay methods.

I created a fork and a branch with a modified version of the NovaInlineRelationship class called ModifiedNovaInlineRelationship. I only tested it with a BelongsTo setup but with some additional changes, this should work just as well for all other relationships.

Thank you for your time and I hope it all made some sense :)

Cheers

wize-wiz avatar Jan 22 '20 17:01 wize-wiz

@wize-wiz Thanks so much for this quite lengthy issue. And nice work on the Dependency package, btw. Now I am going to read the above issue and will respond shortly...

brandonferens avatar Feb 12 '20 16:02 brandonferens

Hi Guys, is this something that will be resolved in the near future?

alechancock1 avatar Mar 11 '20 16:03 alechancock1

We are actually working on a full rewrite of the package which will hopefully take care of this and all issues.

brandonferens avatar Mar 11 '20 16:03 brandonferens

Hello @wize-wiz

I'm using your branch to make a development since I'm using NovaDependencyContainer with NovaInlineRelationship.

My case is as follows:

Select::make('Field 1', 'field_1')->options([
    'option_1' => 'Option 1',
    'option_2' => 'Option 2',
])->displayUsingLabels(),

Select::make('Field 2', 'field_2')->options([
    'option_1' => 'Option 1',
    'option_2' => 'Option 2',
])->displayUsingLabels(),

NovaDependencyContainer::make([
    NovaDependencyContainer::make([
        Text::make('TextField', 'text_field'),
    ])->dependsOn('field_2', 'option_1'),
    NovaDependencyContainer::make([
        Text::make('Other Field', 'other_field'),
    ])->dependsOn('field_2', 'option_2'),
])->dependsOn('field_1', 'option_1'),

NovaDependencyContainer::make([
    Text::make('One More Field', 'one_more_field'),
])->dependsOn('field_1', 'option_2'),

It only works in the last container, I've been looking for a while and I think it's because in the response, when it returns the fields, the container has as key "" and that causes it to only take the last one.

In the function:

#NovaInlineRelationship.php
protected function getPropertiesFromFields(Collection $fields): Collection

I modified the key 'attribute'

attribute' => $field->attribute

by

attribute' => $field->attribute ? $field->attribute : 'unnamed_field_' . $this->unnamedFieldIndex++

And now a "unnamed_attribute_X" key is added to the empty values. A priori it had worked, but when it is rendered no case applies. So I went to see https://github.com/epartment/nova-dependency-container to see if I could get something out of it but I was unable to.

If you could guide me a little bit or do something in your fork I would appreciate it very much.

Cheers!

albertgpdevtrioteca avatar Jun 25 '20 12:06 albertgpdevtrioteca

@albertgpdevtrioteca I have a stupid question : how did you require the branch ?

I tried this in composer.json :

in "require" : "wize-wiz/nova-inline-relationship": "dev-wize-wiz",

in "repositories" :

        {
            "type": "git",
            "url": "https://github.com/wize-wiz/nova-inline-relationship.git"
        }

but I get :

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - The requested package wize-wiz/nova-inline-relationship could not be found in any version, there may be a typo in the package name.

Potential causes:
 - A typo in the package name
 - The package is not available in a stable-enough version according to your minimum-stability setting
   see <https://getcomposer.org/doc/04-schema.md#minimum-stability> for more details.
 - It's a private package and you forgot to add a custom repository to find it

vwasteels avatar Jun 25 '20 14:06 vwasteels

@albertgpdevtrioteca It would be better to post an issue on nova-dependency-container. Short version is, I'm currently using the development branch, which is the only branch so far that supports flexible content and the inline relationship package.

Just switch with composer require epartment/nova-dependency-container:dev-development

@vwasteels It's "type":"vcs" as in

{
    "name": "repo/test",
    "require": {

    },
    "repositories": [
	{
            "type": "vcs",
            "url": "https://github.com/wize-wiz/nova-inline-relationship"
	}
    ]
}

or depending on the structure of your repositories entry:

repositories: {
    "kirschbaum-development/nova-inline-relationship" : {
            "type": "vcs",
            "url": "https://github.com/wize-wiz/nova-inline-relationship"
    }
}

and then install with composer require kirschbaum-development/nova-inline-relationship:dev-wize-wiz

wize-wiz avatar Jun 25 '20 16:06 wize-wiz

@brandonferens Any update on the rewrite status? I'm currently on the verge of rewriting the package myself.

We are actually working on a full rewrite of the package which will hopefully take care of this and all issues.

wize-wiz avatar Jun 25 '20 16:06 wize-wiz

@wize-wiz thank you, I'm able to run your branch, but I'm having an issue that wasn't there before (I was running dev-master before) :

exception: "ArgumentCountError"
file: "....../vendor/kirschbaum-development/nova-inline-relationship/src/NovaInlineRelationshipServiceProvider.php"
line: 33
message: "Too few arguments to function Laravel\Nova\Fields\HasMany::KirschbaumDevelopment\NovaInlineRelationship\{closure}(), 0 passed and exactly 1 expected"

this is the resource field that is causing the error :

HasMany::make('Articles', 'articles', 'App\Nova\Resources\Article')->inline(),

Do you know why is that ? I tried to invetiguate, but honestly I'm very new to Nova, and this is beyond my knowmedge...

vwasteels avatar Jun 29 '20 14:06 vwasteels

Hello,

Can somebody can update me about this. I need to use both nova-dependency-container and nova-inline-relationship? I've been able to download repositories above. But I've got following error:

Too few arguments to function Laravel\Nova\Fields\HasMany::KirschbaumDevelopment\NovaInlineRelationship\{closure}(), 0 passed in /opt/eom-backend/vendor/laravel/framework/src/Illuminate/Macroable/Traits/Macroable.php on line 114 and exactly 1 expected {"userId":1,"exception":"[object] (ArgumentCountError(code: 0): Too few arguments to function Laravel\\Nova\\Fields\\HasMany::KirschbaumDevelopment\\NovaInlineRelationship\\{closure}(), 0 passed in /opt/eom-backend/vendor/laravel/framework/src/Illuminate/Macroable/Traits/Macroable.php on line 114 and exactly 1 expected at /opt/eom-backend/vendor/kirschbaum-development/nova-inline-relationship/src/NovaInlineRelationshipServiceProvider.php:33)

Thank you for the help

scramatte avatar Nov 03 '21 09:11 scramatte