Viele (select-)Logikfehler im Admin, wenn blVariantsSelection = true!?

Moin,

ich bin seit Tagen einer ganzen Reihe von Fehlern auf der Spur, und habe keine Ahnung, wie man das am besten als Bug reportet. Außerdem gibt es auch noch etwas Diskussionsbedarf. Im Kern betrifft es (fast) alle Listen-Zuordnungen (inc/popup) im Admin, wenn unter Grundeinstellungen/System/Varianten der erste Haken gesetzt ist.
Es lässt sich Demoshop gut nachvollziehen, da ausreichend Testdaten vorhanden sind, ansonsten (bei kleinen Shops) übersieht man die Problematik schnell. Es scheinen alle 4.4.x-Versionen seit mind. 4.4.3 betroffen zu sein, auch die aktuelle 4.4.8 (4.5. habe ich noch nie getestet).

Nehmen wir zuerst mal den Fall [B]manufacturer_main.inc.php[/B], also das Popup aus Stammdaten/Hersteller, wenn man dort einen gewählt hat (Artikel zuordnen):
Das überflüssige gruppieren in _addFilter() bewirkt den ersten Fehler, dass lange Listen (linke Seite) nicht gescrollt werden können, da der count(*)-Befehl dann Murks erzeugt, also etwas anderes zählt als die eigentliche Query!
Das kann man in diesem Fall recht einfach beheben, in dem man _addFilter() komplett auskommentiert bzw. löscht. Habe alle Fälle mehrfach durchdacht und getestet, und das “group by” ist wirklich nirgendwo sinnvoll. Sieht fast so aus, als wäre es nachträglich schnell reinkopiert worden, ohne es zu testen, aber egal, weiter im Text…

Der nächste Fehler ist die Reihenfolge der ersten Query (für die linke Seite ohne Kategorieauswahl) in _getQuery(). Dort ist nämlich auch das zweite AND abhängig von blVariantsSelection, was definitiv falsch ist, und deswegen eine unbrauchbare Liste erzeugt, in der auch Einträge stehen, die bereits zugeordnet wurden. Das verwirrt arg!

Als letztes stehen links immer auch alle Artikel, die evtl. aber bereits anderen Herstellern zugeordnet sind. Das bedeutet, dass man schnell auf den Button “alle zuordnen” kommen kann, und damit dann sämtliche bisher mühsam erstellten Herstellerbindungen mit einem Klick über den Haufen wirft. Dann noch ein Klick auf “alle Zuordnungen löschen”, und man hat rigoros aufgeräumt, sprich alle Bindungen komplett gelöscht, was meistens sicherlich nicht gewollt ist. Aber auch im Einzelfall bleibt es ärgerlich, da man ja vorher nicht sieht, ob ein Artikel bereits verknüpft ist, oder nicht.

Also hilft eigentlich nur eine verbesserte komplette _getQuery() und halt das Löschen der _addFilter(). Hier mal meine Version, die eigentlich schon prima funzt:

protected function _getQuery()
    {
        $myConfig = $this->getConfig();

        // looking for table/view
        $sArtTable = getViewName('oxarticles');
        $sO2CView  = getViewName('oxobject2category');

        $sOxid      = oxConfig::getParameter('oxid');
        $sSynchOxid = oxConfig::getParameter('synchoxid');
        $oDb = oxDb::getDb();

        // left side without selected category
        if ( !$sOxid ) {
            $sQAdd  = " FROM {$sArtTable} WHERE {$sArtTable}.oxshopid = ".$oDb->quote($myConfig->getShopId());
            $sQAdd .= " AND {$sArtTable}.oxmanufacturerid = ''";
            $sQAdd .= $myConfig->getConfigParam( 'blVariantsSelection' )
                    ? " AND ({$sArtTable}.oxparentid = '' OR (SELECT 1 FROM {$sArtTable} AS sub WHERE sub.oxid = {$sArtTable}.oxparentid AND sub.oxmanufacturerid = '')) "
                    : " AND {$sArtTable}.oxparentid = '' ";
        } else {
            // left side with selected category
            if ( $sSynchOxid && $sSynchOxid != $sOxid ) {
                $sQAdd  = " FROM {$sO2CView} LEFT JOIN {$sArtTable} ON";
                $sQAdd .= $myConfig->getConfigParam( 'blVariantsSelection' )
                        ? " ( {$sArtTable}.oxid = {$sO2CView}.oxobjectid OR {$sArtTable}.oxparentid = {$sO2CView}.oxobjectid )"
                        : " {$sArtTable}.oxid = {$sO2CView}.oxobjectid";
                $sQAdd .= " WHERE {$sArtTable}.oxshopid = ".$oDb->quote($myConfig->getShopId());
                $sQAdd .= " AND {$sO2CView}.oxcatnid = ".$oDb->quote($sOxid);
                $sQAdd .= " AND {$sArtTable}.oxmanufacturerid = ''";
                $sQAdd .= $myConfig->getConfigParam( 'blVariantsSelection' )
                        ? " AND ({$sArtTable}.oxparentid = '' OR (SELECT 1 FROM {$sArtTable} AS sub WHERE sub.oxid = {$sArtTable}.oxparentid AND sub.oxmanufacturerid = '')) "
                        : " AND {$sArtTable}.oxparentid = '' ";
            // right side
            } else {
                $sQAdd  = " FROM {$sArtTable} WHERE {$sArtTable}.oxshopid = ".$oDb->quote($myConfig->getShopId());
                $sQAdd .= $myConfig->getConfigParam( 'blVariantsSelection' )
                        ? " AND ({$sArtTable}.oxmanufacturerid = ".$oDb->quote($sOxid)." OR (SELECT 1 FROM {$sArtTable} AS sub WHERE sub.oxid = {$sArtTable}.oxparentid AND sub.oxmanufacturerid = ".$oDb->quote($sOxid)."))"
                        : " AND {$sArtTable}.oxmanufacturerid = ".$oDb->quote($sOxid)." AND {$sArtTable}.oxparentid = '' ";
            }
        }

        return $sQAdd;
    }

(ups, da war noch ein kleiner fehler, nun stimmts aber)

Der identische Sachverhalt gilt übrigens auch für [B]vendor_main.inc.php[/B].

Wenn man sich aber das Ergebnis anschaut, wird man evtl. Zweifel bekommen, ob es insgesamt überhaupt Sinn macht, bei Herstellern und Lieferanten die Varianten zu erlauben, da die Wahrscheinlichkeit einer nötigen Unterscheidung wohl gegen Null tendiert, dafür die Listen aber sehr unübersichtlich werden können. Aber das wäre fast wieder eine andere Debatte… :wink:

Was mich hauptsächlich wundert, ist, ob den ganzen Code eigentlich niemand getestet hat, denn es betrifft wie gesagt noch zig andere Listen-Popups, wie Bundles, Kategorien und watt weiß ich alles. Bin noch nicht ganz durch mit allem und brauche auch nicht alles für unseren ersten oxid-Shop. Aber es geht ja ums Prinzip. Wenn sogar “Datenverlust” droht, halte ich das Ganze für mehr als buggy, sondern lieblos angeflanscht ohne Sinn und Verstand. Will aber niemandem zu nahe treten, ist eine recht komplexe Materie und fällt evtl. selten auf.
Die andere Richtung der Zuordnungen, also von den Artikeln aus betrachtet, haut ja meistens hin, der Rest kam evtl. erst später dazu, kann das sein?

So, es wäre schön, etwas Resonanz von den “Machern” (Marco?) zu bekommen, bevor ich noch weiter aushole. Wäre schade, wenn meine Mühe umsonst gewesen wäre, und wie kann ich am ehesten und schnellsten erreichen, dass das Problem erkannt und ernst genommen, und der Sourcecode gefixt wird (evtl. durch meine Patches)? Die engl. Bugreports dümpeln scheinbar auch gerne mal länger vor sich hin, und werden dann auch wieder vergessen, ohne eine Lösung zu bewirkt zu haben.

Sorry für meine langen Threads, aber kürzer kann man das kaum fassen, und es war ja nur die “Spitze des Eisbergs”… :smiley:

Grüße!

Hallo Mitmacher,

boar… was für ein Pamphlet…
Offensichtlich hast Du schon mindestens einen Bug dazu reportet, der noch nicht angefasst wurde, weil wir momentan noch mit dieser 4.5.0 Dingens beschäftigt waren?
Wahrscheinlich hast Du auch Patches, während ich noch an einer CLA “rumgurke”?

Lass uns doch bitte einfach mal kurz telefonieren.

Gruß

Puh, durch die Veröffentlichung von 4.5.0 hat man nun doppelt Arbeit beim Testen… :frowning:

Aber egal, bin noch am Ball, da die Fehler in der neuen Version größtenteils immer noch vorhanden sind. Allerdings war meine Wortwahl evtl. etwas zu forsch (ernst nehmen tun wir uns doch alle, oder? :D) und außerdem gab es doch schon 2 Bugentries, vor allem [diesen].

Aber der steht auf resolved, was leider nicht stimmt. Einfach mal im Demoshop per Admin die Varianten-in-Listen-Option aktivieren und dann bei irgendeinem Hersteller die Artikelzuordnungen anschauen. Die kann man immer noch nicht scrollen, da auch da noch das überflüssige “group by” drinsteht. Meine weiteren Ausführungen gelten ebenso noch.

Wie also weiter? Wie mache ich z.b. den Bugissue #2460 wieder auf, oder wie geht man am besten vor? Erstmal trotzdem per Mailingliste nachhaken?