Skip to content

Resolve "Mock-API für PublicationItemVisibility einführen"

Closes #829

Anmerkungen zum Review:

  1. Es wurde ein "Proposal"-commit gepusht, mit allen Spitzfindigkeiten und Nützlichkeiten, die ich im Review entdeckt habe. Bitte einmal durchsehen - ich mache gern Rückgängig, was für nicht sinnvoll gehalten wird. Die nennenswerten Anpassungen sind:

    • Die types PublicationVisibilityServiceResult und PublicationCategoryServiceResult (u.ä.) wurden aus folgenden Gründen entfernt:
        1. Diese Art Typen waren in der ersten Iteration nützlich, weil sie die returns für sync und async gekapselt haben. Da wir aktuell nur noch async verwenden, ist diese zusätzliche Abstraktionsebene nicht mehr nötig und wir können mit den return Typen direkt arbeiten.
        1. Es ist irreführend, weil mit diesen Typen suggeriert wird, dass es immer einen einheitlichen Return-Typ gibt, es werden aber unterschiedliche Ergebnisse zurückgegeben (und eine noch größere Differenzierung ist evtl. sinnvoll, s.u.).
        1. Es fügt beim Heraussuchen, um welchen Typ es sich handelt einen zusätzlichen Schritt hinzu. (PublicationCategoryServiceResult -> PublicationCategoryCollection -> PublicationItemCategory)
        1. Es spart kaum Zeichen
    • Async functions verwendet, statt in den returns Promise.resolve() immer explizit zu setzen.
    • Dokumentation erweitert/geupdated.
    • publicationEndDateTime und PublicationStartDateTime wurden umbenannt in publicationEnd und publicationStart. Begründung: Da nur jeweils ein timeStamp für Start und Ende vorgesehen sind, ist diese Spezifität hier unnötig. Dem Benutzer wird damit nur suggeriert, nach anderen keys für Start/End-timestamps (z.B. publicationEndDate oder publicationEndTime) Ausschau zu halten.
    • In update-Funktionen ist es immer sehr nützlich, wenn man nicht das komplette Item angeben muss, sondern nur die Felder, die sich ändern. Deshalb habe ich einen Typ PartialWithId<T> eingefügt, der ein komplettes Item optional macht, aber die id required lässt. Alle Update-Funktionen wurden damit erweitert.
    • Der PaginationInfo type wurde in types/Index.ts eingefügt, weil es ein generischer Typ ist, der in allen Services verwendet werden kann und nicht zu erwarten ist, dass weitere Pagination Typen hinzukommen (was eine eigene Datei rechtfertigen würde). Die imports wurden angepasst.
  2. Weitere Anmerkungen:

    • Es wäre evtl. sinnvoll darüber nachzudenken in (update- und) delete-Functionen nicht immer die komplette geupdatete Item-Liste zurückzugeben. Es scheint zwar auf den ersten Blick nützlich, sollte aber geprüft werden, ob der so zusätzlich entstehende Aufwand dadurch gerechtfertigt wird, dass sie tatsächlich verwendet werden. (Üblicher ist es ja das gelöschte Item oder die Id des gelöschten Items zurückzugeben.) Das erzeugt potentiell eine Menge Zusatzarbeit im BE (Besonders wird das deutlich im deletePublicationItemAndContent() im PublicationItemMock.service.) Außerdem könnte man sich so den zusätzlichen DataOption-Parameter in den Löschfunktionen sparen, den ich eigentlich nie angeben will, wenn ich einfach nur ein Item löschen möchte.
    • getPublicationItem und ähnliche Funktionen, die ein "ItemById" zurückgeben, sollten die Möglichkeit haben einen eindeutigen State für „nicht-gefunden“ (=null) zurückzugeben. So wird es dem Konsumenten dieser Funktion wesentlich vereinfacht auf invalide Daten zu reagieren. Wenn der Nutzer immer ein leeres, aber valides Item bekommt, kann es leicht passieren, dass er Probleme mit fehlenden Daten im Rendering bekommt.
Edited by Michael Voigt

Merge request reports