Social Media
Foren
QA Onlinestatus
- 1
- 2
|
QA Onlinestatus
"Datenplugin, mit welchem man auslesen kann, ob ein User online ist. Außerdem können alle eingeloggten User zurückgegeben werden."
Code: http://code.contentlion.de/Plugin+OnlineStatus/
Mein Feedback:
Datenbank:
- Wenn du Spalten in fremde Tabellen einfügst, Namespaces nutzen.
- Warum nimmst du ein int, wenn du nur eine 0 oder eine 1 speichern möchtest? Nimm mal besser BIT ;-)
- Da du ja eine Funktion hast, die alle User zurückliefert, die online sind, lege auf deine online-Spalte am besten noch einen Index, sonst wird es bei zu vielen Usern zu langsam
Hauptklasse
- Da es ein API-Plugin ist, bitte auch PHPDoc hinzufügen, damit man auch sofort weiß, worum es geht.
- Bennen mal updateOnlineStatus in update um, OnlineStatus heißt ja schon die Klasse
- Kannst du machen, dass man über unsere Settings den Zeitraum einstellen kann, wann ein User als online geht? Brauch auch noch nicht per Oberfläche sein. Viele Trackingsysteme nehmen nämlich ne halbe Stunde als Basis, dann könnte man sowas auch abdecken.
- In der Funktion getAllOnlineUser am besten auch User-Objekte zurückgeben. (da muss auf die dauer noch eine get_by_sql hinzu, damit du sowas leichter abfangen kannst, werd ich mal irgendwann einbauen)
Allgemein:
- Kannst du das Updaten des Online-Status auf 0 auch als Task anlegen? Aktuell würdest du bei jedem Seitenzugriff die User-Tabelle updaten - Bei vielen Usern wird das zur Last. Den Task dann einfach alle paar Minuten laufen lassen.
- Wieso nutzt du eigentlich nicht die Spalte last_access_timestamp als Basis? Die wird ja sowieso schon befüllt.
Code: http://code.contentlion.de/Plugin+OnlineStatus/
Mein Feedback:
Datenbank:
- Wenn du Spalten in fremde Tabellen einfügst, Namespaces nutzen.
- Warum nimmst du ein int, wenn du nur eine 0 oder eine 1 speichern möchtest? Nimm mal besser BIT ;-)
- Da du ja eine Funktion hast, die alle User zurückliefert, die online sind, lege auf deine online-Spalte am besten noch einen Index, sonst wird es bei zu vielen Usern zu langsam
Hauptklasse
- Da es ein API-Plugin ist, bitte auch PHPDoc hinzufügen, damit man auch sofort weiß, worum es geht.
- Bennen mal updateOnlineStatus in update um, OnlineStatus heißt ja schon die Klasse
- Kannst du machen, dass man über unsere Settings den Zeitraum einstellen kann, wann ein User als online geht? Brauch auch noch nicht per Oberfläche sein. Viele Trackingsysteme nehmen nämlich ne halbe Stunde als Basis, dann könnte man sowas auch abdecken.
- In der Funktion getAllOnlineUser am besten auch User-Objekte zurückgeben. (da muss auf die dauer noch eine get_by_sql hinzu, damit du sowas leichter abfangen kannst, werd ich mal irgendwann einbauen)
Allgemein:
- Kannst du das Updaten des Online-Status auf 0 auch als Task anlegen? Aktuell würdest du bei jedem Seitenzugriff die User-Tabelle updaten - Bei vielen Usern wird das zur Last. Den Task dann einfach alle paar Minuten laufen lassen.
- Wieso nutzt du eigentlich nicht die Spalte last_access_timestamp als Basis? Die wird ja sowieso schon befüllt.
|
- deactivate.php Bei den Settings fehlt der areaType (auf Plugin)
- Im activate hast noch nen error_reporting drin. Bitte rausnehmen, dafür haben wir ja den DEVELOPMENT-Status in der Consts
- Kommentare bitte auf Englisch, wollen ja international werden
- Wenn man über die Settings jetzt nen Hochkomam in die online_time-Spalte einträgt, kann man ne SQL-Injection erzuegen. Außerdem musst du dort die specify-Funktion nutzen, um den Setting aus der areaType/area herauszulesen. Sag Bescheid, wenn du nicht genau weißt wie.
Der phpdoc-Kommentar bei getAllOnlineUser passt noch nicht ganz, bei @return gibst du ja keine stdClass zurück, kannst da z.B. array(User) nehmen.
Sind aber alles nur Kleinigkeiten, im Großen und Ganzen schon gut.
- Im activate hast noch nen error_reporting drin. Bitte rausnehmen, dafür haben wir ja den DEVELOPMENT-Status in der Consts
- Kommentare bitte auf Englisch, wollen ja international werden
- Wenn man über die Settings jetzt nen Hochkomam in die online_time-Spalte einträgt, kann man ne SQL-Injection erzuegen. Außerdem musst du dort die specify-Funktion nutzen, um den Setting aus der areaType/area herauszulesen. Sag Bescheid, wenn du nicht genau weißt wie.
Der phpdoc-Kommentar bei getAllOnlineUser passt noch nicht ganz, bei @return gibst du ja keine stdClass zurück, kannst da z.B. array(User) nehmen.
Sind aber alles nur Kleinigkeiten, im Großen und Ganzen schon gut.
|
bitte nicht pushen. Ich kann leider nicht jeden Tag Plugins testen, deswegen kann es bis zu einer Woche dauern (hab es sogar aufgeschrieben). Gestern wurde ich nämlich in nem Podcast zu ContentLion interviewed, Link kommt natürlich noch.
Werde das Plugin morgen oder Samstag erneut testen. Wenns länger als eine Woche dauert, am besten auch per PM und nicht hier im forum pushen.
Werde das Plugin morgen oder Samstag erneut testen. Wenns länger als eine Woche dauert, am besten auch per PM und nicht hier im forum pushen.
|
Das Plugin wurde veröffentlicht: http://www.contentlion.de/plugins/onlinestatus-abfragen.html
Mir ist übrigens aufgefallen, dass du ne Funktion drin hast, die eigentlich keinen Sinn macht: Bei isOnline nimmst du als Default ja den aktuellen User => Der dürfte eigentlich immer online sein
Mir ist übrigens aufgefallen, dass du ne Funktion drin hast, die eigentlich keinen Sinn macht: Bei isOnline nimmst du als Default ja den aktuellen User => Der dürfte eigentlich immer online sein
- 1
- 2