Repository navigation
Conversation
There was a problem hiding this comment.
Hier erstmal der review von claude. Ich schaue morgen nochmal selbst genauer drauf. Eine Sache ist mir schon aufgefallen, du lieferst die volle Bootstrap 5 Version mit aus. Ich denke du machst das wegen vite, aber die DBT sollte die schon haben.
Echte Bugs
-
formatDate lässt die ganze Episodenliste abstürzen (PlaylistEpisode.vue:28)
Intl.DateTimeFormat#format(new Date('')) wirft einen RangeError: Invalid time value. Fehlt bei einem einzigen das pubDate oder ist es nicht parsebar, wird die Komponente gar nicht gerendert. Prüfe deshalb vorher mit Number.isNaN(date.getTime()) und gib in dem Fall '' zurück. -
Falscher Fehlerstatus bei schnellem Episodenwechsel (Player.vue:52-59)
Wenn man schnell „Nächste Folge“ klickt, lehnt der play()-Aufruf der vorigen Episode mit AbortError ab („interrupted by a new load request“). Der catch des alten Watcher-Durchlaufs setzt dann audioError = true, obwohl der neue Durchlauf es gerade erst zurückgesetzt hat. Die neue Folge läuft, aber angezeigt wird „AUDIODATEI NICHT VERFÜGBAR“. Der Fehler wird erst beim nächsten Episodenwechsel wieder zurückgesetzt. Lösung: AbortError ignorieren, oder im catch prüfen, ob episode noch playerStore.episode ist.
Ähnlich ist es bei NotAllowedError (Autoplay-Sperre, besonders iOS Safari, weil zwischen Klick und play() ein await nextTick() liegt). Dieser Fall wird ebenfalls als „Audiodatei nicht verfügbar“ angezeigt, obwohl die Datei in Ordnung ist.
- isPlaying kann vom tatsächlichen Audio-Zustand abweichen (stores/player.ts:62, Player.vue:67-84)
toggleEpisode schaltet isPlaying optimistisch um, und der Watcher ruft dann play() auf. Schlägt play() fehl, wird der Fehler nur geloggt. isPlaying bleibt true, und die UI zeigt „Pause“ und „Wiedergabe läuft“, obwohl nichts läuft.
Grundsätzlich ist isPlaying gleichzeitig ein Befehl (Watcher → play/pause) und ein Status (Audio-Events → isPlaying). Diese Kreisbeziehung macht solche Fehler wahrscheinlich. Besser wären Store-Actions wie requestPlay() und requestPause(), wobei isPlaying nur von den Audio-Events gesetzt wird. Zusätzlich sollte closePlayer() isPlaying = false setzen.
- Feed-Parser liest Kindelemente aus beliebiger Tiefe (utils/XMLParser.ts:11-36)
getElementsByTagName sucht in allen Nachfahren, nicht nur in den direkten Kindern. Fehlt am z. B. description, itunes:image, itunes:author oder language, nimmt der Parser stillschweigend den Wert der ersten Episode. Das Podcast-Cover wäre dann das Episodenbild, die Beschreibung die der ersten Folge.
Auch categories[1] ist nicht zwingend die Unterkategorie. Bei zwei Top-Level-Kategorien ist es die zweite Hauptkategorie. Lösung: nur direkte Kinder auswerten, z. B. Array.from(el.children).find(c => c.localName === ... && c.namespaceURI === ...) oder :scope > title.
- Request-Race in PodcastList.vue (searchPodcasts, Zeile 139)
Hier fehlt der requestSequence-Schutz, den Playlist.vue schon hat. Bei schnellem Filtern oder Blättern kann eine langsamere, ältere Antwort die neuere überschreiben. Das gleiche Muster aus Playlist.vue übernehmen.
Design / Robustheit
-
Alle Query-Parameter gehen ungefiltert an Solr (PodcastList.vue:80-84)
Jeder URL-Parameter wird angehängt. ?wt=xml bricht das JSON.parse, und ?rows=100000 oder ?fl=… werden einfach durchgereicht. Ein Sicherheitsproblem ist das nicht (der Solr-Servlet ist ohnehin öffentlich und fq=mods.genre:podcast bleibt bestehen), aber es ist unnötig fragil. Ich würde eine Whitelist einführen: condQuery, fq (nur mit itunes-podcast:-Präfix) und sort (nur die zwei erlaubten Werte). -
Seitennummer steht nicht in der URL (PodcastList.vue:18)
currentPage ist nur lokaler State. Wer auf Seite 3 einen Podcast öffnet und auf „Zurück“ klickt, landet wieder auf Seite 1. Reload und teilbare Links verlieren die Seite ebenfalls. Lösung: page in route.query aufnehmen; die Seite ergibt sich dann aus dem Router, und setPage macht einen router.push. -
RouterLink :to="podcast.id" (PodcastList.vue:296)
Das funktioniert nur, weil ein relativer Pfad von / aus aufgelöst wird. Besser benannt: :to="{ name: 'podcast', params: { podcastId: podcast.id } }". -
PlaylistHeader.vue:12-18: Props werden nicht-reaktiv ausgelesen
const episodes = props.playlistMetadata.episodes und lastEpisode werden nur einmal berechnet. Das funktioniert aktuell nur, weil Playlist.vue die Komponente beim Laden über v-else-if neu mountet. Mit computed wäre es nicht von diesem Detail abhängig.
Außerdem nimmt „Neueste Folge“ das letzte Item. Das stimmt nur, wenn der Feed aufsteigend sortiert ist. RSS-Feeds sind üblicherweise absteigend sortiert. Bitte das gegen das MIR-Stylesheet mycoreobject-podcast prüfen; das liegt nicht in DBT, daher konnte ich die Reihenfolge nicht verifizieren.
- checkDescriptionOverflow läuft nur in onMounted
Nach einer Größenänderung des Fensters oder nach dem Laden von Bildern im Beschreibungs-HTML stimmt das Ergebnis nicht mehr. Ein ResizeObserver würde helfen. Kleinigkeit.
Aufräumen / Kleinigkeiten
- src/main.ts ist toter Code. Es mountet ohne Router; RouterView würde dort nicht funktionieren. Entry-Point ist index.ts. Löschen.
- vite.config.ts: define: { 'process.env.NODE_ENV': 'production' } gilt auch für vite dev, dadurch fehlen im Dev-Modus Vue-Warnungen und Devtools. Nur für command === 'build' setzen.
- Pagination-Ellipse (visiblePages): Bei current = 5 wird 1 … 3 4 5 6 7 angezeigt. Das „…“ ersetzt also nur die Seite 2, und am Ende passiert dasselbe. Die Schwelle sollte so angepasst werden, dass eine Ellipse nur bei einer Lücke von mindestens 2 Seiten erscheint.
- Heading-Hierarchie in PlaylistEpisode.vue: Ein unsichtbares h2 „Folgen“ steht vor den Episoden-Headern, die ebenfalls h2 sind. Die Episoden-Header sollten h3 sein.
- id="episode-${episode.guid}": GUIDs sind oft URLs und können Sonderzeichen enthalten. Besser den Index oder einen Hash als ID verwenden.
- Player-Overlay: Der Player hat position: fixed und überdeckt das Footer-Ende. Solange er sichtbar ist, braucht body ein padding-bottom. Mit z-index: 1090 liegt er außerdem über Bootstrap-Modals (1055).
- ErrorState zeigt rohe Meldungen wie „Request failed with status 500“ oder „RSS channel wurde nicht gefunden“. Das ist eine Mischung aus Deutsch und Englisch, ohne Mehrwert für Nutzer.
- PodcastFilter.vue: Der Submit-Button hat id="podcast-reset", ist aber kein Reset-Button. Die Kategorienamen sind auf Englisch, obwohl die UI sonst deutsch ist.
- types.ts: SolrDocument-Felder sind als required typisiert, werden aber überall mit ?. gelesen. Die Felder sollten optional typisiert werden, damit Typen und Nutzung übereinstimmen.
- PlaylistEpisode.vue:233: Der Selektor [data-play-icon] greift nicht, weil es kein Element mit diesem Attribut gibt.
|
TODO:
|
Link to jira