Skip to content

refactor(api)!: json data for ContentNode (new approach)#2825

Merged
usu merged 22 commits into
ecamp:develfrom
usu:chore/content-node-json2
Sep 6, 2022
Merged

refactor(api)!: json data for ContentNode (new approach)#2825
usu merged 22 commits into
ecamp:develfrom
usu:chore/content-node-json2

Conversation

@usu

@usu usu commented Jun 11, 2022

Copy link
Copy Markdown
Member

This is an alternative POC for leveraging json data:

  • keeps ContentNode subclasses, but changes to Single Table Inheritance (thus avoiding all the extra joins for every subclass)
  • still utilizes $data JSONB property for most use cases (but additional data could be linked for more complex cases)
  • no switch statements anymore 😎

Replaces the POC discussed under https://github.com/usu/ecamp3/pull/8/commits

Open ToDos to make tis POC mergeable

  • fix tests
  • fix frontend
  • fix printing
  • migrate dev data (or start from scratch?)
  • implement copyFromPrototype
  • verify Swagger output
  • fix frontend tests

To discuss

  • keep reference to materialItems from materialNode? (I tend to pay the small performance impact and say yes to simplify coding)

To do (separate PR?)

Time measurement

(contentNodes timing not fully comparable, as data is not migrated)

Endpoint Collection ms Entity ms
activities 240 114
contentNodes 232 73
columnLayouts 126 74
multiSelects 70 68
singleTexts 112 70
storyboards 88 69
materialNodes 82 71
days 82 68
categories 102 83
contentTypes 58 52
camps 84 83
periods 87 85
materialLists 74 62
scheduleEntries 102 81
materialItems 90 70
activityResponsibles 65 65
dayResponsibles 61 64
campCollaborations 103 70

@usu usu added the Meeting Discuss Am nächsten Core-Meeting besprechen label Jun 13, 2022
@manuelmeister manuelmeister removed the Meeting Discuss Am nächsten Core-Meeting besprechen label Jun 13, 2022
Comment thread api/src/Entity/ContentNode.php Outdated
Comment thread api/src/Entity/HasRootContentNodeTrait.php
Comment thread api/src/Entity/Period.php
Comment thread api/src/Repository/FiltersByContentNode.php Outdated
Comment thread api/src/Util/JsonMergePatch.php Outdated
Comment thread api/src/Util/JsonMergePatch.php
Comment thread api/src/Util/JsonMergePatch.php Outdated
@usu usu marked this pull request as ready for review August 23, 2022 05:13
@usu

usu commented Aug 23, 2022

Copy link
Copy Markdown
Member Author

PR would we ready for review. @pmattmann & @carlobeltrame, do you have a chance to look at it before the meeting this week?

@usu usu added the deploy! Creates a feature branch deployment for this PR label Aug 24, 2022
@usu usu removed the deploy! Creates a feature branch deployment for this PR label Aug 25, 2022

@carlobeltrame carlobeltrame left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Von nun an ist es für jeden neuen ContentNode mit Daten nötig, zusätzlich einen DataPersister zu schreiben in dem Validierungen und InputFilters ausprogrammiert werden müssen, mit einem anderen System als bei der "normalen" ContentNode Entity. Das müssen wir dann gut dokumentieren, oder vielleicht finden wir noch einen Weg das zu vereinfachen / wieder zu vereinheitlichen.
Weitere Nachteile der JSON-Daten: Es wird schwieriger werden, nur einzelne Datenpunkte auszutauschen, wenn wir mal Live-Updates einführen. Und es wird schwieriger, von ContentNode-Daten aus andere Entities zu referenzieren (Prime-Beispiel sind Kommentare, oder auch z.B. Links zu anderen Activities, wobei man dann wohl mit GUIDs und ausführlichen Fallbacks arbeiten kann...)

Comment thread api/migrations/schema/Version20220611193723.php Outdated
Comment thread api/src/DataPersister/ActivityDataPersister.php Outdated
Comment thread api/src/Entity/ContentNode.php
Comment thread api/src/Entity/ContentNode/SingleText.php Outdated
Comment thread api/src/Entity/ContentNode/MultiSelect.php Outdated
Comment thread api/src/Util/JsonMergePatch.php
Comment thread frontend/src/components/activity/content/Storyboard.vue
Comment thread frontend/src/components/form/api/ApiSortable.vue
Comment thread frontend/src/mixins/apiPropsMixin.js
@usu usu added Meeting Discuss Am nächsten Core-Meeting besprechen and removed Meeting Discuss Am nächsten Core-Meeting besprechen labels Aug 25, 2022
@usu

usu commented Aug 30, 2022

Copy link
Copy Markdown
Member Author

@carlobeltrame Ready for delta-review. All open topics/workarounds are registered as issues.

@BacLuc BacLuc requested a review from carlobeltrame September 6, 2022 06:56
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.

5 participants