-
Notifications
You must be signed in to change notification settings - Fork 61
Refactor Backend and Frontend for Activity-Layout #950
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich habe kurz case-insensitive nach "activitycate", "activity-cate", "activity_cate", "contenttypeconf", "content-type-conf" und "content_type_conf" durch den Backend-Code gesucht. Alte Bezeichnungen kommen noch in einigen Strings vor, und (wichtiger) in Use Statements in ScheduleEntryTestData.php und Rest\CampTest.php, die ich in diesem Review nicht markieren kann.
'eCampApi\\V1\\Rest\\ActivityCategory\\Validator' => [ | ||
'eCampApi\\V1\\Rest\\Category\\Validator' => [ | ||
0 => [ | ||
'name' => 'short', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wären Filter oder Validierungen nötig / sinnvoll für die neuen Felder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ja - siehe #827, mach ich dann mit den ConfigFactories
|
||
public function removeChild(ActivityContent $activityContent): void { | ||
$activityContent->setParent(null); | ||
$this->children->removeElement($activityContent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removeElement
schlägt glaube ich nicht fehl falls das Element nicht gefunden wird. Müssten wir in solche Methoden einen Safeguard einbauen der auf Plausibilität checkt, bevor $activityContent->setParent(null)
ausgeführt wird?
backend/module/eCampCore/src/EntityService/CategoryContentTypeTemplateService.php
Outdated
Show resolved
Hide resolved
backend/module/eCampCore/test/Data/CategoryTemplateTestData.php
Outdated
Show resolved
Hide resolved
backend/module/eCampCore/src/EntityService/CategoryContentTypeService.php
Outdated
Show resolved
Hide resolved
Das hatte ich auch - sollte aber korrigiert sein. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sehr cool, wir sind auf der Zielgerade.
backend/module/eCampCore/src/EntityService/ActivityContentService.php
Outdated
Show resolved
Hide resolved
Im Frontend scheint auf den ersten Blick beim Durchklicken alles zu funktionieren. Damit #834 wirklich geschlossen werden kann fehlt mir aber noch der Umbau mit dem "Struktur ändern" Button. Das kann man aber gerne noch separat von diesem Refactoring angehen. |
Möchte ich unbedingt trennen. Neue Funktionalität wird bestimmt auch Diskussionen geben. |
Refactoring for Activity-Layout (see #834)