Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Methoden zum Auslesen hinzugefügt #68

Merged
merged 15 commits into from May 17, 2019
Merged

Methoden zum Auslesen hinzugefügt #68

merged 15 commits into from May 17, 2019

Conversation

TobiasKrais
Copy link
Member

Mit den hinzugefügten Methoden lassen sich sowohl der Stream als auch die zugehörigen Items in einem Modul, Template oder Addon auslesen.

Ist sehr praktisch, wenn man die Klasse für ein select Feld verwenden will.
So lassen sich die zum Stream gehörenden Items auslesen
@TobiasKrais
Copy link
Member Author

Ich wäre dankbar um ein kurzfristiges Feedback, damit ich weiß, ob ich auf dieser Basis weiter programmieren kann oder eine eigene Klasse einsetzen muss.

Methode gab bisher immer die ältesten Items zurück, mit dieser Änderung sind es die neuesten
@skerbis
Copy link
Member

skerbis commented Nov 8, 2018

@dergel ping

@alxndr-w
Copy link
Member

alxndr-w commented Nov 8, 2018

@TobiasKrais auch Doku dahingehend ergänzt?

@TobiasKrais
Copy link
Member Author

Ne. Gibts das für das Addon überhaupt? Ich habe mich durch den Code gearbeitet und ein Beispielmodul erstellt: https://github.com/TobiasKrais/d2u_helper/tree/master/modules/12/1

@alxndr-w
Copy link
Member

alxndr-w commented Nov 8, 2018

https://github.com/yakamara/redaxo_yfeed/blob/master/pages/faq.php

Es gibt zumindest diese FAQ. Ansonsten bitte, wenn es dir möglich ist, ein Doku-Issue aufmachen und Methoden exemplarisch aufzeigen. Das fließt sicher früher oder später ins Add-On.

@TobiasKrais
Copy link
Member Author

TobiasKrais commented Nov 8, 2018

Wäre bestimmt eine gute Sache. Der Code ist entsprechend mit PHP Doc kommentiert, so dass man hier schon mal weiter kommt. Bitte den Pull Request nicht wegen der fehlenden Doku aufschieben.

Copy link
Member

@gharlan gharlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grundsätzlich finde ich es gut, wenn es die Möglichkeit gibt, die Items abzurufen.

lib/item.php Outdated Show resolved Hide resolved
lib/item.php Show resolved Hide resolved
$result->setQuery('SELECT id FROM '. rex::getTablePrefix() .'yfeed_item ORDER BY updatedate DESC LIMIT 0, '. $number .';');

for ($i = 0; $i < $result->getRows(); $i++) {
$items[] = rex_yfeed_item::get($result->getValue('id'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hier wäre es besser, wenn direkt die Daten aus der einen Query oben kommen würde, und nicht für jedes Item noch mal separat die Daten abgefragt werden müssten.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das wäre bestimmt performanter. Aber:

  • Von der Logik her gehört diese Methode in die rex_yfeed_stream_abstract Klasse. Außerdem wäre die Trennung der Logik verletzt, wenn wir hier rex_yfeed_item Instanzen erstellen.
  • Wegen der Sichtbarkeit der Variablen der rex_yfeed_item Klasse können hier keine Werte zugewiesen werden.

Wenn du möchtest, können wir aber in der rex_yfeed_item Klasse eine Methode getPreloadedStreamItems($streamId, $numberItems = 5) erstellen.

@TobiasKrais TobiasKrais changed the title Methoden zum auslesen hinzugefügt Methoden zum Auslesen hinzugefügt Nov 18, 2018
@TobiasKrais
Copy link
Member Author

@gharlan Wie sieht es mit dem Pull Request aus?

@alxndr-w
Copy link
Member

alxndr-w commented Jan 5, 2019

Wäre das nicht obsolet und könnte mit YOrm erledigt werden, wenn man das hier macht? #74

Bzw. wäre vlt. trotzdem ergänzend sinnvoll, aber einfacher.

@alxndr-w
Copy link
Member

Was mir noch fehlen würde wären die Anpassungen dazu in der Readme

@TobiasKrais
Copy link
Member Author

Wenn ich weiß, dass der Code rein genommen wird, kann ich gerne ein Codebeispiel in die Readme nehmen. Beispielcode gibt es ja schon: https://github.com/TobiasKrais/d2u_helper/blob/184678c1e953aaed52aa1a19e5f940ac64275c62/modules/12/1/output.php#L36

@skerbis
Copy link
Member

skerbis commented May 17, 2019

@TobiasKrais ich werde den Code übernehmen - bitte noch etwas Geduld

@TobiasKrais TobiasKrais merged commit ff5716d into FriendsOfREDAXO:master May 17, 2019
@TobiasKrais
Copy link
Member Author

TobiasKrais commented May 17, 2019

Doku ist hinzugefügt. Aber ich glaube jetzt habe ich den Code selbst in den Master gemerged. Das sollte man nicht machen, habe ich gelernt. Eigentlich wollte ich den aktuellen Code aus dem FOR master in meinen Branch holen. Irgendwie blicke ich das noch nicht ganz. Ich bitte vielmals um Entschuldigung!

@TobiasKrais TobiasKrais deleted the patch-1 branch May 17, 2019 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants