terminal42 / contao-notification_center

The most popular notification configuration extension for the Contao Open Source CMS!
63 stars 38 forks source link

Field type "file upload" > Wrong result in E-mail on the first automatic notification #315

Closed biowebfr closed 5 months ago

biowebfr commented 6 months ago

Hi!

I've some kind of issue with the file upload field type.

Here the code I tried inside the e-mail :

1- ##form_add##
2- {{file::##form_add##}} 

Here is the result inside the received e-mail :

1- /var/www/a-propos/files/uploads/avatar.jpg
2-

And after that when I use the function "Send notification of the lead" to resend the e-mail I got the good result : image

1- 13fa7af0-daf0-11ee-8bf0-fa163e917345

2- files/uploads/avatar.jpg

Am I doing something wrong ?

Thank you.

biowebfr commented 6 months ago

Another question : Is there a way to only get the name of the uploaded file in the e-mail ?

Toflar commented 6 months ago

What version of NC are you talking about? And what "lead"? Is this coming from the leads extension? Then it would be irrelevant for this project.

biowebfr commented 6 months ago

Sorry for the lack of information.

I use the :

  1. Contao Manager 1.8.4
  2. Contao 4.13.36
  3. Notification Center 1.6.14
  4. Leads 1.4

image

fritzmg commented 6 months ago

What is it that you want to do in your email exactly?

biowebfr commented 6 months ago

The e-mail should have this result :

So we can use insetags like {{file:: }}

At the moment the email has this (path from the server) :

And it's not correct

Do you understand ? :)

fritzmg commented 6 months ago

So we can use insetags like {{file:: }}

Why do you want to use this insert tag? Or another way to state the question: what should the final email look like for your case? Do you want to provide a download link in the notification?

fritzmg commented 6 months ago

Notification Center 1.6.14

You need to update to at least 1.7.8 btw. (which might not be possible due to other dependencies).

biowebfr commented 6 months ago

Yes, I think we should make a link to the file in the e-mail.

biowebfr commented 6 months ago

You are right, I cant update :

> Resolving dependencies using Composer Cloud v3.7.0
[7.5MiB/0.23s] Loading composer repositories with package information
[87.4MiB/16.56s] Updating dependencies
[120.4MiB/17.03s] Your requirements could not be resolved to an installable set of packages.
[120.4MiB/17.04s] 
  Problem 1
    - Root composer.json requires terminal42/notification_center 1.7.8 -> satisfiable by terminal42/notification_center[1.7.8].
    - terminal42/notification_center 1.7.8 requires codefog/contao-haste ^5.0 -> found codefog/contao-haste[5.0.0, ..., 5.1.16] but these were not loaded, likely because it conflicts with another require.
[120.4MiB/17.04s] <warning>Running update with --no-dev does not mean require-dev is ignored, it just means the packages will not be installed. If dev requirements are blocking the update you have to resolve those problems.</warning>
[55.3MiB/17.09s] Memory usage: 55.34MB (peak: 223.98MB), time: 17.1s.
[55.3MiB/17.09s] Finished Composer Cloud resolving.
fritzmg commented 6 months ago

You are right, I cant update :

You need to update all packages when trying to do that.

biowebfr commented 6 months ago

Is there any fix I can do to have the file id instead of the file path ?

fritzmg commented 6 months ago

Is there any fix I can do to have the file id instead of the file path ?

The notification center only receives what Contao provides in the form. If that's the full path, then that's the full path - I don't think the NC can change that.

Usually the path is relative though - so what you would usally have to do in order to provide a direkt link to the file is something like:

https://example.com/##form_file##
biowebfr commented 6 months ago

Ok, I understand what you mean. But... how can we explain that Lead record the ID instead of this absolute path from the root (/var/www/a-propos/files/uploads/avatar.jpg) ?

image

I mean, something is wrong with the first mail send with NC.

There must have been some conversion to get the ID at some point. No ?

biowebfr commented 6 months ago

Here is what I have in the config of the e-mail : image

And here in the e-mail sent : image

fritzmg commented 6 months ago

Ok, I understand what you mean. But... how can we explain that Lead record the ID instead of this absolute path from the root (/var/www/a-propos/files/uploads/avatar.jpg) ?

This has nothing to do with the Notification Center itself. It is instead related to how terminal42/contao-leads stores the data and then which data it passes to its notification.

biowebfr commented 6 months ago

So you can't explain why I've this path (/var/www/...) and not the relative one (files/....) : image

Is it coming from a contao conf ?

biowebfr commented 6 months ago

And terminal42/contao-leads use the same e-mail from NC

fritzmg commented 6 months ago

And terminal42/contao-leads use the same e-mail from NC

It's the same notification - but this time it is terminal42/contao-leads who provides the data for the notification, not the Notification Center itself.

biowebfr commented 6 months ago

So you tell me that NC and Leads dont use the same data for the same variable when sending the same e-mail ?

Isn't it a problem ? Don't Leads and NC come from Terminal42 ? These data shouldn't be unified ?

biowebfr commented 6 months ago

By the way here is what I get on local winamp : image

At least it needs to be a relative path and it's not. So there is no way to fix manually this for at least take the relative path if the ID is impossible ? I don't ask for a release, just a way to fix it in my project.

I'm sorry...

fritzmg commented 6 months ago

So you tell me that NC and Leads dont use the same data for the same variable when sending the same e-mail ?

Correct. Leads feeds the notification with its leads data (naturally) while the Notification Center feeds the notification with the data provided by the Contao form.

Don't Leads and NC come from Terminal42 ?

That has no bearing on this issue. terminal42/contao-leads uses terminal42/notification_center - but terminal42/notification_center is otherwise an independent package.

At least it needs to be a relative path and it's not.

Yes, usually the path is (supposed to be) relative for regular Contao upload forms. I haven't had time to confirm your issue yet.

biowebfr commented 6 months ago

Ok I get this to work like it should by changing this in vendor/terminal42/notification_center/classes/tl_form.php:101 :

image

Form::getFileUploadPathForToken($file) become $file['uuid']

Now it works like contao-leads.

image

But I'm not a dev, so it's probably crap :)

What do you think ?

fritzmg commented 6 months ago

It's definitely wrong as the Notification Center must not pass just the UUID as the token value.

biowebfr commented 6 months ago

So how can I do it right and getting that uuid :) ?

Cause even it's wrong, it works...

asaage commented 6 months ago

In Contao 5.3 / NC 2 I'm getting just the uuid even without leads installed. Thats pretty useless in the text/html field since {{file::##form_upload##}} returns nothing. Adding an attachment with ##form_upload## however works flawlessly.

Toflar commented 6 months ago

It's perfectly correct and by design. Being able to link to a file in text is not a supported use case and never was.

Toflar commented 6 months ago

But you are mixing v1 and v2 here. Also please remember this is an issue tracker, not a support forum. We have other means for that.

biowebfr commented 6 months ago

Sorry, for me it sounds like an issue.

I finally choose to do this.

image

I took inspiration here > https://github.com/terminal42/contao-notification_center/commit/6f34e38c246ec82e37e0376ed8705c2692d9de1e