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

[Constructor error] Fatal error when using package #1

Closed
Cannonb4ll opened this issue Mar 6, 2019 · 28 comments · Fixed by #196
Closed

[Constructor error] Fatal error when using package #1

Cannonb4ll opened this issue Mar 6, 2019 · 28 comments · Fixed by #196
Labels
bug Something isn't working Good Issue Nice catch

Comments

@Cannonb4ll
Copy link

First off; looks pretty awesome. I wanted to give it a run in my application (which has a lot of mailables), but straight away I get an error when I visit /maileclipse:

Too few arguments to function App\Mail\MailClass::__construct(), 0 passed in /Users/****/Workspace/****/vendor/qoraiche/laravel-mail-editor/src/mailEclipse.php on line 548 and at least 2 expected

The constructor of this class has 3 arguments available:

    public function __construct($var1,$var2, $var3 = null)
    {
      
    }

I would debug it myself and PR it if I had the time. Still thanks for such a great package 👍

@biwerr
Copy link

biwerr commented Mar 6, 2019

Where does the MailClass come from? Seems like a Class that is created by you, Can't find them in Laravel default Package....

@Cannonb4ll
Copy link
Author

MailClass is the name I changed because of security of my application.

Lets take another example to be more clear;

Too few arguments to function App\Mail\WelcomeUser::__construct(), 0 passed in /Users/****/Workspace/****/vendor/qoraiche/laravel-mail-editor/src/mailEclipse.php on line 548 and at least 2 expected

@Qoraiche Qoraiche added the Good Issue Nice catch label Mar 6, 2019
@biwerr
Copy link

biwerr commented Mar 6, 2019

Ok, my fault. Did not recognize that your expample shows a mailable Class 🤦‍♂️

In my App, i have similar use cases with Mailables, where an user or order is passed as Parameter to the Constructor or the Mailable.

@Cannonb4ll
Copy link
Author

I'll install it in a second again to debug it somewhat further. (I do not have really that much of complicated mailable classes but it seems to get stuck there)

@Cannonb4ll
Copy link
Author

I think I know what is going wrong.

Just installed it again, and went to line 548 in the mailEclipse.php class. I saw this piece of code:

$mailable_view_data = self::getMailableViewData(new $mailableClass);

And my mailable classes actually accept some constructor options that are not passed in in this piece of code. Thats why it is failing.

Is this ment to be installed in a new Laravel installation, or could it also be used in a already-existing application?

@Qoraiche
Copy link
Owner

Qoraiche commented Mar 6, 2019

@Cannonb4ll Thanks for opening the first issue!

I would be thankful if you share the mailable class App\Mail\MailClass that causes the issue.

@Cannonb4ll
Copy link
Author

Sure @Qoraiche:

<?php

namespace App\Mail\User;

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

class WelcomeEmail extends Mailable implements ShouldQueue
{
    use Queueable, SerializesModels;

    public $user;

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

    /**
     * Build the message.
     *
     * @return $this
     */
    public function build()
    {
        return $this
            ->subject('Welcome!')
            ->markdown('emails.user.welcome');
    }
}

@fridzema
Copy link

fridzema commented Mar 6, 2019

Same problem here:
Too few arguments to function App\Mail\CancelApprovalRound::__construct(), 0 passed in /vendor/qoraiche/laravel-mail-editor/src/mailEclipse.php on line 548 and exactly 1 expected

class CancelApprovalRound extends Mailable
{
  use Queueable, SerializesModels;
 
  public $project_file;
  public $project;
 
  /**
   * Create a new message instance.
   */
  public function __construct(ProjectFile $project_file)
  {
    $this->project_file = $project_file;
    $this->project = $this->project_file->Project;
  }
 

@Cannonb4ll
Copy link
Author

Yes, I can confirm its because there are no variables passed to the constructor.

I would try to read out which types are passed in the constructor and pass them. In case of a Model you could actually do for example new User or even User::first() to get default scaffolding data in the constructors.

@Cannonb4ll Cannonb4ll changed the title Fatal error when using package [Constructor error ] Fatal error when using package Mar 6, 2019
@Cannonb4ll Cannonb4ll changed the title [Constructor error ] Fatal error when using package [Constructor error] Fatal error when using package Mar 6, 2019
@Qoraiche
Copy link
Owner

Qoraiche commented Mar 6, 2019

@fridzema @Cannonb4ll yes thank you, I'm working now to fix this issue.

@biwerr
Copy link

biwerr commented Mar 6, 2019

Maybe an option is to define an Interface that should be implemented by the mailable class that is editable. So the developer can decide which data should be delivered to create an instance...

Interfance

interface EditableMailable{
    public static function createEditableMail();
}

Mailable:

<?php

namespace App\Mail\User;

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

class WelcomeEmail extends Mailable implements ShouldQueue, EditableMailable
{
    use Queueable, SerializesModels;

    public $user;

    public function __construct(User $user)
    {
        $this->user = $user;
    }

    public static function createEditableMail()
    {
        $user = User::first();
        $instance = new self($user):
        return $instance;
    }

    public function build()
    {
        return $this
            ->subject('Welcome!')
            ->markdown('emails.user.welcome');
    }
}

mailEclipse.php

$mailable_view_data = self::getMailableViewData($mailableClass::createEditableMail());

@Cannonb4ll
Copy link
Author

@biwerr I'm not feeling lucky enough to be editing 50+ mailable classes to make it work with this package. Seems not generic enough; idea is good though.

I would rather have it guess the constructor types and pass in.

@Qoraiche
Copy link
Owner

Qoraiche commented Mar 6, 2019

@Cannonb4ll, @biwerr, @fridzema error fixed, check out this commit 536bf7e.
Can you please confirm with your mailables?
Thanks again.

@Qoraiche Qoraiche pinned this issue Mar 7, 2019
@Qoraiche Qoraiche unpinned this issue Mar 7, 2019
@Cannonb4ll
Copy link
Author

Cannonb4ll commented Mar 7, 2019

@Qoraiche

Did you want me to install it as 'dev-master' because I noticed you did not release a new version?

Anyhow, I did that, we got a little bit further now, I am now getting this error:

Symfony \ Component \ Debug \ Exception \ FatalThrowableError (E_ERROR)
Class 'string' not found
            foreach ($args->all() as $arg) {
                if ( is_array($arg) ){
                    $filteredparams[] = new $arg['instance'];  <--- Fails here
                    continue;
                }
                $filteredparams[] = $arg;
            }

This is because I have a mailable class that accepts a string too. Example:

class WelcomeEmail extends Mailable implements ShouldQueue
{
    use Queueable, SerializesModels;

    public $user;
    public $message;

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

    /**
     * Build the message.
     *
     * @return $this
     */
    public function build()
    {
        return $this
            ->subject('Welcome!')
            ->markdown('emails.user.welcome');
    }
}

@fridzema
Copy link

fridzema commented Mar 7, 2019

Hmmm the error is gone, but i think this package can't work for me easily.
This example trows Trying to get property 'project_code' of non-object:

  public function __construct(ProjectFile $project_file)
  {
    $this->project_file = $project_file;
    $this->project = $this->project_file->Project;
  }
 
  /**
   * Build the message.
   *
   * @return $this
   */
  public function build()
  {
    return $this->view('emails.project_files.cancel_approval_round')
  ->subject('Notification | Approval round canceled for: '.$this->project_file->filename.' | Project: '.$this->project->project_code.'');

If i surround all those in optional() the error is gone but i get a Model not found exception for the probably the next mailable that needs to be edited this way.

I think if i edit all mailables it should work, but don't have the time to do it right now.

@Qoraiche
Copy link
Owner

Qoraiche commented Mar 9, 2019

Check out this commit fdabb35, pre-release (v1.1.0-alpha)

  1. The package will look now for the equivalent factory in the mailable constructor and check if the dependency is an eloquent model.
  2. Resolve all other non-eloquent objects.
  3. Editor live preview (for markdown) will output no object variables in the following format: {{ varname }}.

@fridzema, @Cannonb4ll, @biwerr i will be thankful if you confirm with your mailables.

Thanks again!

@sachinkumar121
Copy link

This issue is still in v3.5.1 i.e latest release. I tried with Laravel framework v8.40 and it returns the same error. I even try with dev-master branch as well.

@ReeceM
Copy link
Collaborator

ReeceM commented Jan 6, 2022

This issue is still in v3.5.1 i.e latest release. I tried with Laravel framework v8.40 and it returns the same error. I even try with dev-master branch as well.

Hi @sachinkumar121, thank you for highlighting this. Would it be possible to provide more detail? As I have found usually this is because the constructor is using a class sometimes that cannot be created.

If you can provide a error message and a reproduction it would be great.

@sachinkumar121
Copy link

sachinkumar121 commented Jan 6, 2022

Sure @ReeceM. I would like to share two issues.
Here is the thing. I have an existing Laravel project where I have a lot of Mail classes. I installed this package and open "http://localhost:8000/maileclipse/mailables" on my local system.

Earlier, it gives the error of Class "Database\Factories\WebAppDomainFactory" not found. I checked my WebAppDomain model and remove the hasFactory trait.

But if you create a new model by using the artisan command(php artisan make:model Test) then Laravel automatically uses this trait i.e HasFactory.

So by mistake, if we use any model whose factory class is not defined then it throws an error of factory class not found. I don't know why does this package need to check the factory class.

Anyway, the second issue is about __construct parameters.

It gives the following error with my current configuration.

Too few arguments to function App\Mail\SslNotInstalledMail::__construct(), 0 passed and exactly 1 expected

composer.json

{
    "require": {
        "php": "^7.3|^8.0",
        "ext-json": "*",
        "afosto/yaac": "^1.3",
        "backpack/crud": "4.1.*",
        "biscolab/laravel-recaptcha": "^5.0",
        "cloudflare/sdk": "^1.3",
        "doctrine/dbal": "^3.1",
        "fideloper/proxy": "^4.4",
        "fruitcake/laravel-cors": "^2.0",
        "guzzlehttp/guzzle": "^7.0",
        "inertiajs/inertia-laravel": "^0.3.5",
        "intervention/image": "^2.7",
        "jolicode/slack-php-api": "^4.3",
        "laravel/cashier": "^13.4",
        "laravel/framework": "^8.40",
        "laravel/jetstream": "^2.3",
        "laravel/sanctum": "^2.6",
        "laravel/socialite": "^5.2",
        "laravel/tinker": "^2.5",
        "laravelcollective/html": "^6.2",
        "nyholm/psr7": "^1.4",
        "propaganistas/laravel-disposable-email": "^2.1",
        "qoraiche/laravel-mail-editor": "^3.5",
        "sendinblue/api-v3-sdk": "^7.4",
        "sentry/sentry-laravel": "^2.8",
        "symfony/http-client": "^5.3",
        "tightenco/ziggy": "^1.0"
    },
}

SslNotInstalledMail.php

<?php

namespace App\Mail;

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

use App\Models\WebAppDomain;

class SslNotInstalledMail extends Mailable
{
    use Queueable, SerializesModels;

    /**
     * The webAppDomain instance.
     *
     * @var WebAppDomain
     */
    public $webAppDomain;
    public $server;

    /**
     * Create a new message instance.
     *
     * @return void
     */
    public function __construct(WebAppDomain $webAppDomain)
    {
        $this->webAppDomain = $webAppDomain;
    }

    /**
     * Build the message.
     *
     * @return $this
     */
    public function build()
    {
        return $this->subject("SSL Installation Failed for {$this->webAppDomain->name}")
                    ->from(env('MAIL_FROM_ADDRESS'), env('MAIL_FROM_NAME'))
                    ->markdown('emails.ssl-install-error');
    }
}

@sachinkumar121
Copy link

sachinkumar121 commented Jan 7, 2022

After 3 hours of investigation, I come to know the real reason behind this issue.

in my case the maileclipse's config's factory value is true. so all the conditions of the resolveFactory function of the MailEclipse model never pass because I don't have a factory class associated with my Model.

private static function resolveFactory($eloquentFactory, $model): ?object
{
if (! config('maileclipse.factory')) {
return app($model);
}
// factory builder backwards compatibility
if (isset($eloquentFactory[$model]) && function_exists('factory')) {
return call_user_func_array('factory', [$model])->make();
}
/** @var array|false $modelHasFactory */
$modelHasFactory = class_uses($model);
if (isset($modelHasFactory['Illuminate\Database\Eloquent\Factories\HasFactory'])) {
return $model::factory()->make();
}
return null;
}

So it is returning the null and the model reference parameter in the __construct method didn't pass.

Suggestions:- The error returned by package should be more relevant and mention that as you set the factory true and there is no factory class is associated with that model instead of showing

Too few arguments to function App\Mail\MailClass::__construct(), 0 passed in /Users//Workspace//vendor/qoraiche/laravel-mail-editor/src/mailEclipse.php

and also on Github readme.MD file there should be full information about the factory config value.

@ReeceM
Copy link
Collaborator

ReeceM commented Jan 7, 2022

Hi @sachinkumar121
Thank you for highlighting where the issue is coming from.

I’ll reopen this for now, will have a look later.

But in the meantime if you would like to, you are welcome to PR an exception/warning for the missing factories.

@ReeceM ReeceM reopened this Jan 7, 2022
ReeceM added a commit that referenced this issue Feb 9, 2022
@ReeceM ReeceM linked a pull request Feb 9, 2022 that will close this issue
9 tasks
@ReeceM
Copy link
Collaborator

ReeceM commented Mar 21, 2022

when i run this command php artisan laravel-mail-editor:install

this error show .There are no commands defined in the "laravel-mail-editor" namespace

please help me out

Hi @blackcore003 is this after the package has been installed?

Are you able to provide more information?

This may need to be a new issue rather.

@sachinkumar121
Copy link

Run php artisan route:list to find the route(s) of maileclipse.

@ReeceM
Copy link
Collaborator

ReeceM commented Mar 21, 2022

There seems to be more of a namespace issue in the package on the v2 branch.

Also running the route:list command will also have an error if the package hasn't loaded fully.

I am just busy at the moment and will have a look later.

@ReeceM
Copy link
Collaborator

ReeceM commented Mar 21, 2022

Hi @blackcore003 please could you run composer show qoraiche/laravel-mail-editor, I just want to confirm what version you are using of the package.

Please can you comment in the linked issue below #204 please, this is a separate issue ti this one here

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@ReeceM @Cannonb4ll @biwerr @fridzema @Qoraiche @sachinkumar121 and others