anmerkung: contenido-cvs-2005-06-24.tar

[HerrB]

Na, da ist noch einiges drin.

contenido\includes\include.frontend.user_menu.php:

cInclude("classes", "class.todo.php"); wird IMHO nicht benötigt. $oPage = new cPage; wird zweimal eingesetzt (eins kann entfallen).

Genereller Vorschlag: Statt

Code: Alles auswählen

$aFieldsToSearch = array("--all--" => i18n("-- All fields --"), "username" => i18n("Username"));

nur

Code: Alles auswählen

$aFieldsToSearch = array("all" => i18n("All"), "username" => i18n("Username"));

Warum? Weil dann die Felder kleiner werden und man sie mit ein wenig mehr Optimierung auch verwenden kann, ohne den Frame jedesmal anpassen zu müssen (gilt natürlich auch für andere Felder/Beschriftungen/Übersetzungen und Breite von Feldern). Generell gilt auch, dass die übergebenen Werte nicht überprüft werden (e.g. elemperpage, page usw.) - wenigstens eine Überprüfung auf numerische Werte würde die Sache stabiler machen.

In der Schleife while ($feuser = $oFEUsers->next()) wird mehrfach
$feuser->get("idfrontenduser") verwendet. Da bei jedem get ein DB-Aufruf erfolgt, ist das nicht sehr performant, der Wert sollte am Anfang der Schleife in eine Variable gespeichert werden.

Ähm:

Code: Alles auswählen

$oSelectItemsPerPage->autoFill(array(25 => 25, 50 => 50, 75 => 75, 100 => 100));

if (!isset($_REQUEST["elemperpage"]))
{
	$oSelectItemsPerPage->setDefault(2);
} else {
...

Ich bin da PHP-Array-Laie, aber ich würde zu setDefault(25) tendieren.

Für dieses Konstrukt bitte ich um Erläuterung, wie zusätzliche Felder eingebaut werden können:

Code: Alles auswählen

$aFieldSources = array();
$aFieldSources["username"] = "base"; 
...
while ($feuser = $oFEUsers->next())
{
	foreach ($aFieldSources as $key => $field)
	{
		switch ($field)
		{
			case "base":
				$aUserTable[$feuser->get("idfrontenduser")][$key] = $feuser->get("username");
				break;	
			default:
				$aUserTable[$feuser->get("idfrontenduser")][$key] = call_user_func("frontendusers_".$field."_getvalue", $key);
				break;
...

Ich gehe davon aus, dass man mit $aFieldSources["spaltenname"] = ""; weitere Spalten auswählen könnte - dafür braucht man dann aber callback-Funktionen, bei denen ich nicht weiss, wie bzw. wo die definiert werden. Ich nutze das mal für konstruktive Kritik: Muss es wirklich so kompliziert sein? Reicht nicht auch ein

Code: Alles auswählen

		switch ($field)
		{
			case "base":
				$aUserTable[$feuser->get("idfrontenduser")][$key] = $feuser->get("username");
				break;	
			default:
				$aUserTable[$feuser->get("idfrontenduser")][$key] = $feuser->get($key);
				break;
...

Die Zeile produziert im Endeffekt Murks:

Code: Alles auswählen

$aUserTable = array_csort($aUserTable, $_REQUEST["sortby"], $sortorder);

Problem: Wird die Seite das erste Mal aufgerufen, stimmen die IDs, die in den Links, mit denen die FrontendUser-Namen hinterlegt sind, mit den jeweiligen FrontendUser-IDs überein. Ein Klick auf "Apply" setzt statt den DB-IDs fortlaufende IDs (schön daran zu erkennen, dass das erste Element die ID 0 erhält). Das Array enthält als Schlüssel die DB-ID - nach dem Umsortieren (welches erst nach Apply das erste Mal durchgeführt wird) ist diese Information verloren und als ID liefert das Array die aktuelle Zeigerposition im Array (0, 1, 2, ...). Damit scheitert natürlich schon der Aufruf eines Elements durch Anklicken.

Auch hier konstruktive Kritik: mySQL und alle anderen DBs sind auf die Sortierung von Werten optimiert (und ORDER BY ist ein SQL92-Standard). Ich wäre dafür, die DB-Sortierungsfähigkeiten zu nutzen (Datumseinträge werden z.B. mit array_csort auch garantiert nicht "richtig" sortiert). Das gleiche gilt für das Filtern von Werten. Dazu später mehr.

Das Löschen von Einträgen funktioniert nicht (gilt überall, wo FolderRow eingesetzt wird, also auch http://www.contenido.org/forum/viewtopi ... 0381#50381, Fehlermeldung DEDE). Ansonsten wird der Code in $deleteScript z.Z. kaum genutzt. Wenn, wäre der Funktionsname deleteModule falsch und außerdem enthält die Funktion die Zeile

Code: Alles auswählen

			url += \'&page=\' +form.page.value;

. Im aktuellen Code steht jedoch kein Form-Feld mit Namen page zur Verfügung (zumindest, wenn nur eine Seite zur Verfügung steht, habe es mit mehr Seiten noch nicht getestet. Damit vermute ich aber, dass das Blättern auch nicht funktioniert).

Da ich persönlich ein Fan der JS-messageBox und kein Freund der Standard-confirm-Box bin, würde ich die nächsten Tagen einen funktionierenden Code zur Verfügung stellen. Das würde dann auch ein Code zum Sortieren und Filtern enthalten (der Code zum Filtern in include.rights_menu.php ist mir zu kompliziert).

Hierzu habe ich noch eine Frage:

Code: Alles auswählen

$oListActionRow = new cFoldingRow("339b79d1-48f7-4ac6-ba17-b958c5b3bb2b",i18n("Actions"));

Woher kommt die GUID? Bzw. ich benötige eine weitere GUID für einen Bereich "Options".

Gruß
HerrB

[timo]

$oPage = new cPage; wird zweimal eingesetzt (eins kann entfallen).

Eins wurde entfernt.

Genereller Vorschlag: Statt

Code: Alles auswählen

$aFieldsToSearch = array("--all--" => i18n("-- All fields --"), "username" => i18n("Username"));

nur

Code: Alles auswählen

$aFieldsToSearch = array("all" => i18n("All"), "username" => i18n("Username"));

Warum? Weil dann die Felder kleiner werden und man sie mit ein wenig mehr Optimierung auch verwenden kann, ohne den Frame jedesmal anpassen zu müssen (gilt natürlich auch für andere Felder/Beschriftungen/Übersetzungen und Breite von Feldern).

Naja, generell sollte der linke Frame etwas größer ausfallen. Langsam aber sicher kommt er an seine Grenze...

Generell gilt auch, dass die übergebenen Werte nicht überprüft werden (e.g. elemperpage, page usw.) - wenigstens eine Überprüfung auf numerische Werte würde die Sache stabiler machen.

Wieso? Die Werte können nur von diesem einen Frame kommen. Klar kann man überall Prüfungen einbauen, nur der Aufwand wäre gegenüber dem Nutzen nicht gerechtfertigt...denn wenn man es hier macht, müsste man es überall anders auch machen.

In der Schleife while ($feuser = $oFEUsers->next()) wird mehrfach
$feuser->get("idfrontenduser") verwendet. Da bei jedem get ein DB-Aufruf erfolgt, ist das nicht sehr performant, der Wert sollte am Anfang der Schleife in eine Variable gespeichert werden.

Nein, das ist nicht korrekt. Ein get verursacht keinen Datenbankaufruf, sondern greift auf die vorher geladenen Werte innerhalb der GenericDB zurück.

Ähm:

Code: Alles auswählen

$oSelectItemsPerPage->autoFill(array(25 => 25, 50 => 50, 75 => 75, 100 => 100));

if (!isset($_REQUEST["elemperpage"]))
{
	$oSelectItemsPerPage->setDefault(2);
} else {
...

Ich bin da PHP-Array-Laie, aber ich würde zu setDefault(25) tendieren.

Korrekt Da hing wohl die "5" auf der Tastatur.

Für dieses Konstrukt bitte ich um Erläuterung, wie zusätzliche Felder eingebaut werden können:

Code: Alles auswählen

$aFieldSources = array();
$aFieldSources["username"] = "base"; 
...
while ($feuser = $oFEUsers->next())
{
	foreach ($aFieldSources as $key => $field)
	{
		switch ($field)
		{
			case "base":
				$aUserTable[$feuser->get("idfrontenduser")][$key] = $feuser->get("username");
				break;	
			default:
				$aUserTable[$feuser->get("idfrontenduser")][$key] = call_user_func("frontendusers_".$field."_getvalue", $key);
				break;
...

Ich gehe davon aus, dass man mit $aFieldSources["spaltenname"] = ""; weitere Spalten auswählen könnte - dafür braucht man dann aber callback-Funktionen, bei denen ich nicht weiss, wie bzw. wo die definiert werden. Ich nutze das mal für konstruktive Kritik: Muss es wirklich so kompliziert sein? Reicht nicht auch ein

Code: Alles auswählen

		switch ($field)
		{
			case "base":
				$aUserTable[$feuser->get("idfrontenduser")][$key] = $feuser->get("username");
				break;	
			default:
				$aUserTable[$feuser->get("idfrontenduser")][$key] = $feuser->get($key);
				break;
...

Ja, anders macht es keinen Sinn. Da die Frontendbenutzer über Plugins erweitert werden, macht es durchaus Sinn, im Plugin selbst einen Callback aufzurufen. Das ist die einfachste und sauberste Methode. Beispielcode für ein AIM Plugin:

Code: Alles auswählen

<?php
/*****************************************
* File      :   $RCSfile: aim.php,v $
* Project   :   Contenido
* Descr     :   Example plugin for adding an aim address to a frontend user
* Modified  :   $Date: 2004/01/14 16:05:35 $
*
* © four for business AG, www.4fb.de
*
* $Id: aim.php,v 1.1 2004/01/14 16:05:35 timo.hummel Exp $
******************************************/

function frontendusers_aim_getTitle ()
{
	return i18n("AOL AIM Screen Name", "frontendusers_aim");	
}

function frontendusers_aim_display ()
{
	global $feuser;
	
	$aim = new cHTMLTextbox("aim", $feuser->getProperty("aim", "address"));
	return $aim->render();	
}

function frontendusers_aim_wantedVariables ()
{
	return (array("aim"));	
}

function frontendusers_aim_canonicalVariables()
{
	return (array("aim" => i18n("AOL AIM Screen Name", "frontendusers_aim")));
}

function frontendusers_aim_store ($variables)
{
	global $feuser;
	
	if (!array_key_exists("aim",$variables))
	{
		return false;	
	} else {
		$feuser->setProperty("aim", "address", $variables["aim"]);
		return true;
	}
}

?>

Die Zeile produziert im Endeffekt Murks:

Code: Alles auswählen

$aUserTable = array_csort($aUserTable, $_REQUEST["sortby"], $sortorder);

Gesehen, verstanden. Komisch, daß das bei meinen Tests nicht aufgefallen ist?

Auch hier konstruktive Kritik: mySQL und alle anderen DBs sind auf die Sortierung von Werten optimiert (und ORDER BY ist ein SQL92-Standard). Ich wäre dafür, die DB-Sortierungsfähigkeiten zu nutzen (Datumseinträge werden z.B. mit array_csort auch garantiert nicht "richtig" sortiert). Das gleiche gilt für das Filtern von Werten. Dazu später mehr.

Das ist mir schon klar. Aber wie willst du Werte, die du nicht (bzw nicht direkt) aus der Datenbank erhälst, über die Datenbank sortieren?

Das Löschen von Einträgen funktioniert nicht (gilt überall, wo FolderRow eingesetzt wird, also auch http://www.contenido.org/forum/viewtopi ... 0381#50381, Fehlermeldung DEDE).

Hängt aber zu 99.9% nicht mit der FoldingRow zusammen. Werde ich nochmal testen.

Ansonsten wird der Code in $deleteScript z.Z. kaum genutzt. Wenn, wäre der Funktionsname deleteModule falsch und außerdem enthält die Funktion die Zeile

Code: Alles auswählen

			url += \'&page=\' +form.page.value;

. Im aktuellen Code steht jedoch kein Form-Feld mit Namen page zur Verfügung (zumindest, wenn nur eine Seite zur Verfügung steht, habe es mit mehr Seiten noch nicht getestet. Damit vermute ich aber, dass das Blättern auch nicht funktioniert).

Blättern funktioniert IMHO überall...

Da ich persönlich ein Fan der JS-messageBox und kein Freund der Standard-confirm-Box bin, würde ich die nächsten Tagen einen funktionierenden Code zur Verfügung stellen. Das würde dann auch ein Code zum Sortieren und Filtern enthalten (der Code zum Filtern in include.rights_menu.php ist mir zu kompliziert).

Ich bin kein Fan dieser Contenido-spezifischen Messagebox. Bis das Ding immer richtig läuft, vergeht vielzuviel Zeit, deshalb verwende ich die Standard-JS-Box.

Hierzu habe ich noch eine Frage:

Code: Alles auswählen

$oListActionRow = new cFoldingRow("339b79d1-48f7-4ac6-ba17-b958c5b3bb2b",i18n("Actions"));

Woher kommt die GUID? Bzw. ich benötige eine weitere GUID für einen Bereich "Options".

Die GUID kannst du mit dem Progamm "uuidgen" erzeugen (bei Linux normalerweise installiert, es gibt aber auch Windows-derivate).

[HerrB]

[Prüfen auf numerische Werte]Wieso? Die Werte können nur von diesem einen Frame kommen. Klar kann man überall Prüfungen einbauen, nur der Aufwand wäre gegenüber dem Nutzen nicht gerechtfertigt...denn wenn man es hier macht, müsste man es überall anders auch machen.

Ähm, ja, halte ich aber generell für eine gute Idee. Eine Prüfung auf if ($Wert="" || !is_numeric($Wert)) ... genügt ja. Ich gebe Dir recht, dass es kein Must-Have ist, mein Gedanke geht immer in Richtung Missbrauch: "Jetzt zeige ich mal meinem Chef, was das für ein *%§/)-System das ist". An anderen Stellen ist es vielleicht nicht so harmlos (in meinem Code z.B. werden die Variablen-Werte für eine Abfrage gegen die DB genutzt) und wenn man es generell macht, gewöhnt man sich daran. Aber, wie gesagt, muss ja nicht.

Nein, das ist nicht korrekt. Ein get verursacht keinen Datenbankaufruf, sondern greift auf die vorher geladenen Werte innerhalb der GenericDB zurück.

OK, mein Irrtum. Hätte schwören können, dass das mal anders war.

Das ist mir schon klar. Aber wie willst du Werte, die du nicht (bzw nicht direkt) aus der Datenbank erhälst, über die Datenbank sortieren?

Guter Punkt. Mmmh, Du meinst in Richtung PlugIn? Mmmh, gut, dann meckere ich rum, dass die Datumssortierung nicht funktionieren dürfte

Hängt aber zu 99.9% nicht mit der FoldingRow zusammen. Werde ich nochmal testen.

Nein, hängt es auch nicht, der fehlerhafte Code ist nur überall drin, wo das neue FoldingRow eingesetzt wurde.

Ich bin kein Fan dieser Contenido-spezifischen Messagebox. Bis das Ding immer richtig läuft, vergeht vielzuviel Zeit, deshalb verwende ich die Standard-JS-Box.

Na, vielleicht kann Dich ja mein Code überzeugen... (das könnte ich auch bei allem, was auf den Klassen/genericdb basiert, sagen... ).

Die GUID kannst du mit dem Progamm "uuidgen" erzeugen

Ähm, danke, es gibt sogar ein Webtool: http://kruithof.xs4all.nl/uuid/uuidgen. Dann ist Options jetzt 5ddbe820-e6f1-11d9-8cd6-0800200c9a66.

Muss man doch nicht für jede Seite neu erzeugen, oder?

Gruß
HerrB

[timo]

Ähm, ja, halte ich aber generell für eine gute Idee. Eine Prüfung auf if ($Wert="" || !is_numeric($Wert)) ... genügt ja. Ich gebe Dir recht, dass es kein Must-Have ist, mein Gedanke geht immer in Richtung Missbrauch: "Jetzt zeige ich mal meinem Chef, was das für ein *%§/)-System das ist". An anderen Stellen ist es vielleicht nicht so harmlos (in meinem Code z.B. werden die Variablen-Werte für eine Abfrage gegen die DB genutzt) und wenn man es generell macht, gewöhnt man sich daran. Aber, wie gesagt, muss ja nicht.

Naja, ich würde gerne dafür etwas generisches bauen. Leider fehlt mal, wie immer, die Zeit...

Guter Punkt. Mmmh, Du meinst in Richtung PlugIn? Mmmh, gut, dann meckere ich rum, dass die Datumssortierung nicht funktionieren dürfte

Brav Hast du das Plugin mal getestet?

Nein, hängt es auch nicht, der fehlerhafte Code ist nur überall drin, wo das neue FoldingRow eingesetzt wurde.

Das ist warscheinlicher

Dann ist Options jetzt 5ddbe820-e6f1-11d9-8cd6-0800200c9a66.

Muss man doch nicht für jede Seite neu erzeugen, oder?

Wie, für jede Seite? Die UUID ist dafür da, daß jede Folding-Row eine eigene (einigermaßen eindeutige) ID bekommt, damit die Informationen über den Aufklappzustand gespeichert werden können.

[HerrB]

Brav Hast du das Plugin mal getestet?

Nein, ich fummele noch am Newsletter (ja, jetzt mit gefalteten Zeilen) und Optionen zum Sofort-Einstellen ohne Mandanteneinstellungen-Fummeling. Wenn ich Zeit habe, teste ich das mal.

Wie, für jede Seite? Die UUID ist dafür da, daß jede Folding-Row eine eigene (einigermaßen eindeutige) ID bekommt, damit die Informationen über den Aufklappzustand gespeichert werden können.

Gut, dass Du das sagst. Mit Seite meinte ich Bereich, also z.B. unter Extras -> Empfänger, Extras -> Newsletter. Demzufolge muss man für jede Seite, auf der es eingesetzt wird und jede Kategorie eine neue UUID verwenden (ich habe z.B. gerade unter Extras -> Empfänger die UUID aus Administration -> Frontend verwendet. Was offensichtlich keine so gute Idee ist).

Gruß
HerrB

[timo]

Gut, dass Du das sagst. Mit Seite meinte ich Bereich, also z.B. unter Extras -> Empfänger, Extras -> Newsletter. Demzufolge muss man für jede Seite, auf der es eingesetzt wird und jede Kategorie eine neue UUID verwenden (ich habe z.B. gerade unter Extras -> Empfänger die UUID aus Administration -> Frontend verwendet. Was offensichtlich keine so gute Idee ist).

Naja im Moment macht das nicht allzuviel - das einzige, was im Moment passieren würde wäre, daß wenn das Menü bei den Frontendbenutzern aufgeklappt wäre, das beim Newsletter auch aufgeklappt wäre, und umgekehrt.

[HerrB]

So, mein Vorschlag wäre fertig. Damit man Extras -> Empfänger gefaltet erhält, benötigt man diese Dateien (-> includes): hier

include.recipients_menu.php enthält noch ein paar Besonderheiten:

Standardwerte werden gleich am Anfang definiert (kein Dickkopf meinerseits, mehr Zufall, da ich die Elemente recht früh benötigte) und - so ein Zufall - da stören auch Überprüfungen nicht:

Code: Alles auswählen

/* Set default values */
if (!isset($_REQUEST["elemperpage"]) || !is_numeric($_REQUEST['elemperpage'])) {
	$_REQUEST["elemperpage"] = 25;
}

if (!isset($_REQUEST['restrictgroup']) || !is_numeric($_REQUEST['restrictgroup'])) {
	$_REQUEST['restrictgroup'] = "--all--";
}

Dieser Code

Code: Alles auswählen

$aFields = array();
$aFields["name"]  	= array("field" => "name", "caption" => i18n("Name"), "type" => "base,sort,search");
$aFields["email"] 	= array("field" => "email", "caption" => i18n("E-Mail"), "type" => "base,sort,search");
$aFields["confirmed"]	= array("field" => "confirmed", "caption" => i18n("Confirmed"), "type" => "base");
$aFields["deactivated"] = array("field" => "deactivated", "caption" => i18n("Deactivated"), "type" => "base");

ersetzt drei Arrays ($aFieldSources, $aFieldsToSearch, $aFieldsToSort) und bietet das Feature, dass auch Elemente, die via Plugin eingebunden würden z.B. für Sortierung und Filterung verwendet werden könnten (man lernt ja dazu...).

Die Berücksichtigung für SearchIn sieht dann so aus:

Code: Alles auswählen

$oSelectSearchIn = new cHTMLSelectElement("searchin");
$oOption = new cHTMLOptionElement(i18n("-- All fields --"), "--all--");
$oSelectSearchIn->addOptionElement("all", $oOption);

foreach ($aFields as $sKey => $aData) {
	if (strpos($aData["type"], "search") !== false) {
		$oOption = new cHTMLOptionElement($aData["caption"], $sKey);
		$oSelectSearchIn->addOptionElement($sKey, $oOption);
	}
}
$oSelectSearchIn->setDefault($_REQUEST["searchin"]);

Die Datenübernahme in das Daten-Array sieht dann so aus:

Code: Alles auswählen

while ($oRecipient = $oRecipients->next()) {
	foreach ($aFields as $sKey => $aData) {
		if (strpos($aData["type"], "base") !== false) {
			if ($sKey == "name") {
				$name = $oRecipient->get("name");
				if (empty($name)) {
					$aDataTable[$oRecipient->get("idnewsrcp")][$sKey] = $oRecipient->get("email");
				} else {
					$aDataTable[$oRecipient->get("idnewsrcp")][$sKey] = $name;
				}
			} else {
				$aDataTable[$oRecipient->get("idnewsrcp")][$sKey] = $oRecipient->get($aData["field"]);
			}
		} else {
			$aDataTable[$oRecipient->get("idnewsrcp")][$sKey] = call_user_func("recipients_".$aData["field"]."_getvalue", $sKey);
		}
	}
	
	if ($_REQUEST["filter"] != "") {
		if ($_REQUEST["searchin"] == "--all--" || $_REQUEST["searchin"] == "") {
			$found = false;
			
			foreach ($aDataTable[$oRecipient->get("idnewsrcp")] as $key => $value) {
				if (strpos($value, htmlentities($_REQUEST["filter"])) !== false) {
					$found = true;
				}
			}
			
			if ($found == false) {
				unset($aDataTable[$oRecipient->get("idnewsrcp")]);
			}			
		} else {
			if (strpos($aDataTable[$oRecipient->get("idnewsrcp")][$_REQUEST["searchin"]], htmlentities($_REQUEST["filter"])) === false) {
				unset($aDataTable[$oRecipient->get("idnewsrcp")]);
			}
		}
	}
}

Ich habe es mir mal wieder einfach gemacht: Alles, was "base" im Typ enthält, wird einfach aus der DB ausgelesen, alles andere kommt aus einem Plugin (bzw. einer entsprechenden Callback-Funktion).

Wichtig ist noch das htmlentities bei htmlentities($_REQUEST["filter"]), da sonst Umlaute nicht gefunden werden (man suche mal nach "ä" -> gilt für alle Bereiche mit diesem Code).

Dinge, die nicht so elegant sind:
Die Integration eines zweiten Form-Bereiches (Options) erfordert, dass die Angaben zu sortby, sortorder, filter usw. als hidden-Felder auch in dieses Form eingetragen werden. Da diese bei Änderung im List Options-Bereich nicht aktualisiert werden, kann es zu folgendem Effekt kommen: Man legt z.B. ein Suchkriterium fest (List Options) und speichert dann Einstellungen mit Save (Options) -> das Suchkriterium ist weg.

Was nicht geht:
Sortieren (o.a. Funktion ist auskommentiert)
Plugins für Empfänger (mal bei Gelegenheit integrieren)

Gerne nehme ich Hinweise und Meinungen, wie es noch eleganter geht, entgegen...

Sobald eine korrigierte Sortierfunktion verfügbar ist, würde ich den Code nochmal putzen, aber man kann ihn jetzt schon verwenden. In include.recipients_edit.php wurde nur geändert, dass purgetimeframe jetzt eine Mandanteneinstellung ist.

Gruß
HerrB

[timo]

Ist glaube ich mit dem neuen Newsletter obsolet