AddOn beachtet nicht "status" Spalte in "rex_article_slice" #65
Comments
|
Ok, lag doch nicht an bloecks. Hab den PR zurückgezogen, lag nur daran, wie ich getestet habe. Kurz als Info: lösche ich den Cache und starte cache_warmup, bevor ich das Frontend mit der entsprechenden Seite aufgerufen habe, dann wird cache_warumup den Status von bloecks ignorieren. Lösche ich den Cache, rufe die Seite im Frontend auf und starte danach cache_warmup, dann funktioniert es (da cache_warmup vermutlich den Artikel überspringt, bzw bereits ein Cache existiert). D.h. hier ist definitiv ein Bug. Das ganze würde vermutlich über einen Crawler nicht passieren. |
|
/cc @gharlan vllt hast du ne magische Idee |
|
Ich würde das Status-Feld default in die article_slice Tabelle aufnehmen und mit 1 default setzen. Blöcks kann sich weiterhin um das wechseln des Status kümmern. Content-Plugin müsste das Feld dann ebenso auswerten. |
|
Also ist das ein Core-Ding @tbaddade ? |
|
ja, wäre mein Vorschlag. Dann könnten alle AddOns auch auf das Feld reagieren, da immer vorhanden. Ps: Blöcks selber wertet m.E. den Status nur im Frontend aus. |
|
cache_warmup könnte auch prüfen ob die spalte slice_status exisitiert und falls ja diese berücksichtigen. sehe hier kein todo für den core (zumindest nicht aus den genannten gründen) |
|
Mir scheint, zumindest Bloecks kann gegen das Problem nicht sinnvoll angehen. Denn es muss ja notwendigerweise zwischen Frontend und Backend trennen, und im Backend müssen (anders als im Frontend) auch Offline-Slices ausgegeben werden, um sie bearbeiten zu können. Das Problem liegt nun darin, dass Cache-Warmup sich plump die Artikeldaten holt, dabei aber zwangsläufig als Backend agiert, nicht als Frontend. Es kriegt also alle Daten, auch die, die Offline sind. Falls ihr das irgendwie im Core angehen wollt, gerne. Ansonsten hat @staabm vermutlich recht, und wir sollten Cache-Warmup beibringen, den Status zu beachten. |
|
@hirbod mal andersherum gefragt: oben im issue ist ja nur eine behauptung beschrieben dass dies ein bug ist. was genau das problem ist steht gar nicht dort. was genau ist das problem wenn cache_warmup auch für "offline-slices" caches generiert? was du beschreibst könnte ja nur eintreffen falls cache_warmup in der datenbank die spalte status von 0 auf 1 ändern würde o.ä.? |
|
@staabm Blöcks fügt der article slice Tabelle einen Status-Wert hinzu. Der Wert wird per 0/1 gesetzt und triggered beim SLICE_SHOW. Der EP sendet einen leeren String für den SLICE falls ausgeblendet. Funktioniert Prima, außer wenn Cache Warmup den Cache mittels new rex_article_content erstellt. Dann wird der EP anscheinend nicht getriggered und der ausgeblendete Block mit sensiblen Inhalt ist wieder sichtbar. |
|
@hirbod Genau, der EP triggert nicht, und es wäre auch nicht sinnvoll, weil die Slices im Backend ja nicht ausgeblendet werden sollen, sondern zur Bearbeitung sichtbar bleiben. Nach meinem Verständnis kann Bloecks deshalb gar nichts tun, um das Issue zu vermeiden, sondern wir müssen es an anderer Stelle beheben. |
|
Für mich gehört das Issue zu Blöcks. Auch wenn Blöcks eventuell da nichts machen kann, Blöcks stellt das Feature zur Verfügung und müsste es entsprechend händeln. Auch sehe ich das |
|
ich denke der ansatz in blocks ist falsch. falls ich es richtig verstanden habe wird dort aktuell zur laufzeit nen filter aufgerufen der entscheidet ob ein slices angezeigt werden soll/muss oder nicht. eigentlich müsste bloecks den von redaxo generierten php-code (article-cache) modifizieren und dort ein IF reinbringen anstatt zur Laufzeit mit SLICE_SHOW was zu unterdrücken. der bloecks ansatz ist fragil, was sich auch in der readme durch folgenden Satz sehr gut erkennen lässt
es taugt nicht dass andere addons andere EPs nützen müssen weil ein addon da ist, als das vom Core. |
|
Die einfachste Lösung aktuell wäre wohl den content von artikeln nicht zu generieren sondern nur die metadaten und listen (bis bloecks hier besser funktioniert). |
|
Wenn du dir den CODE jedoch ansiehst, wirst du feststellen, das SLICE_SHOW_BLOECKS_FE bei SLICE_SHOW aufgerufen wird. Ich kann es noch nicht ganz debuggen, aber der EP wird einfach nicht aufgerufen. Entweder es liegt an Bloecks oder diesem Addon bzw. am Core. Man müsste mal testen. |
|
cache_warmup ruft $article = new rex_article_content;
$article->getArticle();auf. Die Funktion widerrum callt generateArticleContent wenn es kein Cache-File gibt, und in dieser Funktion wird der EP gar nicht getriggered. Ich weiss gar nicht, wer hier anpassen sollte. |
|
@staabm @gharlan wie wäre es denn mit SLICE_SHOW, SLICE_SHOW_FRONTEND und SLICE_SHOW_BACKEND? Kann man nicht expliziter diesen EP trennen? Dann bedarf es keiner weiteren Trennung in der boot.php und der Core kümmert sich darum, welcher EP getriggered wird. |
|
@hirbod Cache-Warmup agiert als Backend, deshalb generiert es Cachefiles, die auch Offline-Inhalte enthalten. Ich glaube nicht, dass die EPs hier weiterhelfen können, sondern es bleibt immer der Konflikt bestehen zwischen Backend und Frontend? |
|
@schuer nicht ganz. |
|
@hirbod Vielleicht habe ich den Denkfehler, aber meiner Meinung nach bleibt das Problem bestehen: Cache-Warmup agiert im Backend. Auch wenn REDAXO die EPs irgendwie automatisch regeln würde, käme das Cachefile als Backend-Version raus, also mit den nicht gewünschten Offline-Slices. Denn wie sollte REDAXO wissen, dass die Frontend-Variante benötigt wird? Das müsste man dann schon explizit definieren können mittels Parameter oder so. |
|
wie wäre es mit nem EP ARTICLE_RENDER, in dem man festlegen kann, ob ein Artikel wie im Frontend oder Backend gerendert werden soll? (my 2 cents als Laie) Oder eben einen Parameter bei getArticle. |
|
In meinen Augen werden wir - egal wie wir die EPs jetzt nennen - damit das Problem nicht lösen können. |
Außer es würde eine Art Paramter geben, welcher der Core beachtet. (Simulate Frontend) oder ähnliches. |
|
Es würde m.E. einiges erleichtern, wenn wir das Feld "status" in die Tabelle (default 1) aufnehmen. Blöcks kann die Werte setzen, es im Frontend auswerten und im Backend ignorieren (Slices wären immer zu sehen). Jedes andere AddOn kann ebenso das Feld für sich auswerten, insbesondere Cache Warmup. |
|
Was @tbaddade sagt. |
|
Bin auch für Thomas' Vorschlag. @gharlan ? |
|
Release 2.1.1 deaktiviert nun vorerst die Artikelinhalte und generiert nur noch Metadaten und Listen, bis wir entschieden haben, wie wir weiter vorgehen wollen. |
|
Mein Vorschlag: Unabhängig davon, ob REDAXO das BlÖcks entfernt beim Deinstallieren vorbildlich das Status-Feld, deshalb können wir aktuell sicherlich davon ausgehen: Ist das Status-Feld vorhanden, wird es auch genutzt. Also können wir den Status auch beachten. Was meint ihr? |
|
man könnte es ja so bauen dass man das ein evlt. vorhandenes status feld beachtet wird wenn bloecks installiert ist. ich glaube aber am einfachsten fährst du wenn du den content einfach nicht mehr generierst... der gewinn beim vor-generieren des contents von artikel ist ja auch eher marginal. |
|
@staabm Inhalte abzuklemmen ist bestimmt die einfachste Variante, ich finde nur schade, dass dann eben nicht mehr alles generiert wird. Aktuell ist es ja so, dass man nach dem Warmup die Datenbank abklemmen könnte, falls man nur mit Artikeln rummacht und kein yform etc. nutzt. Es fühlt sich nach einem echten Warmup an. Wenn wir nun die Artikelinhalte ausklammerten, hätte das zwar keine merklichen Auswirkungen auf die Performance, aber trotzdem fühlte es sich nicht mehr vollständig aufgewärmt an ;) |
|
Ich bin nicht dafür, das Feld im Core nur als Feld aufzunehmen. Ich bin aber dafür (wie schon mehrfach gesagt), das ganze Feature Slice-Status in den Core zu übernehmen. Ich verstehe übrigens auch nicht, wie es bei dem konkreten Problem hier helfen würde, wenn wir das Status-Feld aufnehmen würden. Allgemein verstehe ich nicht, wie euer Lösungsansatz "Cache-Warmup berücksichtigt Slice-Status" funktionieren soll? |
|
@gharlan Ich habe das noch nicht im Detail überdacht, gehe aber bisher davon aus, dass Cache-Warmup ähnlich vorgehen müsste wie blÖcks: per EP einhaken und leere Slices zurückgeben, wenn sie offline sind. Unterm Strich also lediglich eine andere Unterscheidung: An der Stelle, wo blÖcks Inhalte durchreicht, weil der Kontext Backend ist, würde Cache-Warmup sie durchgängig ausblenden. |
|
Ah ok verstehe. Ja würde vermutlich funktionieren. |
|
Ich finde die Situation auch etwas unbefriedigend gerade. Cache-Warmup benutzt eigentlich nur Hausmittel, hat aber bereits Probleme mit SVGs und dem Slice-Status. Sehe das wie du: Die Anpassungen für Slice-Status sind nicht unbedingt allzu schön. Aber sollte das nicht wirklich ein Core-Feature werden, werden wir sie wohl einbauen müssen oder in Kauf nehmen, dass Artikelinhalte nicht mehr generiert werden können. Beides doof. |
das wird in einer echten website auch so gut wie nie funktionieren, da es es meist ja zig addons/yform tabellen o.ä. die sowieso eine db connection brauchen. Ich glaube dass es dieses szenario so gut wie nie geben wird, daher würde ich dafür keine energie verschwenden. |
Oder wir passen blöcks an. Kann versuchen, mir das zeitnah mal anzuschauen. Wobei ich auch alles mögliche andere auf der Liste habe, was ich zeitnah angehen will ;) |
ich hatte es oben schon erwähnt.. meiner meinung nach darf blöcks nicht via EP filtern sondern sollte den generated source (article-cache) von den artikel "anpassen". |
|
Ein Nebeneffekt von Cache-Warmup ist übrigens, dass beim generieren aller Artikel auch Fehler erkennbar werden, auf die man vorher nicht geachtet hat (eine kleine fehlerhafte Änderung im Modul bspw.). Dieses Feature finde ich sehr praktisch, um grob die Seite zu testen und hat mich auch schon vor dem ein oder anderen Fehler auf der Live-Seite bewahrt - denn nach jeder Änderung auf einer Seite 100 Artikel zu testen ist auch utopisch. |
@staabm Kriegt man sowas denn robust hin, die generierten Artikel nachträglich anzupassen, um Offline-Slices rauszufischen? Klingt ein bisschen nach RegEx-Action. |
|
Dann gebe ich meinen Senf auch mal dazu (Sorry, war in Elternzeit): Das mit dem Frontend/Backend-Rendern ist das grundlegende Problem, ja. Meiner Meinung nach bleiben da zwei Lösungsansätze: (a) Der EP SLICE_SHOW wird immer ausgeführt (auch wenn der Slice aus dem Cache kommt), bzw. es muss einen EP geben, der immer ausgeführt wird, wenn ein Slice ausgegeben wird (egal ob frisch gerendert oder aus dem Cache). (b) Das Cache_Warmup-Addon (wie auch das Search_IT und alle anderen Addons die im Backend Frontend-Content generieren) müssen das System vor dem Rendern in einen FRONTEND-Modus setzen und hinterher wieder zurück in den BACKEND-Modus. |
|
Ich bin für B), ansonsten hätte man für jeden SLICE einen SQL-Call |
|
Ich habe gerade einen PR dazu angelegt, vielleicht habt ihr Zeit und Lust, mal kurz draufzuschauen. In meiner lokalen Umgebung hat das Zusammenspiel mit blÖcks prima geklappt. |
* switch REDAXO to frontend mode before generating cache files * enable generating article content again fixes #65
Das AddOn bloecks fügt zur
rex_article_sliceeine Status-Spalte hinzu, welche beim EP SLICE_SHOW beachtet wird. Anscheined greift das aber nicht in Verbindung mit dem cache_warmup. Es tauchen nach dem Ausführen des AddOns plötzlich alle inaktiven Slices wieder im Frontend auf (obwohl der Value nach wie vor auf "0" steht).Kann sich das jemand mal ansehen? Ich hab es jetzt noch auf einer Dev-Seite getestet (mit zwei Seiten und zwei Bildern - es ist reproduzierbar)
The text was updated successfully, but these errors were encountered: