in2code-de / powermail

This is the official repository of the TYPO3 extension powermail! Powermail is a well-known, editor-friendly, powerful and easy mailform extension for TYPO3
https://in2code.de
88 stars 175 forks source link

Signalslot for FormController throws error whenn using createActionAfterMailDbSaved #411

Closed chequille closed 5 years ago

chequille commented 5 years ago

Hi Georg, I do have an extension using a signal slot for your FormController Class. Unfortunately if I use "createActionAfterMailDbSaved", my createAction throws an error: "Uncaught TYPO3 Exception: Argument 2 passed to Bvt\BvtPowermailPdf\PdfGen::createAction() must be of the type string or null, object given ......"

When I am using "createActionBeforeRenderView", everything is ok and working.

Unfortunately my extension needs the uid from the saved mail and of course it is zero when not using "createActionAfterMailDbSaved".

This is the createAction function definition:

public function createAction(\In2code\Powermail\Domain\Model\Mail $mail, string $hash = '' )

Any idea why this happens?

THanks and best regards, JÜrgen

chequille commented 5 years ago

Hi Georg, I know the reason for it. In your FormController Class in the createAction function, the signal is called in line 116 $this->signalDispatch(__CLASS__, __FUNCTION__ . 'AfterMailDbSaved', [$mail, $this]);

Because second parameter is now defined as string as well in my function, it is called as second parameter with $this. Of course this is not a string but an object.

The other dispatch functions you are calling with paramter [$mail, $hash, $this], therefore it is working. I changed it as well in line 116 and it did not throw the exception and is working as expected.

In the version I used before, second parameter in createAction was not explicitly defined as string, therefore it worked. Please let me know if there is another reason for your code. I guess it is a bug.

Best regards, Jürgen

einpraegsam commented 5 years ago

Hi Jürgen, first of all I'm not Georg. This is the guy with the news extension. To your question: I can't answer it, because I don't know your code. Uncaught TYPO3 Exception: Argument 2 passed to Bvt\BvtPowermailPdf\PdfGen::createAction() must be of the type string or null, object given leads to the situation that you have defined that the second argument should be a string.

chequille commented 5 years ago

My god sorry, this mistake I did already in the past :-) These are the two extensions I am working with !!! -:)

Ok Alex, you do not have to know my code, because the error is forced from your code in FormController.php line 116 as I wrote alread above. The signalDispatch in this line calls createAction with paramater $mail and $this. Therefore second parameter is an object not a string. All the other signalDispatches have $mail, $hash and $this as parameter. Second parameter is a string, therefore no error in my createAction function. I changed this line in your code to $this->signalDispatch(CLASS, FUNCTION . 'AfterMailDbSaved', [$mail, $hash, $this]); and everything is working fine.

So please have a look into this code and let me know if I am wrigth or wrong.

Best regards, Jürgen

einpraegsam commented 5 years ago

Hello again,

it doesn't matter what kind of type the parameters are. I can pass string, array, objects, and whatever. The signal in 97 passes three parameters while the signal in 106 passes two. That's fine.

Alex

chequille commented 5 years ago

Hi Alex, sorry, but this is not closed for me, because when I change my declartion of the createAction f.e. to: public function createAction(\In2code\Powermail\Domain\Model\Mail $mail, $formController = null ) { .... } there is no more error, but a warning like: ...... should be compatible with In2code\Powermail\Controller\FormController::createAction(In2code\Powermail\Domain\Model\Mail $mail, string $hash = '')

Therefore as I understand it, the line 116 in your code must look like: $this->signalDispatch(CLASS, FUNCTION . 'AfterMailDbSaved', [$mail, $hash, $this]); and not $this->signalDispatch(CLASS, FUNCTION . 'AfterMailDbSaved', [$mail, $this]);

Of course, I can change it in your extension, but in the next update, it is changed again. So please rethink this and have a look into it.

BR Jürgen

einpraegsam commented 5 years ago

No, I still think that there is no failure. Pls post your code including the slot registering and the class you are using.

chequille commented 5 years ago

Hi Alex, here the slot registering:

\TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(\TYPO3\CMS\Extbase\SignalSlot\Dispatcher::class)->connect(
        'In2code\\Powermail\\Controller\\FormController',
        'createActionAfterMailDbSaved',
        \Jsc\JscPowermailPdf\PdfGen::class,
        'createAction'
        );

And here the class:

<?php

namespace Jsc\JscPowermailPdf;

use TYPO3\CMS\Extbase\Utility\DebuggerUtility;
use TYPO3\CMS\Core\Error\Exception;
use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
 *  PdfGen class handles pdf generation and adds filename and link to powermail mail
 */
class PdfGen extends \In2code\Powermail\Controller\FormController {

     * @param \In2code\Powermail\Domain\Model\Mail $mail
     * @param \string $hash
     */
    public function createAction(\In2code\Powermail\Domain\Model\Mail $mail, string $hash = ''  )
    {   
        ......
    }
}

I removed the code inbetween, because it is not relevant. When I use as above, I get the error. If I change your code for the dispatch as in my comments above everything is working. BR Jürgen

einpraegsam commented 5 years ago

Hi again,

1) don't extend your PdfGen class with the FormController 2) createAction() can be the name but it has not to be the name of the function that you register 3) write createAction(Mail $mail, FormController $form)

that's it

Btw: I used the short writings for Mail and FormController (that needs a use statement of course). Otherwise you can also use the full qualified name as you already used in your example for Mail.

chequille commented 5 years ago

Ok, Alex, I did this, because the examples of signalslots I found did it as well this way. I will try it. But last question: Why not adding the $hash parameter in the signalDispatch in your code? You did it with all the other dispatches, why not for this one?

einpraegsam commented 5 years ago

Simply because at this point a hash is not needed any more and I would like to pass only arguments that are usefull

chequille commented 5 years ago

It is not working, because inside my createAction Function I do need as well such functions like `$formRepository = $this->objectManager->get('In2code\Powermail\Domain\Repository\FieldRepository'); THis throws now an eception call to a member function get() on Null ....

I do need all the content of the fileds in the $mail object and do need as well to add some new fields. It is probably possible to do this different, but I enhanced an existing extensions for my needs and therefore many such calls are in the code of createAction.

For the time being, I will do it with my patch of your code and will have a deeper look into my function to get the information out of the $mail object with different methods.

Any way thanks for your help Jürgen

einpraegsam commented 5 years ago

Hey Jürgen,

that is not a free support forum for your personal needs. If you really need the classes from the FormController you can extend it, but you have to name your function different then an already existing function in the parent class, to use a different number of variables to pass. Nevertheless you should not extend it (as I wrote). If you need the repository, then simply use a \In2code\Powermail\Utility\ObjectUtility::getObjectManager()->get(FieldRepository::class) or instanciate the objectmanager with makeInstance by yourself.

Alex

chequille commented 5 years ago

Hi Alex, I do agree, but if you would have had the same problem, that your extension using a signal slot from another one and it would not work after updating the other extension, you would have asked the developer as well. That is what I did !!! Thanks again for your help Have a nice weekend. Jürgen