Status-Filter in /admin/..../customers_listing.php nicht richtig

[noRiddle]

In /admin/includes/modules/customers_listing.php gibt es folgenden Code:

if (isset($_GET['status']) && ($_GET['status'] != '100' || $_GET['status'] == '0')) {
    $status = xtc_db_prepare_input($_GET['status']);
    $search = "AND c.customers_status = '".$status."'";
}

Das

|| $_GET['status'] == '0'

erschließt sich mir nicht.

Da "Bitte wählen Sie" einen leeren value hat

value=''

müsste die if-Clause nach meiner Meinung so aussehen:

if (isset($_GET['status']) && $_GET['status'] != '100' && $_GET['status'] != '') {

Der Code wie er jetzt noch ist hat zur Folge, daß nach Wahl eines Status und Zurückstellen auf "Bitte wählen Sie" nur Admins angezeigt werden, was verwirrend ist.
Nur Admins weil der Wert 0 (= Admin-Status) in einem INT-Feld in der DB verglichen mit irgendeinem String (auch einem leeren) true ergibt (es sei denn der String ist als INT vorhanden).
Man sollte vielleicht das xtc_db_prepare_input() (wofür überhaupt stripslashes hier ?) entfernen, das ganze zu INT casten und die "single quotes" weglassen:

if (isset($_GET['status']) && $_GET['status'] != '100' && $_GET['status'] != '') {
    $status = (int)$_GET['status'];
    $search = "AND c.customers_status = ".$status;
}

Es kann ohnehin sein, daß in zukünftigen mySQL-Versionen das Vergleichen von Strings mit einem Wert in einem INT-Feld in der DB nicht mehr funktioniert.

Gruß,
noRiddle

[Gerhard Waldemair]

In 11231:

fix #1417

[Gerhard Waldemair]

In 11232:

fix #1417

[anonymous]

Im Changeset r11387 (​gepostet in diesem Thread) wurde aus folgendem Code (aus Shop-Version 2.0.4.2)

                if (isset($_GET['status']) && ($_GET['status'] != '100' || $_GET['status'] == '0')) {
                  $status = xtc_db_prepare_input($_GET['status']);
                  $search = "AND c.customers_status = '".$status."'";
                }

das gemacht

                if (isset($_GET['status']) && $_GET['status'] != '') {
                  $search = "AND c.customers_status = '".(int)$_GET['status']."'";
                }

Die Vereinfachung des Sichern des GET-Paraeters ist klar.
Ich bin jedoch der Meinung, daß die Abfrage nach != '100' drinbleiben muß, also so

                if (isset($_GET['status']) && $_GET['status'] != '100' && $_GET['status'] != '') { //corrected, noRiddle
                  $search = "AND c.customers_status = '".(int)$_GET['status']."'";
                }

weil die Query ansonsten mit

                AND customers_status = '100';

enden könnte, nämlich dann wenn man "alle Gruppen" wählt, und diese Query wird kein Ergebnis geben.

Wie bereits vorher gesagt bin ich weiterhin der Meinung, daß man die Anführungsstriche weglassen sollte weil das fehleranfällig ist (siehe meine vorigen Aussagen in diesem Ticket).
Wenn mit einem INT-Feld in der DB verglichen wird sollte man keinen String senden.

Gruß,
Oli (aka noRiddle)

[Torsten Riemer]

Nicht ganz richtig. Der Teil flog bereits in r11231 aus dem Code raus.

[Gerhard Waldemair]

Warum soll $_GETstatus denn 100 ergeben wenn man "alle Gruppen" wählt ?
Da wird der Wert auf also leer gesetzt.

[anonymous]

Der value für "Alle Gruppen" ist doch auf 100 gesetzt.
Entsprechender Ausschnitt aus der /admin/includes/modules/customers_listing.php:

        <?php echo xtc_draw_form('status', FILENAME_CUSTOMERS, '', 'get');
          $select_data = array ();
          $select_data = array (array ('id' => '', 'text' => TEXT_SELECT), array ('id' => '100', 'text' => TEXT_ALL_CUSTOMERS));
        ?>

GTB, probiere es doch einfach mal im Backend des Demo-Shops aus.
Ich sage es nochmals:
"Der Code wie er jetzt noch ist hat zur Folge, daß nach Wahl eines Status und Zurückstellen auf "Bitte wählen Sie" nur Admins angezeigt werden, was verwirrend ist.
Nur Admins weil der Wert 0 (= Admin-Status) in einem INT-Feld in der DB verglichen mit irgendeinem String (auch einem leeren) true ergibt (es sei denn der String ist als INT vorhanden)."

Also bitte das Ticket nicht schließen, bzw. wieder öffnen.

Ich sehe gerade, daß es im Entwickler-Demo-Shop gefixt zu sein scheint.
Da gibt es nach Auswählen einer Kundengruppe "Bitte auswählen" gar nicht mehr zur Auswahl.

Gruß,
noRiddle

[Torsten Riemer]

Das wurde mit r11231, r11232 korrigiert, dass nach Auswahl einer Kundengruppe nicht mehr "-- Bitte wählen Sie --" ausgewählt werden kann. In die Ausgangsansicht gelangt man durch Auswahl von "Alle Gruppen".

Das Ticket ist damit korrigiert.