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

undefined index error after upgrading to nova 2.5.0 #86

Closed
nemrutco opened this issue Oct 16, 2019 · 13 comments
Closed

undefined index error after upgrading to nova 2.5.0 #86

nemrutco opened this issue Oct 16, 2019 · 13 comments
Labels
bug Something isn't working confirmed This issue has been confirmed and reproduceable

Comments

@nemrutco
Copy link

i am not sure why but i think there is a compatibility issue with nova 2.5.0, everything is fine until nova 2.4.0 version.

@snk-lab
Copy link

snk-lab commented Oct 16, 2019

Exactly the same in my environment

@Tarpsvo
Copy link

Tarpsvo commented Oct 16, 2019

Created a pull-request in laravel/nova to address this issue.

Another option to work around this is to make NovaDependencyContainer fill in the requiredCallback property and have it return anything (either true or false).

Follow updates at https://github.com/laravel/nova/pull/684.

@nemrutco
Copy link
Author

nemrutco commented Oct 16, 2019

Created a pull-request in laravel/nova to address this issue.

Another option to work around this is to make NovaDependencyContainer fill in the requiredCallback property and have it return false.

Follow updates at laravel/nova#684.

Are you sure you created pull request in right place? And it would be nice to have a sample code for the requiredCallback you mentioned

@Tarpsvo
Copy link

Tarpsvo commented Oct 16, 2019

It's definitely in the right place.

To fix this temporarily in your project, you can just add this to each NovaDependencyContainer:

->required(function () {
  return false;
})

@wize-wiz
Copy link
Contributor

@nemrutco @Tarpsvo @snk-lab Yea they added requiredCallback to Fields.php. A simple return false in the NovaDependencyContainer class constructor would fix it.

public function __construct($fields, $attribute = null, $resolveCallback = null)
{
    parent::__construct('', $attribute, $resolveCallback);

    $this->withMeta(['fields' => $fields]);
    $this->withMeta(['dependencies' => []]);

    $this->requiredCallback = function() { return false; };
}

But I'm not sure if this is just a workaround or an actual fix. They seem to be mixing a lot now in Fields.php.

@wize-wiz wize-wiz added bug Something isn't working confirmed This issue has been confirmed and reproduceable labels Oct 16, 2019
@Tarpsvo
Copy link

Tarpsvo commented Oct 16, 2019

I don't really like the solution of adding a useless requiredCallback (even though it is a fix) and I'm hoping they merge my PR with the !empty($this->attribute) check.

Giving the NovaDependencyContainer an actual attribute should also be a fix, though I haven't tested it and the attribute itself would also be equally useless.

@nemrutco
Copy link
Author

I don't really like the solution of adding a useless requiredCallback (even though it is a fix) and I'm hoping they merge my PR with the !empty($this->attribute) check.

Giving the NovaDependencyContainer an actual attribute should also be a fix, though I haven't tested it and the attribute itself would also be equally useless.

Follow updates at laravel/nova#684.

your link gets 404, that's why i asked are you sure you posted in right place.

wize-wiz added a commit that referenced this issue Oct 17, 2019
@wize-wiz
Copy link
Contributor

@nemrutco Yea laravel/nova isn't public as far as I know.

@Tarpsvo A PR doesn't help much if nova expects a key to be present in an array, where some PR for this package obviously didn't wrote anything compatible to novas behaviour prior to 2.5.0.

The default behaviour should apparently always be.

return [$this->attribute => [..]];

Although it would nice to check the index first, I think they made this choice to escalate deliberately.

Should be fixed in 1.2.7, please verify and if so close this issue.

@snk-lab
Copy link

snk-lab commented Oct 17, 2019

The problem could be avoided by including the following.
->required(function () { return false; })

@wize-wiz
Copy link
Contributor

@snk-lab composer update epartment/nova-dependency-container should accomplish the same thing without requiring anything in your code.

@snk-lab
Copy link

snk-lab commented Oct 17, 2019

@wize-wiz

@snk-lab composer update epartment/nova-dependency-container should accomplish the same thing without requiring anything in your code.

Thank you very much. Updated to remove unnecessary modifications.

@nemrutco
Copy link
Author

nemrutco commented Oct 18, 2019

@snk-lab composer update epartment/nova-dependency-container should accomplish the same thing without requiring anything in your code.

Problem solved. Thanks

@wize-wiz
Copy link
Contributor

Please close this issue if resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed This issue has been confirmed and reproduceable
Projects
None yet
Development

No branches or pull requests

3 participants