EZP-27646: Implement editing support for User FieldType #145
Conversation
webhdx
force-pushed
the
ezuser_integration
branch
from
588a472 to
bddebce
Compare
webhdx
force-pushed
the
ezuser_integration
branch
from
bddebce to
145e95f
Compare
webhdx
force-pushed
the
ezuser_integration
branch
from
145e95f to
8f71f77
Compare
webhdx
force-pushed
the
ezuser_integration
branch
3 times, most recently
from
f7b48c3 to
4123999
Compare
webhdx
requested review from
bdunogier,
lserwatka,
adamwojs,
mikadamczyk,
Nattfarinn,
alongosz and
micszo
There was a problem hiding this comment.
wow, that's a large pull-request :)
I'm not gonna dig into a detailed code review, it's not my role anyway 👼 The PR looks really good and clean. But I'll focus on the extensibility of that, meaning customizing the forms when integrating user interaction.
I need to explain what the initial plan, and its benefits, were. I understand that you have built from what was there, and it makes sense as well. Just know that it wasn't the final version of it. Maybe these information can help make some things more consistent.
The way this was done, that you have built from, is that View objects are returned by the various actions (except for UserRegister, btw ?) for the various user actions. The user templates configuration is a custom one, where each can be configured per scope, that is injected using a ViewTemplateListener.
The real plan was to add a ViewBuilder (similar to ContentViewBuilder), that builds the View object based on the Request and the user View configuration (also not implemented). Then the views are rendered by the ViewRenderer (used in your version as well). The main benefits were customizable controllers, as well as conditional overriding of templates (per user content type, for instance).
It would require:
- that the actual action,the code you currently have in the controllers, is moved to the builder. The controller actions themselves will be empty, since they can be overridden.
- that view configuration (as in
content_view) is added - that we have support for matchers that apply to these views
Let me know if you have questions about the above.
| * | ||
| * @return UserCreateView|Response | ||
| * | ||
| * @throws InvalidArgumentType |
There was a problem hiding this comment.
Ain't that a lot of exceptions for such a little method ? :)
I'm wondering if we shouldn't catch and group most of these into exceptions that make sense from the standpoint of a controller.
| ezrepoforms.form_processor.content_type.class: EzSystems\RepositoryForms\Form\Processor\ContentTypeFormProcessor | ||
| ezrepoforms.form_processor.content_type.options.redirect_route_after_publish: ~ | ||
| ezrepoforms.form_processor.content_type.options: | ||
| redirectRouteAfterPublish: "%ezrepoforms.form_processor.content_type.options.redirect_route_after_publish%" | ||
| ezrepoforms.form_processor.content_type_group.class: EzSystems\RepositoryForms\Form\Processor\ContentTypeGroupFormProcessor | ||
| ezrepoforms.form_processor.content.class: EzSystems\RepositoryForms\Form\Processor\ContentFormProcessor | ||
| ezrepoforms.form_processor.user_register.class: EzSystems\RepositoryForms\Form\Processor\UserRegisterFormProcessor | ||
| ezrepoforms.form_processor.user_create.class: EzSystems\RepositoryForms\Form\Processor\User\UserCreateFormProcessor |
There was a problem hiding this comment.
One remark about the services definitions: what about adding a new file for the user stuff, that uses the newer conventions (no %% placeholders for classes names, for instance) ? At least we wouldn't have to change those later on.
It would also help keep the files reasonable in size, and that can't hurt :)
|
Hey @bdunogier. I really like "the master plan" and will happily work on this further more. With current setup there is still a lot of flexibility as developers are able to inject their templates, extend editing types, handle actions differently using custom processors etc. We use that a lot with AdminUI. I like the idea but for the time being I think it's just best to extend what we have. This was my goal with this PR. |
webhdx
force-pushed
the
ezuser_integration
branch
from
4123999 to
6dcab65
Compare
|
No problem @webhdx. I don't imply that this should be applied to this PR anyway, the change is large enough as it is. If it helps you better understand what the intention was, it was good enough. On the long term, it's more about building an architecture we can scale from. The rest is a set of details :) |
webhdx
force-pushed
the
ezuser_integration
branch
from
6dcab65 to
2b9c8b8
Compare
webhdx
force-pushed
the
ezuser_integration
branch
from
23c185a to
66d3e16
Compare
Description
This PR provides basic editing support for:
ezuserEZP-27646: Implement editing support for User FieldTypeRemarks
I had to implement completely different infrastructure for editing Users due to how kernel manages users. My implementation introduces separate action dispatcher with new event names, separated form processors, types, data classes etc.
TODO