Closed alexandersch closed 1 year ago
Hello everybody!
We would also be interested to have this feature and are happy to help making it happen.
We noticed that there is already some logic in the current version for copying articles so it looks like this feature was already there in an older version of the bundle: see https://github.com/sulu/SuluArticleBundle/blob/2.x/Controller/ArticleController.php#L565
With some hacks it's pretty easy to get it working:
deleteUnavailableLocales
when calling $webspace->getAllLocalizations()
handleCopy
when calling the chainRouteGenerator
CopyToolbarAction
that was implemented in https://github.com/sulu/sulu/pull/6475 to provide the locale when triggering the copy
action in the controller So what do you think?
Hi, I was wondering if someone could give me a quick feedback. If there is interest I could start working on a PR.
Hi, I was wondering if someone could give me a quick feedback. If there is interest I could start working on a PR.
@rogamoore It would be nice if you could contribute this feature to the bundle.
👋 Any update on this?
@mario-fehr @Jelmer7 Please see the "Next steps?" section on my post. I need feedback from someone (Sulu Team) on the two subscribers that are failing. They are not part of the SuluArticleBundle, but of Sulu Core and I have not enough knowledge at the moment to fix this part.
@alexander-schranz Sorry to ping you, but as we have some time available to contribute to this feature, I would like to ask for a quick opinion/feedback of the Sulu team regarding the above questions. If it's a bad moment, just let me know, otherwise I think it could be a great addition to the next release.
@rogamoore I think the copy function should be very similar to the snippet copy function which was lately implemented here by @wachterjohannes: https://github.com/sulu/sulu/pull/6859/files. Let me know if you can get it based on https://github.com/sulu/sulu/pull/6859/files get it work also for articles.
The deleteUnavailableLocales
sounds like something which can be skipped in copy where no locale exist.
The RoutableSubscriber
sounds required. It is strange that it fails here and the documentInspector can not get all locales? Or what is the current stack trace here?
@alexander-schranz Thanks for your feedback! I've checked the PR you've linked (https://github.com/sulu/sulu/pull/6859/files) and my changes are pretty much the same (which I think is a good sign) with the difference that after copying the article it's not published immediately, but opened in draft mode.
The linked PR also fixes one of my problems mentioned above, where it fails in the WebspaceSubscriber
.
So right now there is only one issue left (all other details can be discussed in the upcoming PR).
RoutableSubscriber
calls the ChainRouteGenerator
here: https://github.com/sulu/sulu/blob/2.5/src/Sulu/Bundle/RouteBundle/Document/Subscriber/RoutableSubscriber.php#L278
$route = $this->conflictResolver->resolve($this->chainRouteGenerator->generate($localizedDocument));
generate()
, which is $path
if
-Condition here: https://github.com/sulu/sulu/blob/0c564eb9b8df8a163dc6a4381c710b003f80641a/src/Sulu/Bundle/RouteBundle/Generator/ChainRouteGenerator.php#L52
if (!$path) {
$generator = $this->routeGenerators[$config['mapping']['generator']];
$path = $generator->generate($entity, $config['mapping']['options']);
}
Warning: Undefined array key ""
$config
variable at this point:
array:2 [
"className" => "Sulu\Bundle\ArticleBundle\Document\ArticleDocument"
"mapping" => array:3 [
"resource_key" => "articles"
"generator" => null
"options" => []
]
]
generator
is null
, but will cast into ""
when accessing the $this->routeGenerators
array.$this->routeGenerators
:
array:1 [
"null" => Sulu\Bundle\RouteBundle\Generator\NullRouteGenerator
]
Here is the stack trace:
ErrorException:
Warning: Undefined array key ""
at vendor/sulu/sulu/src/Sulu/Bundle/RouteBundle/Generator/ChainRouteGenerator.php:53
at Sulu\Bundle\RouteBundle\Generator\ChainRouteGenerator->generate()
(vendor/sulu/sulu/src/Sulu/Bundle/RouteBundle/Document/Subscriber/RoutableSubscriber.php:278)
at Sulu\Bundle\RouteBundle\Document\Subscriber\RoutableSubscriber->handleCopy()
(vendor/symfony/event-dispatcher/EventDispatcher.php:270)
at Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}()
(vendor/symfony/event-dispatcher/EventDispatcher.php:230)
at Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
(vendor/symfony/event-dispatcher/EventDispatcher.php:59)
at Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
(vendor/sulu/sulu/src/Sulu/Component/DocumentManager/DocumentManager.php:85)
at Sulu\Component\DocumentManager\DocumentManager->copy()
(vendor/sulu/article-bundle/Controller/ArticleController.php:565)
at Sulu\Bundle\ArticleBundle\Controller\ArticleController->postTriggerAction()
(vendor/symfony/http-kernel/HttpKernel.php:163)
at Symfony\Component\HttpKernel\HttpKernel->handleRaw()
(vendor/symfony/http-kernel/HttpKernel.php:75)
at Symfony\Component\HttpKernel\HttpKernel->handle()
(vendor/symfony/http-kernel/Kernel.php:202)
at Symfony\Component\HttpKernel\Kernel->handle()
(public/index.php:77)
So now this raises a few questions:
generator
is null
, should the NullRouteGenerator
be called? If so, it looks like a bug regarding the way the array key is used.ArticleRouteGeneratorByType
, ArticleRouteGeneratorByTemplate
and ArticlePageRouteGenerator
in there when using the article bundlegenerate
method even be called without a $path
? So, I found that the reason why generator
is null
is because in my config there is no mapping defined:
# missing
sulu_route:
mappings:
Sulu\Bundle\ArticleBundle\Document\ArticleDocument:
generator: schema
options:
route_schema: '/articles/{implode("-", object)}'
The reason is that I have configured the route schema per template as described here: https://github.com/sulu/SuluArticleBundle/blob/2.x/Resources/doc/routing.md#overwrite-route-schema-in-template
<property name="routePath" type="route" mandatory="true">
<meta>
<title>sulu_admin.url</title>
</meta>
<params>
<param name="route_schema" value="/{translator.trans('article.routes.events')}/{implode('-', object)}"/>
<param name="mode" value="leaf"/>
</params>
<tag name="sulu_article.article_route"/>
</property>
So this raises a new question: If the route schema is defined in the template, how to make sure it's also used when copying an article?
Meanwhile I will test the yaml config to see, if I can get past the error with this.
I can get past the error, when using this mapping (not the default from the docs):
sulu_route:
mappings:
Sulu\Bundle\ArticleBundle\Document\ArticleDocument:
generator: schema
options:
route_schema: '/articles/{is_array(object) ? implode("-", object) : object.getTitle()}'
Without the default from the docs I'm getting
implode(): Argument #2 ($array) must be of type ?array, Sulu\\Bundle\\ArticleBundle\\Document\\ArticleDocument given
My next step will be preparing a PR, but I will need some help with this route generating stuff, because IMHO it should work with the default YAML config and also when using the route schema per template config.
I think this issue can be closed now that #610 has been merged - the feature will be available in the next release (2.5.0 I guess)
I would like to request a copy article feature. Would be a nice addition to the article bundle.