sulu / SuluFormBundle

Form Bundle for handling Dynamic and Symfony Forms in https://sulu.io
MIT License
81 stars 78 forks source link

createCollection for tree strategy not working #354

Open matthiasseghers opened 1 year ago

matthiasseghers commented 1 year ago
Q A
Bug? yes
New Feature? no
Bundle Version 2.5.1
Sulu Version 2.5.6
Browser Version Firefox 111.0b8 (64-bit)

Actual Behavior

When uploading a file using the "attachment" field, a 500 occurs when submitting the form.

When using the collection strategy "tree", I am getting a 500 error stating that $title needs to be string, but null is given at CollectionTreeStrategy.php at line 86. After further investigation, I noticed that in the collection strategy at line 60, this variable is supposedly being set. However, this is null in my case. Since the title is null but is required for the createCollection method, I'm wondering if I'm missing something, or if this is a bug (for what I'll be creating an issue if this is the case). When looking inside the getTitle method, I noticed that of the check returning null, (!$structure instanceof StructureInterface || $structure->getUuid() !== $typeId at line 38 in the StructureTitleProvider seems to fail on the $structure->getUuid() !== $typeId check. Since I'm unsure what the logic behind the whole structure is, I'm unsure if this is a bug, or if I'm missing something in my code to prevent this error.

Expected Behavior

The form to submit and a collection to be made for the form uploads.

Steps to Reproduce

Setting the sulu form collection strategy to "tree". Adding an attachment type to the form, loading the form on a template and trying to submit by uploading a file.

alexander-schranz commented 1 year ago

Are you using the form on a Sulu Page or Article page?

What is returned here? https://github.com/sulu/SuluFormBundle/blob/6c424abf8966d081f2366ac2af8e5f4b840a9aef/TitleProvider/StructureTitleProvider.php#LL38C9-L38C94

What values do exist in the attributes bag of the request: $request->attributes->all()?

Since I'm unsure what the logic behind the whole structure is, I'm unsure if this is a bug, or if I'm missing something in my code to prevent this error.

The structure is the Page or Article using the Form and the StructureTitleProvider is responsible for providing the Title of that page to create the new Collection.

matthiasseghers commented 1 year ago

I am currently using this on a Sulu page.

https://github.com/sulu/SuluFormBundle/blob/6c424abf8966d081f2366ac2af8e5f4b840a9aef/TitleProvider/StructureTitleProvider.php#LL38C9-L38C94 returns false. It seems that both Uuids that are being compared differ ($structure->getUuid() !== $typeId).

[StructureTitleProvider.php](http://127.0.0.1:8000/_profiler/open?file=vendor/sulu/form-bundle/TitleProvider/StructureTitleProvider.php&line=37#line37) on line 37:
array:9 [▼
  "_stopwatch_token" => "3d17b5"
  "_fos_rest_zone" => false
  "_sulu" => [Sulu\Component\Webspace\Analyzer\Attributes\RequestAttributes](http://127.0.0.1:8000/_profiler/open?file=vendor/sulu/sulu/src/Sulu/Component/Webspace/Analyzer/Attributes/RequestAttributes.php&line=17#line17) {#2352 ▶}
  "routeDocument" => [Symfony\Component\Routing\Route](http://127.0.0.1:8000/_profiler/open?file=vendor/symfony/routing/Route.php&line=20#line20) {#2975 ▶}
  "_controller" => "Sulu\Bundle\WebsiteBundle\Controller\DefaultController::indexAction"
  "structure" => [Sulu\Component\Content\Compat\Structure\PageBridge](http://127.0.0.1:8000/_profiler/open?file=vendor/sulu/sulu/src/Sulu/Component/Content/Compat/Structure/PageBridge.php&line=16#line16) {[#2851 ▶](http://127.0.0.1:8000/en/events/test#sf-dump-1766926474-ref22851)}
  "partial" => false
  "_route" => "vacancy_28270cbe-0cf0-4131-8765-c867aab7f1a8"
  "_route_params" => array:2 [▶]
]

I did notice that the hash after the route name is the same as the one inside $structure->getUuid() but different that the one provided by $typeId.

alexander-schranz commented 1 year ago

Where does the page load the form from? The structure uuid normally should be the same or are you doing some cross includes that the uuids do not match?

Is the single_form_selection on the current page you are submitting the form? And is the uuid here correctly:

https://github.com/sulu/SuluFormBundle/blob/6c424abf8966d081f2366ac2af8e5f4b840a9aef/Content/Types/SingleFormSelection.php#L110-L116

matthiasseghers commented 1 year ago

It does not seem to reach the loadShadowForm function. I am indeed doing some cross includes, the form is being set in a snippet. On the template, it uses the webspaceSettings to get the form.
Afterwards, it gets the forms using {% set form = sulu_form_get_by_id(form.vars.value.form.id, 'page', form.vars.value.type) %}.

alexander-schranz commented 1 year ago

Sorry this would be correct code part in the SingleFormSelection: https://github.com/sulu/SuluFormBundle/blob/6c424abf8966d081f2366ac2af8e5f4b840a9aef/Content/Types/SingleFormSelection.php#L77-L83

I think the problem here is that it comes from a default snippet and can so not correctly detect the title. Not sure how we should behave in this situation, as the saved data seems not connected to the page or is it. What typeId is send as post with the form the snippet uuid or the page uuid? Also not sure what the correct behaviour here should be, if the data should be part of that snippet or part of a page.

matthiasseghers commented 1 year ago

Inside the SingleFormSelection, the uuid is the same as the $typeId variable (matching the one from the page). I just checked and the ID that $structure->getUuid() inside StructureTitleProviderreturns is the ID of the snippet. I'm thinking this is more caused by my cross-referencing being an edge-case scenario than a real bug.

What I tried to achieve with my approach is to have a dynamic form reusable. I want the content manager to be able to create a form that's applied to all templates of a certain type. The reason is that the form configuration is the same, the only difference is the origin being tracked. I'm not sure that my approach is the ideal or correct one.

matthiasseghers commented 1 year ago

Any advice on how I would be able to make one form, that's reusable on multiple pages? I would like to have the end user define the different form fields in the admin site. It should also keep track of what page it's submitted on. Is this possible with the dynamic forms? Or can I only achieve this by using a custom Symfony Form instead, taking away the dynamical part that the bundle provides?