Erstellt vor 3 Jahren

Geschlossen vor 2 Jahren

#1954 closed Fehler (fixed)

CSV-Import benutzerdef. Variablen mit Großbuchstaben geht nicht

Erstellt von: grichardson@… Verantwortlicher: s.schoeling@…
Priorität: normal Meilenstein: 3.0.0
Komponente: kivitendo ERP Version: 2.7.0
Schweregrad: normal Stichworte: CSV-Import
Beobachter:

Beschreibung

Hat man eine benutzerdefinierte Variable für Waren mit Kurznamen "RABATT"
sollte man diese im CSV-Importer mit cvar_RABATT als Kopfzeile in der
Importdatei füllen können (wird auch so in der Hilfestellung angezeigt). Die
Zeile wird aber so nicht erkannt, heißt die Variable "rabatt" klappt es aber mit
csv_rabatt.

Ich sehe im Code Stellen die Header case insensitive zu machen, aber ich kenne
das Importmodul zu schlecht, ob das so nicht funktioniert oder an anderer
Stelle noch was geändert werden muß:

6833aa9af01409d28ba7cf605357850f81b85682
7d9888e3edac66a713dceae3e0bab34e788c039c

Was bei meinem Problem jedenfalls geholfen hat war am Ende von
sub _check_header in SL/Helper/Csv.pm das lowercase rauszunehmen, dann kann ich auch wieder csv_RABATT importieren:

-  return $self->header([ map { lc } @$header ]);
+  return $self->header([ @$header ]);

Anhänge (2)

test.diff (1.9 KB) - hinzugefügt von t.heck@… vor 3 Jahren.
csv_submodule_fix.diff (1.6 KB) - hinzugefügt von t.heck@… vor 3 Jahren.

Alle Anhänge herunterladen als: .zip

Änderungshistorie (10)

comment:1 Geändert vor 3 Jahren durch s.schoeling@…

Hmm. Mosu hat das implementiert weil Anwender doof sind, und einen Hang dazu haben Header nicht so zu schreiben, wie es in der Doku steht. Aber die Analyse ist korrekt, die header auf klein zu normalisieren ist für diesen Fall Gift.

Ich vermute, das wird wirklich wieder raus müssen. Von meiner Seite aus, kannst Du die beiden Commits reverten. Mosus Meinung wäre noch interessant, weil der den konkreten Fall hatte.

comment:2 Geändert vor 3 Jahren durch grichardson@…

Reverten möchte ich die Commits eigentlich nicht, zumindest im zweiten Commit von Sven passiert noch eine Menge mehr. Oder hat das alles damit zu tun und kann auch raus weil es nichts bringt?

comment:3 Geändert vor 3 Jahren durch s.schoeling@…

Das hat alles damit zu tun, ja, sollte aber teilweise erhalten bleiben. _check_header war in der ersten Iteration nur ein Accessor, hat dann aber durch die case insensitive Änderungen eine wirkliche check-Rolle bekommen. Deshalb musste der short circuit bei vorhandenem Header raus. Mittlerweile hat die Funktion aber auch noch die Funktion UTF8 Byte Order Marks zu entfernen, also muss das drin bleiben.

Ich schnack das nächste Woche mit Mosu ab, wenn er mal Zeit hat.

comment:4 Geändert vor 3 Jahren durch m.bunkus@…

Sven hat es erfasst: "implementiert, weil Menschen doof sind" ;) Natürlich können wir das lc herauswerfen und uns auf den Standpunkt "ja dann schreib es doch richtig!" zurückziehen. Solches strikte Verhalten ist allerdings eher benutzerunfreundlich.

Was haltet ihr von folgender Variante:

  1. Header für Nicht-CVAR-Spalten werden weiterhin in Kleinbuchstaben gewandelt.
  2. Header für CVAR-Spalten werden automatisch in die richtige Schreibweise gewandelt.

Der 2. ist leicht zu implementieren, meiner Meinung nach. Eigentlich könnte man 2. sogar für alle Spaltenheader implementieren. Pseudocode:

  foreach my $expected_column_header (@column_headers_as_they_should_be) {
    my $column_index = first { lc $row->[$_] eq lc $expected_column_header } (0..scalar(@$row));
    $row->[$column_index] = $expected_column_header if defined $column_index;
  }

comment:5 Geändert vor 3 Jahren durch m.bunkus@…

  • Status von new nach assigned geändert
  • Verantwortlicher von m.bunkus@… nach theck@… geändert

Geändert vor 3 Jahren durch t.heck@…

Geändert vor 3 Jahren durch t.heck@…

comment:6 Geändert vor 2 Jahren durch m.bunkus@…

  • Meilenstein auf 3.0.0 gesetzt

comment:7 Geändert vor 2 Jahren durch s.schoeling@…

  • Verantwortlicher von theck@… nach s.schoeling@… geändert

comment:8 Geändert vor 2 Jahren durch s.schoeling@…

  • Lösung auf behoben gesetzt
  • Status von assigned nach closed geändert

In a54fc3925496fe0cda2c6ba6d4a8d748d0035a50 und f01bf6359050e913b1907f1d9c5d197e7c8c2835 behoben.

Ich habe das jetzt so gemacht, dass:

  • Helper::Csv weiterhin keine ahnung von cvars haben muss
  • profile pfade können jetzt leer sein, und werden dann in die daten geparst, aber nicht ins objekt
  • diese profile Daten können dann abgeglichen werden mit den übergebenen Headerdaten.
  • Das ganze ist jetzt ein Flag im csv Helper.
  • SL::Controller::CsvImport::Base konsturiert jetzt ein profil, was die cvars enthält, und fragt strict_profile und case_insensitive_header an.

Den zweiten patch habe ich nicht eingespielt, weil das mittlerweile von Rose sichergestellt wird, dass sub_module nicht NULL ist.

Hinweis: Hilfe zur Verwendung von Tickets finden Sie in TracTickets.