contao-community-alliance / dc-general

Universal data container for Contao!
https://dc-general.readthedocs.io/de/latest/
GNU Lesser General Public License v3.0
16 stars 21 forks source link

Copy action does not work #484

Closed baumannsven closed 4 years ago

baumannsven commented 5 years ago

See by the issue from metamodels core.

https://github.com/MetaModels/core/issues/1300

richardhj commented 5 years ago

Please check if you can reproduce it? :-)

baumannsven commented 5 years ago

Can you send me the complete stack trace as mail. Thanks.

richardhj commented 5 years ago

I tested in our test environment:

The URI of the copy button is http://test.metamodel.me/contao?do=metamodel_mm_test&&act=copy&source=mm_test::2&rt=_eF_flmqs07lyqBM4RJXMTGTSxvRsnBe36p-upG-1Xo&ref=WY8dlNxK After I clicked on the button, the URI is http://test.metamodel.me/contao?do=metamodel_mm_test&table=mm_test&act=edit&id=mm_test::6&pid&rt=_eF_flmqs07lyqBM4RJXMTGTSxvRsnBe36p-upG-1Xo

So where does the pid parameter come from?

richardhj commented 5 years ago

Stack trace is available via the sentry link.

ContaoCommunityAlliance\DcGeneral\Exception\DcGeneralRuntimeException: Unparsable encoded id value: ''
#24 vendor/contao-community-alliance/dc-general/src/Data/ModelId.php(117): fromSerialized
#23 vendor/contao-community-alliance/dc-general/src/Controller/ModelCollector.php(146): getModel
#22 vendor/contao-community-alliance/dc-general/src/EventListener/ModelRelationship/TreeEnforcingListener.php(78): process
#21 vendor/symfony/event-dispatcher/EventDispatcher.php(212): doDispatch
#20 vendor/symfony/event-dispatcher/EventDispatcher.php(44): dispatch
#19 vendor/contao-community-alliance/dc-general/src/Contao/View/Contao2BackendView/EditMask.php(780): execute
#18 vendor/contao-community-alliance/dc-general/src/Contao/View/Contao2BackendView/ActionHandler/EditHandler.php(120): process
#17 vendor/contao-community-alliance/dc-general/src/Contao/View/Contao2BackendView/ActionHandler/EditHandler.php(82): handleEvent
#16 vendor/symfony/event-dispatcher/EventDispatcher.php(212): doDispatch
#15 vendor/symfony/event-dispatcher/EventDispatcher.php(44): dispatch
#14 vendor/contao-community-alliance/dc-general/src/Controller/DefaultController.php(158): handle
#13 vendor/metamodels/core/src/BackendIntegration/Module.php(74): generate
#12 vendor/contao/core-bundle/src/Resources/contao/classes/Backend.php(420): getBackendModule
#11 vendor/contao/core-bundle/src/Resources/contao/controllers/BackendMain.php(169): run
#10 vendor/contao/core-bundle/src/Controller/BackendController.php(48): mainAction
#9 vendor/symfony/http-kernel/HttpKernel.php(150): handleRaw
#8 vendor/symfony/http-kernel/HttpKernel.php(67): handle
#7 vendor/symfony/http-kernel/Kernel.php(198): handle
#6 vendor/symfony/http-kernel/HttpCache/SubRequestHandler.php(85): handle
#5 vendor/symfony/http-kernel/HttpCache/HttpCache.php(448): forward
#4 vendor/symfony/framework-bundle/HttpCache/HttpCache.php(57): forward
#3 vendor/symfony/http-kernel/HttpCache/HttpCache.php(238): pass
#2 vendor/symfony/http-kernel/HttpCache/HttpCache.php(183): handle
#1 vendor/friendsofsymfony/http-cache/src/SymfonyCache/EventDispatchingHttpCache.php(98): handle
#0 web/app.php(58): null
baumannsven commented 5 years ago

I think, the copy is damaged after replace some deprecations. I look it for.

richardhj commented 5 years ago

An easy fix would be to check '' !== $pid = $input->getParameter('pid') in TreeEnforcingListener:77

discordier commented 5 years ago

Can't do so as even '' is a perfectly legal id. Imagine a data provider that only allows to edit one dataset, that one may have an empty id then.

baumannsven commented 5 years ago

I think extend the if statement here https://github.com/contao-community-alliance/dc-general/blob/034702e2220558b26d41b542b17fd1923778dff7/src/EventListener/ModelRelationship/TreeEnforcingListener.php#L77 for pid has an valid serialized model id.

@discordier Do you agree with me?

baumannsven commented 5 years ago

@richardhj Sorry I missed your comment :(

discordier commented 5 years ago

IMO we should not monkey patch here. We must ensure that the parameter is not present instead!

This is also for future consistency where we will move to symfony forms and the like which will bail if additional parameters are given.

zonky2 commented 5 years ago

at DCG/MM 2.1 and Contao 4.4 works copy fine - maybe the reason is Contao 4.7

baumannsven commented 4 years ago

This has been fixed long time ago.