#2221 closed Aufgabe (fixed)
Anzahl der angezeigten Artikel in products_new.php nicht schlüssig
| Reported by: | noRiddle | Owned by: | somebody |
|---|---|---|---|
| Priority: | normal | Milestone: | modified-shop-2.0.8.0 |
| Component: | Shop | Version: | 2.0.7.0 |
| Keywords: | Cc: | ||
| Blocked By: | Blocking: |
Description
In der products_new.php ("Neue Artikel") werden wenn es keine Produkte gibt die innerhalb des Zeitfensters liegen welches bei
Konfiguration => Maximum Werte => "Anzahl der Tage für Neue Produkte"
angegeben ist alle Produkte angezeigt.
Das ist doch sicher nicht so gewollt. (Das war auch in der 2.0.6.0 bereits so.)
Wenn man lange keine neuen Produkte eingepflegt hat muß man den Konfigurations-Wert unnatürlich hoch setzen damit nicht alle Produkte die im Shop aktiv sind als "Neue Artikel" angezeigt werden.
Das macht so wenig Sinn.
Das liegt an diesem Code in der /includes/modules/default.php:
if (basename($PHP_SELF) == FILENAME_PRODUCTS_NEW) {
if (MAX_DISPLAY_NEW_PRODUCTS_DAYS != '0'
&& check_whatsnew() === true
)
{
$date_new_products = date("Y-m-d", mktime(1, 1, 1, date("m"), date("d") - MAX_DISPLAY_NEW_PRODUCTS_DAYS, date("Y")));
$where .= " AND p.products_date_added > '".$date_new_products."' ";
$daysfound = true;
}
$sorting = ' ORDER BY p.products_date_added DESC ';
}
Das gilt auch für die Box whats_new.php, wo der Auswahl-Pool unter den eingangs beschriebenen Bedingungen alle aktiven Artikel des Shops beinhaltet, was bei sehr vielen Artikeln im Shop und gerade bei Verwendung der random Auswahl in der Box die Performance gravierend in den Keller fallen lässt. (Bei meinen Test und etwas über 200.000 Artikeln im Shop benötigt die Query für die Box über 26 Sek..)
Ich denke es wäre zu überlegen gar keine Artikel als "Neue Artikel" anzeigen zu lassen wenn die Bedingung mit dem eingangs genannten Zeitfenster nicht zutrifft.
Gruß,
noRiddle
Attachments (0)
Change History (10)
comment:1 by , 4 years ago
| Milestone: | → modified-shop-2.0.7.1 |
|---|
comment:2 by , 4 years ago
comment:3 by , 4 years ago
Damit alles erfasst wird:
Auch im listing_filter wird für products_new.php die Logik benutzt.
Evtl. sollte man die Routine mit dem eingangs zitierten Code, der so oder so ähnlich in allen aufgeführten Files vorkommt, in eine Funktion auslagern, damit man eine einzige Stelle hat die das Vorgehen definiert.
Gruß,
noRiddle
comment:4 by , 4 years ago
Dieses Verhalten gibt es schon seit der 1.0x Version.
Aus meiner Sicht ist es das Schlimmste, eine Seite ohne Ergebnisse anzuzeigen. Deshalb ist diese Routine damals eingeflossen. Bin aber hier gerne dazu bereit was neues zu machen.
Gruss Gerhard
comment:5 by , 4 years ago
Jau, hast Recht, soweit zurück habe ich gar nicht überprüft.
Ich denke wir sollten da was machen, überlege mir auch einen Vorschlag...
Gruß,
noRiddle
comment:6 by , 4 years ago
| Milestone: | modified-shop-2.0.7.1 → modified-shop-2.0.8.0 |
|---|
comment:7 by , 3 years ago
Wie noRiddle richtig schreibt, wird der im Ticket genannte Code an mehreren Stellen genutzt.
Diese Dateien können aus meiner Sicht vernachlässigt werden, da dort in den SQL-Querys Limits gesetzt werden.
- includes/extra/default/center_modules/new_products_default.php
- includes/modules/new_products.php
- templates/tpl_name/source/boxes/whats_new.php
In diesen Dateien werden keine Limits gesetzt, weshalb alle aktiven Artikel als "Neue" ausgegeben werden.
- includes/modules/default.php
- includes/modules/listing_filter.php
Ein Limit für die Ausgabe kann nach meiner Meinung in der "class splitPageResults" gesetzt werden.
Als maximalen Anzeigewert habe ich im folgenden Vorschlag die Konstante "MAX_DISPLAY_NEW_PRODUCTS" genutzt.
Mein Vorschlag:
In der Datei includes/classes/split_page_results.php die Zeile
$this->sql_query .= " LIMIT " . max((int)$offset, 0) . ", " . $this->number_of_rows_per_page;
mit diesem Block ersetzen
$next_offset = 0;
if (basename($_SERVER['PHP_SELF']) == FILENAME_PRODUCTS_NEW && check_whatsnew() !== true) {
if ($this->number_of_rows > MAX_DISPLAY_NEW_PRODUCTS) $this->number_of_rows = MAX_DISPLAY_NEW_PRODUCTS;
$next_offset = $this->number_of_rows_per_page * $this->current_page_number;
}
if ($next_offset > $this->number_of_rows) {
$this->sql_query .= " LIMIT " . max((int)$offset, 0) . ", " . ($this->number_of_rows - $offset);
} else {
$this->sql_query .= " LIMIT " . max((int)$offset, 0) . ", " . $this->number_of_rows_per_page;
}
Durch diese Änderung wird die Anzeige "Neue Artikel" begrenzt.
Gruß Karl
comment:8 by , 3 years ago
Habe gestern vor dem Schreiben den Code nochmals verändert und nicht aufmerksam genug getestet.
Bei obigem Vorschlag gibt es Fehler bei der Berechnung der Gesamtzahl an Seiten.
Hier nochmal mein korrigierter Code-Vorschlag, diesmal der komplette Konstruktor.
// class constructor
function __construct($query, $page, $max_rows, $count_key = '*') {
$this->sql_query = $query;
if (empty($page) || !is_numeric($page) || $page < 0) $page = 1;
$this->current_page_number = $page;
$this->number_of_rows_per_page = $max_rows;
$pos_to = strlen($this->sql_query);
$pos_from = strpos(strtoupper($this->sql_query), ' FROM', 0);
$pos_group_by = strpos(strtoupper($this->sql_query), ' GROUP BY', $pos_from);
if (($pos_group_by < $pos_to) && ($pos_group_by !== false)) $pos_to = $pos_group_by;
$pos_having = strpos(strtoupper($this->sql_query), ' HAVING', $pos_from);
if (($pos_having < $pos_to) && ($pos_having !== false)) $pos_to = $pos_having;
$pos_order_by = strpos(strtoupper($this->sql_query), ' ORDER BY', $pos_from);
if (($pos_order_by < $pos_to) && ($pos_order_by !== false)) $pos_to = $pos_order_by;
if (strpos(strtoupper($this->sql_query), 'DISTINCT') !== false || strpos(strtoupper($this->sql_query), 'GROUP BY') !== false) {
$count_string = 'DISTINCT ' . xtc_db_input($count_key);
} else {
$count_string = xtc_db_input($count_key);
}
$count_query = xtDBquery("SELECT count(" . $count_string . ") as total " . substr($query, $pos_from, ($pos_to - $pos_from)));
$count = xtc_db_fetch_array($count_query, true);
$this->number_of_rows = $count['total'];
if (basename($_SERVER['PHP_SELF']) == FILENAME_PRODUCTS_NEW && check_whatsnew() !== true) {
if ($this->number_of_rows > MAX_DISPLAY_NEW_PRODUCTS) $this->number_of_rows = MAX_DISPLAY_NEW_PRODUCTS;
}
if ($this->number_of_rows_per_page > 0) {
$this->number_of_pages = ceil($this->number_of_rows / $this->number_of_rows_per_page);
} else {
$this->number_of_pages = 0;
}
if ($this->current_page_number > $this->number_of_pages) {
$this->current_page_number = $this->number_of_pages;
}
$offset = ($this->number_of_rows_per_page * ($this->current_page_number - 1));
if ($offset < 1) $offset = 0;
$next_offset = 0;
if (basename($_SERVER['PHP_SELF']) == FILENAME_PRODUCTS_NEW && check_whatsnew() !== true) {
$next_offset = $this->number_of_rows_per_page * $this->current_page_number;
}
if ($next_offset > $this->number_of_rows) {
$this->sql_query .= " LIMIT " . max((int)$offset, 0) . ", " . ($this->number_of_rows - $offset);
} else {
$this->sql_query .= " LIMIT " . max((int)$offset, 0) . ", " . $this->number_of_rows_per_page;
}
}

Das oben Dargelegte gilt übrigens auch für die Startseitenartikel (new_product_default.php).
Wenn nicht explizit Artikel auf die Startseite verlinkt sind werden zufällig Artikel aus der DB geholt nach MAßgabe
Konfiguration => Maximum Werte => "Neue Artikel Anzeigemodul"
und ebenfalls
Konfiguration => Maximum Werte => "Anzahl der Tage für Neue Produkte"
Auch hier wird der Auswahl-Pool auf alle aktiven Artikel erweitert wenn das Zeitfenster nicht greift.
Gruß,
noRiddle