contao / core

Contao 3 → see contao/contao for Contao 4
GNU Lesser General Public License v3.0
490 stars 214 forks source link

Widget FormFileUpload: Methode validate() definiert keinen $this->varValue #7256

Open nkirstein opened 10 years ago

nkirstein commented 10 years ago

Gibt es einen bestimmten Grund, warum die Methode validate() in der FormFileUpload.php keinen $this->varValue definiert wie die anderen Formular-Widgets?

Mir ist das gerade aufgefallen, weil ich das Mitglieder-Modul "Persönliche Daten" um ein Upload-Feld ergänzen wollte und dann festgestellt habe, dass beim Upload nichts in der Datenbank gespeichert wird, obwohl ich in der DCA eine entsprechende Spalte konfiguriert habe:

$GLOBALS['TL_DCA']['tl_member']['fields']['foto'] = array
(
    'label'                   => &$GLOBALS['TL_LANG']['tl_member']['foto'],
    'exclude'                 => true,
    'search'                  => true,
    'inputType'               => 'upload',
    'eval'                    => array('mandatory' => false,'feEditable' => true,'feViewable' => true,'feGroup' => 'address','tl_class'=>'clr long','extensions'=>$GLOBALS['TL_CONFIG']['uploadTypes'], 'storeFile'=>true, 'uploadFolder' => $this->User->homeDir, 'readonly' => false),
    'sql'                     => "varchar(255) NOT NULL default ''''
); 

Das ModulePersonalData ruft ja nacheinander für jedes Formularfeld die Methode validate() auf und holt sich den $objWidget->value. Der wird dann im entsprechenden Feld des Member Models abgelegt und am Ende wird das Model gespeichert. Da aber das Widget FormFileUpload keinen $this->varValue definiert, bekommt das ModulePersonalData in diesem Fall keinen $objWidget->value geliefert.

Spricht irgendetwas dagegen, die Methode validate() in der FormFileUpload.php so zu ergänzen, dass die UUID der hochgeladenen Datei als $this->varValue abgelegt wird? Etwa so (Version Contao 3.2.13, FormFileUpload.php, ab Zeile 259):

// Add the session entry (see #6986)
$_SESSION['FILES'][$this->strName] = array
(
    'name'     => $file['name'],
    'type'     => $file['type'],
    'tmp_name' => TL_ROOT . '/' . $strFile,
    'error'    => $file['error'],
    'size'     => $file['size'],
    'uploaded' => true,
    'uuid'     => \String::binToUuid($objFile->uuid)
);

//Set the value
$this->varValue = $_SESSION['FILES'][$this->strName]['uuid'];

// Add a log entry
$this->log('File "'.$file['name'].'" has been moved to "'.$strUploadFolder.'"', __METHOD__, TL_FILES);
leofeyer commented 10 years ago

@contao/developers Wie seht ihr das? Und falls ja, ist das ein Bug oder ein Feature?

Toflar commented 10 years ago

Naja, ich hab das immer als "ist halt so" zur Kenntnis genommen ;-) Das Problem bei der vorgeschlagenen Lösung, ist dass die uuid nicht in jedem Fall da ist. Wenn die Datei nicht dem DBAFS hinzugefügt werden soll, wird sie keine uuid haben. Und was wenn ich nicht die uuid sondern size möchte? Dann muss ich ja trotzdem wieder

$size = $_SESSION['FILES'][$objWidget->name]['size'];

nutzen. Also wenn schon, dann würde ich erwarten, dass varValue das gesamte Array enthält.

aschempp commented 10 years ago

Siehe https://github.com/isotope/core/blob/master/system/modules/isotope/library/Isotope/CheckoutStep/OrderConditions.php#L66

Einfach so ins Dbafs und UUID geht nicht, weil die Anforderungen immer unterschiedlich sind. Die Datei muss ja gar nicht innerhalb Contao liegen.

@Toflar wie macht unser fineuploader das?

Toflar commented 10 years ago

Woher soll ich das wissen? Lies den Code doch selbst? :-)

discordier commented 10 years ago

Ich hab es bislang auch immer als "ist halt so" verwendet. Allgemein ist die gesamte Upload Thematik ein wenig "krampfhaft" zu verwenden.

leofeyer commented 10 years ago

Was ist denn mit @Toflar's Vorschlag, $_SESSION['FILES'][$objWidget->name] als $this->varValue zu setzen? Das halte ich für eine gute Lösung.

aschempp commented 10 years ago

Das löst das Problem nicht wirklich, weil das Upload-Widget z.B. auch $blnSubmitInput = false hat. Und das hat sicher seine Gründe. Beispielsweise liefert der Formular-Hook die Files als separates Array, das wäre ja dann gar nicht mehr nötig?

Ich glaube wenn wir das wirklich einführen wollen, dann in einem neuen Widget. Ein Upload-Feld das auch "state" behält (siehe auch #5486), das ggf. mehrere Dateien kann, das Fortschritt anzeigt etc. Im Backend haben wir ja bereits ein Uploader-Script, vielleicht kann das auch in einem Frontend-Widget eingesetzt werden?

PS: oder man nutzt Extensions ;-)