Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Trying to get property 'fullname' of non-object #63

Closed
4unkur opened this issue Dec 16, 2019 · 17 comments · Fixed by #157
Closed

Trying to get property 'fullname' of non-object #63

4unkur opened this issue Dec 16, 2019 · 17 comments · Fixed by #157
Labels
bug Something isn't working Good Issue Nice catch V2 MailEclipse v2.x scoped Issues/PR V3 Version 3 of MailEclipse (Laravel 8)

Comments

@4unkur
Copy link

4unkur commented Dec 16, 2019

I have seen that similar issues have been already submitted & resolved, but I was not able to find a clear explanation what I have to do in order to fix this issue. I have MailEclipse version 1.3 installed
There is a ProductCreated mailable & product-created.blade.php markdown template. In that template file I have the following line:

New product "{{ $product->title }}" was created by user: {{ $product->user->fullname }}

Mailable class:

class ProductCreated extends Mailable
{
    use Queueable, SerializesModels;
    /**
     * @var User
     */
    public $user;
    /**
     * @var Product
     */
    public $product;

    /**
     * Create a new message instance.
     *
     * @param User $user
     * @param Product $product
     */
    public function __construct(User $user, Product $product)
    {
        $this->user = $user;
        $this->product = $product;
    }

    /**
     * Build the message.
     *
     * @return $this
     */
    public function build()
    {
        return $this->markdown('emails.product-created');
    }
}

Please assist. Thanks.

@LorenzoSapora
Copy link

LorenzoSapora commented Dec 16, 2019

Commenting so I can follow this, and also because I'm facing a similar error, but

"Trying to get property 'email' of non-object"

from

// vendor/laravel/framework/src/Illuminate/Mail/Mailable.php

    protected function setAddress($address, $name = null, $property = 'to')
    {
        foreach ($this->addressesToArray($address, $name) as $recipient) {
            $recipient = $this->normalizeRecipient($recipient);
 
            $this->{$property}[] = [
                'name' => $recipient->name ?? null,
                'address' => $recipient->email,
            ];
        }
 
        return $this;
    }
 

Fresh install laravel-mail-editor 1.3 into a development environment of laravel 5.8.*

EDIT: This is after install, clearing (all) cache, and going to project.dev/maileclipse which redirects to project.dev/maileclipse/mailables

EDIT2: I can, however, get to /maileclipse/templates, but clicking on 'new' gives me a general php error.

EDIT3: Appears to be an issue with my development environment. Not the stack, but the codebase/packages. Possibly a conflict, as I can freely use all the features in a blank 5.8 laravel install

@Qoraiche Qoraiche added the bug Something isn't working label May 6, 2020
@ReeceM
Copy link
Collaborator

ReeceM commented May 24, 2020

Hi @4unkur and @LorenzoSapora, are you still experiencing issues?

@KhoaDuongUQ
Copy link

The whole package is unusable with this bug. Is there anyone knows how to this?

@ReeceM
Copy link
Collaborator

ReeceM commented Jun 24, 2020

Hi @KhoaDuongUQ, the bug from what I have traced down is the fact that the relations are not loaded up.

So what you could do is remove the Model and leave it as an undefined object type and then the variable will be given a dynamic object, this would be a temporary solution to use when you are testing the mails in the preview pane.

But seems that this is still and issue and you are actively experiencing it, can you share details of the class that this affected ? Like if it is a relation that you are calling that is giving the error? 👍

@s72817
Copy link

s72817 commented Sep 15, 2020

Hi, same issue like @LorenzoSapora "Trying to get property 'email' of non-object".
Laravel: 6.18.40 no fresh install -> is an existing project

There is actually no way to call the dashboard of Mail Editor.

@ReeceM
Copy link
Collaborator

ReeceM commented Sep 15, 2020

Hi @s72817 thank you for your input.

Is the class or object that the email value is being retrieved from a Model or a different class?

If you would be able to share some details that can help to see what is the main cause of this

@s72817
Copy link

s72817 commented Sep 15, 2020

Hi @ReeceM thanks for your quick reply!!
If found the issue... in my Mail Folder there are some existing Mailable and one of them trigger it.
It works, if I comment out all of the existing Mailable. Actually I'm on the way to found my issue :)

@ReeceM
Copy link
Collaborator

ReeceM commented Sep 15, 2020

@s72817 so the reason why the issue may be there is that MailEclipse generates Model classes or provides variables to the mailable so that you can preview it.

There are two classes that are passed to the mailable, the one is an Eloquent Model if it is type hinted, and then if the other is not it is a Mocked class, this handles dynamic object calls.

So if there is a different type of class, it may be that it isn't instantiated to the correct value or that the Model class doesn't have the right things.

So if you are able to when you find which Mailable is causing it, you should be able to share the __construct() parameters. :)

Also, another thing that would be useful for your error would be the error log for when it happens, this would help to see which area it is coming from.

From my reply to another user on this issue, the main thing is when it is called via a relationship, then it doesn't load unfortunately, I am trying to figure what a practical way is to hydrate any relationship models attached to the parent model

@ReeceM ReeceM added Good Issue Nice catch good first issue A good issue to get your feet wet Hacktoberfest V2 MailEclipse v2.x scoped Issues/PR V3 Version 3 of MailEclipse (Laravel 8) labels Sep 15, 2020
@ReeceM ReeceM added this to To do in Hacktoberfest Sep 27, 2020
@BoGnY
Copy link

BoGnY commented Mar 5, 2021

same problem 😢

@ReeceM
Copy link
Collaborator

ReeceM commented Mar 5, 2021

same problem 😢

Hi @BoGnY sorry that you are having issues, may I ask if the value that you are calling is through a relation on the model or is it a normal class?

@BoGnY
Copy link

BoGnY commented Mar 5, 2021

@ReeceM commented on 5 mar 2021, 14:58 CET:

same problem cry

Hi @BoGnY sorry that you are having issues, may I ask if the value that you are calling is through a relation on the model or is it a normal class?

Hi @ReeceM the problem is on property of relation on the model.
This is one of Mailable with problem:

namespace App\Mail;

use App\Models\Subscription;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Mail\Mailable;
use Illuminate\Queue\SerializesModels;

class SendSubscriptionPaymentReminder extends Mailable implements ShouldQueue
{
    use Queueable;
    use SerializesModels;

    /** @var \App\Models\Subscription */
    public $subscription;

    /** @var string */
    public $url;

    /**
     * Create a new message instance.
     *
     * @return void
     */
    public function __construct(Subscription $subscription, string $url)
    {
        $this->subscription = $subscription;
        $this->url = $url;
    }

    /**
     * Build the message.
     */
    public function build(): self
    {
        $this->subject(trans('mail.subscription_payment_reminder.title', ['plan_name' => $this->subscription->plan->name])); // <-- HERE THE ERROR: Attempt to read property "name" on null

        return $this->view('mail.subscription-payment-reminder')
            ->with([
                'subscription' => $this->subscription,
                'url' => $this->url,
            ]);
    }
}

I'm using MailEclipse v2.2.4 with Laravel 7.30.4

@ReeceM
Copy link
Collaborator

ReeceM commented Mar 5, 2021

@BoGnY, alright, so I see that it is also from a relationship that you are calling name property.

I have been trying to wrap my head around a sensible fix for this. This would make it that all the possible relations for a model need to be faked. But I am not sure the depth that it would need to go so as to be practical or sensible.

It could be something that is defined in the config file, like max_depth kind of thing.

For any of the other mailable that you have there, is there anything that is deeper than a first / second level relationship call?

@BoGnY
Copy link

BoGnY commented Mar 5, 2021

No @ReeceM , I have at most 2 levels of depth in relationships 👍
Defining it in the configuration file seems to me the correct idea to avoid problems with larger applications

@ReeceM ReeceM added this to the New Features and updates milestone Mar 22, 2021
@wawanneutron
Copy link

same problem 😢.
Any solution for this? I am also getting the same

@ReeceM
Copy link
Collaborator

ReeceM commented Jun 21, 2021

📢 Just to provide some feedback. 📢

I have had some time to test out the idea I had to be able to detect/guess relations that are on models, as this is where the issue comes in, and to try and initialize those relations as well.

Through testing, the initial method is lending itself to limiting the total number of relations searched to at most 2, this is because to actual do it further would result in longer response time.

What will be available I am thinking then is this:

  1. At most 2 levels deep reaction testing
  2. No more than one model hydrated per relation,
    • possibly 2 from a config setting for things like loops (e.g. comments)
  3. If it can't hydrate the model because there isn't a factory, it will load the Mocker object class to provide dynamic response.

The reason for number 3 is the case where someone might have a relation, but because the package uses factories to create a new model instance, if it isn't there usually it provides the blank instance.

If someone is calling a relation like below:

$user->account->plan->name
^      ^        ^      ^
|      |        |       \ ------------------- adds to the headache, as plan is `null`
|      |        \ -------------------------- where the issue happens, the relation hasn't been loaded
|       \ ------------------------------------- this is still okay, as the Model::class returns null
\--------------------------------------------- this is okay

Then with the changes, hopefully the account will be a valid relation, the plan will also be, but name will have to be based on the plan.

At that point, my earlier comment of allowing a config to be set for the number of relations would have to come into effect for certain use cases. #63 (comment)

Please feel free to share a GH reaction to give some feedback 👍

@ReeceM ReeceM added WIP Work in progress and removed good first issue A good issue to get your feet wet labels Jun 25, 2021
@ReeceM ReeceM added this to High priority in Bug Tracking Jun 25, 2021
ReeceM added a commit that referenced this issue Jun 27, 2021
@ReeceM ReeceM linked a pull request Jun 27, 2021 that will close this issue
@ReeceM
Copy link
Collaborator

ReeceM commented Jun 28, 2021

Hi all that have commented on the issue or had this issue, I would like to see if those who had the issue are able to make use of the branch version to test in their Laravel apps.

You can spin up a new Laravel app or try it in one that already has MailEclipse installed , you can change the version to dev-fix/issue-63-loading-relations in the composer file and run composer update, it should require the branch and not the tagged version, obviously, this is just to test and confirm that the PR #157 does indeed fix the issue that some have faced.

You can share your issues back in here should the PR add issue for you.

Bug Tracking automation moved this from High priority to Closed Jul 16, 2021
@ReeceM
Copy link
Collaborator

ReeceM commented Jul 16, 2021

Hi all, we are happy to note that the versions 3.4 and 2.3 each have a fix for this issue.

Thank you all for being patient for the fix and also for initially raising the issue and providing more information about it so that it could be sorted out.

Please feel free to open a new issue should there be bugs for it.

@ReeceM ReeceM removed Reviewed WIP Work in progress labels Jul 16, 2021
@Qoraiche Qoraiche moved this from Closed to High priority in Bug Tracking Aug 2, 2023
@Qoraiche Qoraiche moved this from High priority to Needs triage in Bug Tracking Aug 2, 2023
@Qoraiche Qoraiche removed this from Needs triage in Bug Tracking Aug 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working Good Issue Nice catch V2 MailEclipse v2.x scoped Issues/PR V3 Version 3 of MailEclipse (Laravel 8)
Projects
No open projects
Hacktoberfest
  
To do
Development

Successfully merging a pull request may close this issue.

8 participants