Reihenfolge selektive Seitenstruktur bei Benutzern fehlerhaft

[ghost]

Ich habe bei einer frischen Installation von Contao 2.10 neue Benutzergruppen angelegt und selektive Pagemounts erstellt. Dabei ist mir aufgefallen das dabei die Reihenfolge der Strukturpunkte verloren geht. Um auszuschließen das dies nur in meiner Installation auftritt, habe ich in der Onlinedemo eine testsite ganz oben eingefügt und bei Pagemounts des User hinzugefügt. Bei User h.lewis selbst wird sie allerdings ganz unten angezeigt. Das liegt dann wohl an der Page ID. Sieht so aus als sortiert er danach und nicht nach der Struktur im Backend. Siehe Anhänge

Hier die Diskussion im Forum: http://www.contao-community.de/showthread.php?22684-Verdrehte-Seitenstruktur-bei-nicht-Adminbenutzer

Gruß
Frank

Download the attachments
Related issues: #2475

--- Originally created by FrankyMUC on August 31st, 2011, at 10:59am (ID 3423)

[leofeyer]

Behoben in 69de575.

--- Originally created on September 12th, 2011, at 05:00pm

[aschempp]

Wie kann ein array_reverse dieses Problem lösen? Ist das nicht eher Zufall?

--- Originally created on September 12th, 2011, at 05:22pm

[ghost]

$arrPages = array_reverse($arrPages); in die controller.php zu schreiben löst bei mir das Problem nicht wirklich. Struktur ist jetzt zwar verändert...aber die Reihenfolge wie bei einem Admin ist es nach wie vor nicht.

--- Originally created by FrankyMUC on September 12th, 2011, at 05:23pm

[ghost]

Testzugang kann ich gerne zur Verfügung stellen.

--- Originally created by FrankyMUC on September 12th, 2011, at 05:24pm

[leofeyer]

Wie kann ein array_reverse dieses Problem lösen? Ist das nicht eher Zufall?

Ich dachte, ein umgekehrtes Array wäre die Folge Deiner Änderung der eliminateNestedPages(). Die Zeile 2928 ist jedenfall die, die die Reihenfolge ändert. Und die ist von Dir :)

$arrPages = array_intersect($this->getChildRecords(0, $strTable), $arrPages);

--- Originally created on September 12th, 2011, at 06:25pm

[aschempp]

Das Problem ist, dass die Reihenfolge nicht mehr definiert ist, sondern "zufällig" bzw. nach Level. Ich arbeite bereits an einer Lösung ;-)

--- Originally created on September 12th, 2011, at 06:33pm

[aschempp]

Sooo... der angehängte Patch sollte das Problem beheben. Die angepasste Funktion erlaubt einen Mix aus der alten und neuen Variante. Es kann zwar (Optional) nach dem Sortierschlüssel der Datenbank sortiert werden, allerdings werden trotzdem nicht mehr so extrem viele Datenbankabfragen benötigt (es läuft in PHP). Ich hoffe das ist schneller...

--- Originally created on September 12th, 2011, at 06:50pm

[ghost]

Passt jetzt. Danke!

--- Originally created by Kahmoon on September 12th, 2011, at 07:12pm

[leofeyer]

Was genau machen die ganzen Schleifen und $arrByParent? Führt mein Patch eventuell zum selben Ergebnis?

--- Originally created on September 13th, 2011, at 09:55am

[aschempp]

Nein, leider wohl nicht. Mein Patch sortiert die gefundenen Child-Datensätze jeweils direkt nach den Parent-Datensätzen ein. Bei dir wird alles einfach zusammengeführt (array_merge()).
Die findInSet-Funktion der Database-Klasse kannte ich allerdings nicht, die liesse sich bei mir sicher einbauen.

--- Originally created on September 13th, 2011, at 10:50am

[leofeyer]

Dann bräuchte ich bitte mal ein Test-Szenario. Welche Seiten aus der Music Academy habt ihr der Benutzergruppe als Pagemout zugeordnet und in welcher (falschen) Reihenfolge werden diese angezeigt?

--- Originally created on September 13th, 2011, at 11:07am

[aschempp]

Für mich ist das Problem logisch... ich versuch es mal zu erklären:

• Dem DC_Table wird ein root-Array übergeben
• Wir können nicht sicher sein dass die Reihenfolge in diesem root-Array korrekt ist
• Die Funktion treeView() geht allerdings die Root-IDs in der vorgegebenen Reihenfolge durch
• *Ergo: wir müssen die Sortierung sicherstellen

Die Funktion eliminateNestedPages() sollte immer eine Sortierung sicherstellen, denn es kann keine "nested IDs" ohne Sortierung geben (daher Patch in DC_Table unnötig).

Der erste Aufruf der getChildRecords() in eliminateNestedPages() wird als ID-Liste weiterverwendet, danach werden die ungültigen IDs entfernt

Ergo: Der erste Aufruf für getChildRecords() muss sicherstellen, dass die Sortierung der IDs dem Baum entspricht.

Falls es noch nicht klar ist, kannst du mich gerne auf Skype kontaktieren ;-)

--- Originally created on September 13th, 2011, at 11:22am

[leofeyer]

Ich stimme Dir fast zu, bis auf

Wir können nicht sicher sein dass die Reihenfolge in diesem root-Array korrekt ist

Wir können sehr wohl sicher sein, dass die Reihenfolge stimmt, denn sie wird ja über das PageTree-Widget festgelegt und das ist wie die Seitenstruktur aufgebaut.

--- Originally created on September 13th, 2011, at 11:33am

[aschempp]

Das habe ich mich auch gefragt, warum es bei Franky falsch ist.
Allerdings können wir nicht unbedingt sicher sein, das Root-Array kann auch über das DCA gesetzt werden...

--- Originally created on September 13th, 2011, at 11:34am

[leofeyer]

Was dann die Eigenverantwortung des Entwicklers wäre. Immerhin kostet die zusätzliche Sortierung Zeit und Ressourcen - und das bei einer rekursiven Funktion. Nicht optimal …

--- Originally created on September 13th, 2011, at 11:35am

[aschempp]

Aber ohne diese Funktion gibt es ja auch keine Möglichkeit, die IDs in korrekter Reihenfolge zu bekommen? Beispiel ich möchte per Code eine zusätzliche ID zum PageMount des Benutzers hinzufügen.

--- Originally created on September 13th, 2011, at 11:37am

[ghost]

Aktuell habe ich beide gepatchten PHP Files bei meiner Installation laufen und es sieht alles gut aus. Die Seite ist noch nicht produktiv. Benutzer mit einzelnen Pagemounts wären angelegt. Testzugang + FTP Zugang kann ich per PN im Forum schicken -> Kahmoon.

--- Originally created by FrankyMUC on September 13th, 2011, at 11:49am

[aschempp]

Was meinst du mit "beide gepatchten PHP Files"?

--- Originally created on September 13th, 2011, at 11:56am

[ghost]

Ich hab die Controller.php und DC_Table.php mit euren Änderungen angepasst.

--- Originally created by FrankyMUC on September 13th, 2011, at 11:59am

[aschempp]

"euren"? Es gibt eine Variante von Leo und eine von mir, kombinieren lassen sich diese nicht wirklich...

--- Originally created on September 13th, 2011, at 12:03pm

[ghost]

Deine Variante

http://dev.contao.org/attachments/1124/Controller.php.patch

und die von LEO für DC_Table.php

http://dev.contao.org/attachments/1126/DC_Table.php.patch

--- Originally created by FrankyMUC on September 13th, 2011, at 12:05pm

[aschempp]

Ah, verstehe. Die von Leo für DC_Table hat mit meinem Controller-Patch keine Auswirkung ;-)

--- Originally created on September 13th, 2011, at 12:06pm

[leofeyer]

Aber ohne diese Funktion gibt es ja auch keine Möglichkeit, die IDs in korrekter Reihenfolge zu bekommen? Beispiel ich möchte per Code eine zusätzliche ID zum PageMount des Benutzers hinzufügen.

Theoretisch könntest Du die neue ID mittels array_insert an die gewünschte Position bringen. Es ist halt die Frage, ob wir für so einen spezielleren Fall den ganzen Overhead in Kauf nehmen wollen oder nicht. Und ich bin mit einem Blick auf die Geschwindigkeit eher dagegen.

--- Originally created on September 13th, 2011, at 12:26pm

[aschempp]

das kann ich nur in meinem eigenen Modul, wenn ich meine Version der getChildRecords nachbaue, oder? Ich meine, man sollte die Option zur Verfügung stellen im Core, ob eliminateNestedPages sie dann nutzt ist eine andere Frage.

--- Originally created on September 13th, 2011, at 12:31pm

[leofeyer]

das kann ich nur in meinem eigenen Modul, wenn ich meine Version der getChildRecords nachbaue, oder?

Was genau meinst Du jetzt?

--- Originally created on September 13th, 2011, at 12:48pm

[aschempp]

Wenn ich beispielsweise dem PageMount-Array meine eigenen Einträge hinzufügen möchte, muss ich ja eine Möglichkeit haben diese korrekt zu sortieren. Das müsste ich dann selber bauen, wenn die entsprechende Funktion im Core nicht vorhanden ist.

--- Originally created on September 13th, 2011, at 01:17pm

[leofeyer]

Ja, das müsstest Du. Aber wie oft kommt das wirklich vor?

--- Originally created on September 13th, 2011, at 01:23pm

[aschempp]

Das kann ich nicht sagen, aber es schadet nicht diese Funktion zu haben, oder? Wenn der Core sie nicht benutzt, besteht auch kein Performance-Problem.

--- Originally created on September 13th, 2011, at 01:31pm

[ghost]

Controller::eliminateNestedPages():
Anstatt:

$arrPages = array_intersect($this->getChildRecords(0, $strTable), $arrPages);
$arrPages = array_values(array_diff($arrPages, $this->getChildRecords($arrPages, $strTable)));

Das hier:

$arrPages = array_intersect($arrPages, $this->getChildRecords(0, $strTable));
$arrPages = array_keys(array_flip(array_diff($arrPages, $this->getChildRecords($arrPages, $strTable))));

Hält die Sortierung von $arrPages aufrecht. (Ich rede hier nicht von aufeinanderfolgenden Index-Keys)
Vielleicht kann man auch weiterhin array_values nehmen, aber da ist die PHP doc etwas blöd.

Sofern dann $arrPages die richtige Reihenfolge hat, passt das.
Übrigens kann dieses Problem, dass die Einstellung der Pagemounts nicht richtig sortiert ist, ganz einfach herbei geführt werden:

1. Pagemounts auswählen
2. Admin verändert Sortierung der Seiten die Pagemounts sind

Also muss man die Sortierung bei jedem Laden der DC_Table gewährleisten.

Sofern ->getChildRecords die Datensätze in Preorder zurückgibt, kann man auch weiterhin folgendes nehmen:

array_intersect($this->getChildRecords(0, $strTable), $arrPages);

Aber bisher gibt ->getChildRecords in Levelorder zurück.

Ok Leo Patch führt die Preorder in ->getChildRecords ein, und das sollte auch ziemlich performant sein.

@andreas: Wenn du dynamisch etwas richtig sortiert hinzufügen willst machst du das einfach so mit leos variante:
($this muss ein Controller sein)

$objDC->roots[] = $myOwnID;
$objDC->roots = $this->eliminateNestedPages($objDC->roots, $objDC->table, true);

Das sollte auch schnell sein, da alle Querys gecacht sind.

--- Originally created by backbone on September 13th, 2011, at 01:33pm

[aschempp]

Danke Oli, das mit dem Umsortieren nach dem Speichern der PageMounts war der richtige Tipp. Wir können also nicht sicher sein, dass die Reihenfolge stimmt...
Deine Anpassung würde das Problem beheben, wenn die Reihenfolge von $arrPages korrekt wäre - was sie aber zwingend nicht ist. Wir brauchen also trotzdem meinem Patch :D

--- Originally created on September 13th, 2011, at 01:41pm

[leofeyer]

array_keys(array_flip

Wo genau ist der Unterschied zu array_values?

--- Originally created on September 13th, 2011, at 01:56pm

[ghost]

`leo:
Die Doku von array_values macht irgendwelche komischen aussagen über die Sortierung, array_keys+array_flip erhält die sortierung definitiv (wende ich bei bei meiner navi erweiterung an). Ich hab array_values allerdings noch nicht getestet.

andreas: Leos Patch sollte auch funktionieren, da die ChildRecords in preorder zurückgegeben werden (sofern $blnSorting = true). Und$arrPages = array_intersect($arrChildRecordsDerKomplettenSeitenstrukturInPreorder, $arrPages);` übernimmt dann die Sortierung der ChildRecords.

--- Originally created by backbone on September 13th, 2011, at 02:44pm

[aschempp]

Was meinst du mit "preorder"? Die Sortierung erfolgt nach pid und sorting, aber das bezieht sich ja nicht auf alle Level.

--- Originally created on September 13th, 2011, at 09:19pm

[ghost]

Preorder ist eine Ausgabevariante der Tiefensuche:
http://de.wikipedia.org/wiki/Tiefensuche
Das ist im Prinzip genau die Reihenfolge, wie sie in der Seitenstruktur auftritt.

Und ich hab nochmal Leos Lösung angeschaut angeschaut, das passt leider nicht.
Bei folgendem Baum:

0
|-1
| |-2
| `-3
`-4
  |-5
  `-6

Gibt Leos Variante anstatt: 0, 1, 2, 3, 4, 5, 6 folgendes aus: 0, 1, 4, 2, 3, 5, 6 (Was die sogenannte Level-order ist)

--- Originally created by backbone on September 13th, 2011, at 10:56pm

[aschempp]

Ja das meinte ich eben auch. Aber mit meiner sollte es funktionieren :-)

--- Originally created on September 14th, 2011, at 01:01pm

[leofeyer]

Behoben in 86393f8.

--- Originally created on November 3rd, 2011, at 02:29pm

[ghost]

Ich möchte hier nochmal drauf hinweisen, dass der Algorithmus nicht sehr effizient ist. Du machst für JEDE Seite ein Query, das ist der Overkill, vorallem da die Querys durch das FIND_IN_SET-Zeug extrem lang werden.

Bitte schau dir mal meine Erweiterung backboneit_navigation an und da in das Navigationsmodul, da hab ich einen iterativen Algorithmus, der nur einmal für jede Ebene abfragt und das ohne diesen ganzen FIND_IN_SET-Kram. Ich schau mal ob ich dir dafür einen Patch zusammenbauen kann.

--- Originally created by backbone on November 3rd, 2011, at 03:07pm

[leofeyer]

Du machst für JEDE Seite ein Query

Schau Dir den Code bitte noch mal genau an.

--- Originally created on November 3rd, 2011, at 09:22pm

[ghost]

Ich nehm alles zurück und behaupte das Gegenteil ;)
Ok, die Query ist i.O. bis auf das FIND_IN_SET, die Nachbearbeitung mit dem array_search/reverse/insert ist aber net so prickelnd, das muss einfacher gehen ;) (die Kritiken beziehen sich ausschließlich auf die Performance, aber wenns jetzt erstmal so tut ist gut)

--- Originally created by backbone on November 4th, 2011, at 01:56pm

[leofeyer]

FIND_IN_SET() brauchen wir aber, damit die Reihenfolge beibehalten wird. Wenn Du eine effektivere Lösung hast, bauen wir diese natürlich gerne alternativ ein :)

--- Originally created on November 4th, 2011, at 02:19pm

[ghost]

Ja, eine effektivere Lösung habe ich in meinem Navigationsmodul umgesetzt. Da werden auch die Kindknoten lediglich Anhand der Spalte "sorting" sortiert und dann in PHP der jeweiligen Parent-ID zugeordnet. Das ist wesentlich effizienter als das FIND_IN_SET da dort im Prinzip alles mit schnellen array-index Zugriffen durchgeführt wird.
Zu finden ist das Ganze in backboneit_navigation in der Methode ModuleNavigationMenu::fetchItems(), da sind natürlich noch die ganzen Checks drin, die für die Navigation relevant sind, drin. Die kannst du einfach ignorieren. Auch kannst du das Level zeug ignorieren, da ja der komplette Teilbaum geholt werden soll (es sei denn man bohrt die getChildRecords-Methode noch etwas auf um Level-Beschränkungen zu berücksichtigen).

--- Originally created by backbone on November 4th, 2011, at 05:55pm

[leofeyer]

Wie hoch stehen unsere Chancen, dass Du einen Patch auf Basis des aktuellen Branches bereitstellst? :)

--- Originally created on November 4th, 2011, at 07:58pm

[ghost]

Ich setz mich heut abend mal dran.

--- Originally created by backbone on November 4th, 2011, at 07:59pm

[ghost]

Siehe Patch (Nur Controller).
In der DC_Table musst du dann die neue Funktion Controller::getPreorder() verwenden (mit $blnUnnest = true).
Die vorhandenen Methoden eliminateNestedPages & getChildRecords delegieren an die neuen Tree-Methoden.
Es wär vielleicht sogar mal hier ein guter Start um die neuen Methoden in eine eigene statische Lib zu packen. Mein Namens-Vorschlag wär "Tree".

Edit: Der diff-View sieht etwas unglücklich aus, aber es sind im Prinzip neue Methoden und die vorhandenen Methoden die bearbeitet wurden sind nur noch Delegates. Zu beachten ist hier das eliminateNestedPages die Input-Order der Nodes beibehält, das sollte mehr backwards-kompatibel sein.
Auch hab ich den Patch noch nicht getestet, aber es sind im Prinzip die gleichen Algorithmen wie in meinem Navi-Modul.

--- Originally created by backbone on November 5th, 2011, at 12:18am

[leofeyer]

--- Originally completed on November 3rd, 2011, at 02:29pm