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

Anhänge via Tokens funktionieren nicht (mehr) mit dem contao-fineuploader #342

Open
wiphi opened this issue Jun 25, 2024 · 21 comments
Open

Comments

@wiphi
Copy link

wiphi commented Jun 25, 2024

Hallo zusammen,
folgende Module setze ich ein:

Contao: 4.13.44
terminal42/contao-fineuploader: 3.4.11
terminal42/notification_center: 2.0.9 

In einem Formular habe ich den Contao-Fineuploder als Feld eingebunden (file). Bei diesem können die Frontend-Benutzer mehrere Dateien hochladen. Diese Dateien sollen dann per Mail über das Notification-Center versandt werden. Dazu habe ich in der Benachrichtigung das Feld Dateianhänge via Tokens mit dem Token ##form_file## gefüllt.

In der Notification-Center Version 1.7 funktionierte das noch ohne Probleme, in der Version 2.0.9 leider nicht mehr.
Es wird keine Datei mehr übermittelt.
Ändere ich den Token in ##form_file_0## wird zumindest eine Datei übermittelt. Da ich aber nie genau sagen kann wie viele Dateien der Benutzer hochlädt, ist das leider keine Option eine Liste an Tokens ##form_file_XX## zu schreiben.

Beim Debuggen ist mir aufgefallen, dass im MailerGateway.php die Prüfung isBulkyItemVoucher fehlschlägt. Aber ich hab leider zu wenig Wissen, um das Problem korrekt zu beheben.

    private function addAttachmentsFromTokens(LanguageConfig $languageConfig, Parcel $parcel, EmailStamp $emailStamp): EmailStamp
    {
        $tokens = StringUtil::trimsplit(',', $languageConfig->getString('attachment_tokens'));

        foreach ($tokens as $token) {
            $vouchers = StringUtil::trimsplit(',', $this->replaceTokens($parcel, $token));

            foreach ($vouchers as $voucher) {
                if ($this->isBulkyItemVoucher($parcel, $voucher)) {  // <-- hier schlägt die Prüfung fehl, da keine ID sondern der Dateiname enthalten ist
                    $emailStamp = $emailStamp->withAttachmentVoucher($voucher);
                }
            }
        }

        return $emailStamp;
    }

Viele Grüße und besten Dank für die schöne Erweiterung und Pflege dessen!

@Toflar
Copy link
Member

Toflar commented Jul 17, 2024

Kann das irgendwer reproduzieren? Wir haben zig Kunden mit Installationen mit Fineuploader und NC2 und Fileuploads und bisher kamen keine Reports rein.

@wiphi
Copy link
Author

wiphi commented Jul 22, 2024

Danke für die Info @Toflar. Dann scheint ers ja kein allgemeines Problem zu sein. Ich schau mal, ob ich das reproduzierbar in einer leeren Umgebung aufbauen kann. Die Installation wurde geupgraded, vielleicht hängt das damit zusammen...
Ich melde mich wieder.

@w3scout
Copy link

w3scout commented Aug 14, 2024

@Toflar: hier (C5.3, NC2.0, kein Fineuploader, stattdessen mit dem Token eines Formular-Textfeldes in "Dateianhänge via Tokens") besteht das gleiche Problem. Die isBulkyItemVoucher-Prüfung gibt false zurück.
Könnte Zugangsdaten zur Verfügung stellen, falls Du es Dir selbst anschauen möchtest?

@Schmidty2
Copy link

Hallo,
ich habe das selbe Problem mit der Erweiterung https://github.com/trilobit-gmbh/contao-zipuploads-bundle.
Konfiguration genau wie in der Doku von Zipuploads beschrieben.
Bei Dateienhänge via Tokens ist ##form_autogeneratedZippedUploads## eingetragen.
Unter Contao 4.13 und dem NC 1.7 funktioniert das, unter Contao 5.3.12 und NC 2.0.13 funktioniert es nicht.

@Toflar
Copy link
Member

Toflar commented Aug 15, 2024

@Toflar: hier (C5.3, NC2.0, kein Fineuploader, stattdessen mit dem Token eines Formular-Textfeldes in "Dateianhänge via Tokens") besteht das gleiche Problem. Die isBulkyItemVoucher-Prüfung gibt false zurück.
Könnte Zugangsdaten zur Verfügung stellen, falls Du es Dir selbst anschauen möchtest?

Nicht nötig, ein Pfad zu einer Datei in einem Token ist nicht mehr supported in Version 2. Ich habe die Gründe dokumentiert: 55749ed

ich habe das selbe Problem mit der Erweiterung https://github.com/trilobit-gmbh/contao-zipuploads-bundle.

Gleiches Problem, das Bundle muss angepasst werden und den Zip-Upload als Bulky Item für das Notification Center zur Verfügung stellen. Also aktuell nicht kompatibel mit NC2.

@w3scout
Copy link

w3scout commented Aug 15, 2024

@Toflar: alles klar, danke für die Aufklärung.
Nur noch zum vollständigen Verständnis: das Feld "Dateianhänge via Tokens" ist also ab NC2.0 ohne Funktion?

@Toflar
Copy link
Member

Toflar commented Aug 15, 2024

Nein, das funktioniert natürlich (oder soll funktionieren) sonst müsste es ja nicht da sein. Aber das verwendete Token muss einen Bulky Item Storage Voucher beinhalten und dem Parcel muss via Stamp mitgeteilt worden sein, dass dieses Token zu diesem Parcel gehört.

@wiphi
Copy link
Author

wiphi commented Aug 16, 2024

@Toflar Danke für die Erläuterung!
Das bedeuet doch eigentlich, dass das NC korrekt arbeitet und die anderen Erweiterungen, wie contao-fineuploader, contao-zip-uploads-bundle, etc. angepasst werden müssten? Also besser ein Issue in den jeweiligen Repos aufmachen und auf dieses hier verlinken?

@fritzmg
Copy link
Collaborator

fritzmg commented Aug 16, 2024

There should be no issues with terminal42/contao-fineuploader (see also #342 (comment)).

@Toflar
Copy link
Member

Toflar commented Aug 16, 2024

Fineuploader stellt keine eigenen Tokens zur Verfügung. Das ist ja ein Formularfeld und läuft über den normalen Prozess von Formularfeldern mit form_*. Das contao-zip-uploads-bundle macht eigene Tokens die von Formularfeldern unabhängig sind.

@veronikaplenta
Copy link

@Toflar ich hab das Problem, wenn ich selber Dateianhänge via Tokens in einem CreateParcelEventListener hinzufüge. Ich erstelle ein Bulky Item Storage Voucher, schreibe das in den Token und hänge das Bulky Item Storage Voucher als Bulky Item Stamp mit dran. Wie bei wiphi gibt bei mir isBulkyItemVoucher im MailerGateway false zurück. Ich konnte das schon näher eingrenzen, hab mir den Inhalt des Tokens in meinem EventListener und im MailerGateway angeguckt und die Werte sind unterschiedlich. Warum das passiert, hab ich bisher aber nicht rausgefunden. (Mehr Infos sind auch im Slack zu finden: https://contao.slack.com/archives/CLTUNH78X/p1723208283618539)

@theDyingMountain
Copy link

Ich kann das Problem nachstellen. Gleicher Server (IONOS)
Contao 4.13, NC 1.7, FineUploader 3.4 -> Datei in E-Mail als Anhang
Contao 5.3, NC 2.0, FineUplaoder 3.4 -> Datei nicht als Anhang, nur Pfad ist in der E-Mail

@fritzmg
Copy link
Collaborator

fritzmg commented Aug 22, 2024

Contao 5.3, NC 2.0, FineUplaoder 3.4 -> Datei nicht als Anhang, nur Pfad ist in der E-Mail

Poste die Widget Konfiguration.

@theDyingMountain
Copy link

theDyingMountain commented Aug 22, 2024

Es wurde nichts geändert an den Einstellungen
Screenshot 2024-08-22 at 11-02-09 Formulare Online-Bewerbung ohne Select Felder Formularfeld ID 55 bearbeiten

@fritzmg
Copy link
Collaborator

fritzmg commented Sep 23, 2024

@Toflar I can reproduce the issue now. If you have a FineUploader widget with the name fineuploader, all you had to do in NC 1.x is put in ##form_fineuploader## into Attachments via tokens, which is intuitive.

In NC 2.x this will not work. You would have to use ##form_fineuploader_0## instead, even if you only allow the upload of one file.

And if you would allow the upload of 3 files, you would have to put in

##form_fineuploader_0##,##form_fineuploader_1##,##form_fineuploader_2##

And if you would allow the upload of 100 files, you would have to repeat this pattern 100 times.

This is not ideal. The main ##form_fineupload## token would already contain all the information needed:

image

It should be possible to use that information to attach all the files from one widget, I think?

@fritzmg
Copy link
Collaborator

fritzmg commented Sep 23, 2024

The issue is the following: \Contao\Form::processFormData() expects the $files array to look like this:

[
    'fieldname' => [
        'name' => '',
        'tmp_name' => '',
        'type' => '',
        'uploaded' => false,
    ]
]

Otherwise sending emails with files as attachments (or links) will not work correctly (I mean Contao's standard functionality, not the emails from NC).

Since you can upload multiple files with one widget with the FineUploader, FineUploader fills this the following way:

[
    'fieldname_0' => [
        'name' => '',
        'tmp_name' => '',
        'type' => '',
        'uploaded' => false,
    ],
    'fieldname_1' => [
        'name' => '',
        'tmp_name' => '',
        'type' => '',
        'uploaded' => false,
    ],
    …
]

This ensures that Contao will send the files as attachments correctly.

However, the ProcessFormDataListener of NC 2.x expects it like this in such a case:

[
    'fieldname' => [
        'fieldname_0' => [
            'name' => '',
            'tmp_name' => '',
            'type' => '',
            'uploaded' => false,
        ],
        'fieldname_1' => [
            'name' => '',
            'tmp_name' => '',
            'type' => '',
            'uploaded' => false,
        ],
    ]
    …
]

foreach ($this->fileUploadNormalizer->normalize($files) as $k => $files) {
$vouchers = [];
foreach ($files as $file) {
$fileItem = \is_resource($file['stream']) ?
FileItem::fromStream($file['stream'], $file['name'], $file['type'], $file['size']) :
FileItem::fromPath($file['tmp_name'], $file['name'], $file['type'], $file['size']);
$vouchers[] = $this->notificationCenter->getBulkyGoodsStorage()->store($fileItem);
}
$tokens['form_'.$k] = implode(',', $vouchers);
$bulkyItemVouchers = array_merge($bulkyItemVouchers, $vouchers);
}

Then ##form_fieldname## would contain the correct bulky items vouchers. But this would be incompatible with Contao's standard functionality - see https://github.com/contao/contao/blob/5668c8f3cdb39a6d708ef379e7cf89a06aafe319/core-bundle/contao/forms/Form.php#L529-L543

@Toflar
Copy link
Member

Toflar commented Sep 23, 2024

The FileUploadNormalizer takes care of exactly normalizing those different formats.

@fritzmg
Copy link
Collaborator

fritzmg commented Sep 23, 2024

The FileUploadNormalizer takes care of exactly normalizing those different formats.

It does not cover the case of the FineUploader - where the key will already be fineuploader_0, fineuploader_1 etc.

@Toflar
Copy link
Member

Toflar commented Sep 23, 2024

Then I guess that has to be fixed :)

@fritzmg
Copy link
Collaborator

fritzmg commented Sep 23, 2024

The question is how. The format of $files must stay

[
    'fieldname_0' => [
        'name' => '',
        'tmp_name' => '',
        'type' => '',
        'uploaded' => false,
    ],
    'fieldname_1' => [
        'name' => '',
        'tmp_name' => '',
        'type' => '',
        'uploaded' => false,
    ],
    …
]

otherwise the functionality of \Contao\Form won't work. But this means we have no direct connection between file and field anymore - as the keys are suffixed with integers.

The FileUploadNormalizer could use a regex to check whether a key ends with an integer and then assume the first part is the field name and use that as the key. But that in turn would mean you cannot have field names for file upload widgets anymore that end with _[0-9]+.

@fritzmg
Copy link
Collaborator

fritzmg commented Sep 23, 2024

Another solution could be to change the ProcessFormDataListener in such a way so that when it processes $submittedData it checks whether the value or values exist as a file in $files and then creates the bulky item accordingly and stores the bulky item ID or IDs directly, under one key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants