Skip to content

Experimental: List patch + sorting #2196

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

Closed
wants to merge 3 commits into from
Closed

Conversation

usu
Copy link
Member

@usu usu commented Nov 7, 2021

What we want to achieve

Um Elemente umzusortieren wollen wir mehrere Elemente einer Liste gleichzeitig patchen, um das position property zu aktualisieren.

Im alten Backend hatten wir das gelöst, in dem wir PATCH auf collections erlaubt hatten, bzw. selbst implementiert haben. Siehe z.B. den PR #689 oder die patchList function im Abstract Entity Service. Der request payload sah dabei so aus:

{
  591add37: {pos: 0}, 
  dca1e48a: {pos: 1}, 
  b1a7d9b2: {pos: 2}
}

List patch in API Platform

Patchen von collections ist in API Platform aktuell nicht implementiert oder vorgesehen.

Mögliche Varianten:

Variante 1 (in diesem Pull request als Draft drin)

Updaten von mehreren Entities via embedded relation: Mittels writableLink: true können mittels POST/PATCH/PUT auf die Owning/Parent-side einer Relation auch Subelemente der Relation erstellt oder aktualisiert werden.

https://api-platform.com/docs/core/operations/

It is also possible to embed a relation in PUT, PATCH and POST requests.

Wir erlauben das heute schon z.B. beim Erstellen von Camps: Perioden können beim Erstellen auch gleich mitgeliefert werden und werden mit-erzeugt.

Laut Doku kann ein @id (oder id?) property mitgeliefert werden, dann wird die die entsprechende Entity aktualisiert. Ansonsten wird eine neue erstellt:

The following rules apply when denormalizing embedded relations:

  • If an @id key is present in the embedded resource, then the object corresponding to the given URI will be retrieved through the data provider. Any changes in the embedded relation will also be applied to that object.

  • If no @id key exists, a new object will be created containing data provided in the embedded JSON document.

Hab ich out-of the Box nicht zum Laufen gebracht. Früher war das anscheinend nur für PUT möglich (wobei API Platform's PUT eigentlich wie ein PATCH funktioniert). Inzwischen ist PATCH auch in der Docu, funktioniert aber dennoch nicht.

Bin noch nicht sicher ob dies ein Bug ist oder ob das davon kommt, dass API Platform versucht, die JSON Merge Patch Spezifikation strikt zu befolgen. Wird auf API Platform auch in mehreren Issues diskutiert. Geholfen hat mir dieser Workaround:
api-platform/core#4293 (comment)

Der ist nun im PR auch implementiert. Ist super hacky und setzt stark voraus, dass man die Interna von API Platform kennt (=Chance gross, dass das dann mit einem Upgrade von API Platform kaputt geht). Aber es funktioniert.

Vorteil: Theoretisch ist der Use-case von API Platform unterstützt (evt. Verbesserungen in zukünftigen Versionen)

Nachteil: Die Subentities werden nicht nur aktualisiert, sondern je nach dem auch erstellt oder entfernt (!). Dieses Verhalten ist bei einem reinen Resorting nicht wirklich gewünscht.

Variante 2: List patch als custom operation

List patch (oder spezifisch nur list sort?) als eigene custom operation implementieren

Variante 3: JSON Patch

Warten, bis JSON Patch format implementiert ist: api-platform/core#759

Variante 4: Nur jeweils eine Entity aktualisieren

Implementieren des Sortierens ohne auf list patch angewiesen zu sein. Das wäre theoretisch in Verbindung mit der doctrine-extension sortable möglich. Allerdings muss dann Frontend-seitig und Backend-seitig die Logik gleichermassen implementiert sein, sonst führt dies zu Inkonsistenzen.
Oder: man setzt ein PATCH auf die verschobene Entity ab und lädt danach die komplette Liste neu.

doctrine extension sortable

Docu hier:
https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/doc/sortable.md

Vorteile/Features:

  • Neue Elemente werden automatisch ans Ende platziert
  • Beim Resorting werden alle position properties automatisch richtig aktualisiert (ich muss nur eine Entity manuell ändern)
  • Kann nach mehreren Feldern gruppieren
  • Position property ist (hoffentlich) immer konsistent

Nachteile:

Alternative 1: Keine Logik Backendseitig einbauen und die Verantwortung an das Frontendend geben, vernünftige Daten in das position Property zu schreiben.

Alternative 2: Kein Bulk-Update erlauben, sondern nur einzelne Entitäten. Dann muss Frontendseitig die Berechnung der Position aber konsistent sein mit der Backend-Logik. Oder ich muss die Collection neu laden, damit ich die neuen position properties aller Entities habe.

Im alten Backend hatten wir lediglich eine Logik drin, welche eine neue Entität ans Ende stellte, wenn das position property beim Erstellen nicht gesetzt wurde:
https://github.com/ecamp/ecamp3/blob/devel/backend/module/eCampCore/src/EntityService/ContentNodeService.php#L109

@usu usu added the Meeting Discuss Am nächsten Core-Meeting besprechen label Nov 9, 2021
@BacLuc
Copy link
Contributor

BacLuc commented Nov 17, 2021

Ist das weiterhin WIP und wir sollen vor dem Meeting uns das anschauen,
oder möchtest du das vor dem Meeting schon mergebar hinkriegen, und dann schauen wir uns das am meeting an?

@usu
Copy link
Member Author

usu commented Nov 18, 2021

Ist das weiterhin WIP und wir sollen vor dem Meeting uns das anschauen,
oder möchtest du das vor dem Meeting schon mergebar hinkriegen, und dann schauen wir uns das am meeting an?

Ne, bitte schon mal anschauen. Die Tests kann ich dann immer noch fixen, aber würde am Meeting gerne zuerst diskutieren, ob der Ansatz Sinn macht oder ob das eher ein Holzweg ist.

*/
private int $pos = 0;
private int $pos = -1;
Copy link
Member

Choose a reason for hiding this comment

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

Das ist damit es ans Ende der Liste kommt, oder?

@manuelmeister
Copy link
Member

Core Meeting

Nur eine Entity patchen, danach selber Liste reloaden

@manuelmeister manuelmeister removed the Meeting Discuss Am nächsten Core-Meeting besprechen label Nov 23, 2021
@usu
Copy link
Member Author

usu commented Nov 25, 2021

Superseded by #2292

@usu usu closed this Nov 25, 2021
@usu usu deleted the feature/sorting branch November 6, 2022 05:52
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.

3 participants