BUGFIX: Two node publishing issues #763
Conversation
aertmann
commented
Sep 17, 2016
aertmann
commented
- Flush routing cache when a node is discarded
- Use publishing service to publish workspace in workspaces module
When the workspace is used to publish the custom logic in the publishing service is bypassed and thus some signals are not fired that are wired up to ensure cache clearing etc.
When a node gets discarded the routing cache needs to be flushed to prevent issues when creating a new node with the same route.
| @@ -395,7 +395,7 @@ public function publishWorkspaceAction(Workspace $workspace) | |||
| if (($targetWorkspace = $workspace->getBaseWorkspace()) === null) { | |||
| $targetWorkspace = $this->workspaceRepository->findOneByName('live'); | |||
| } | |||
| $workspace->publish($targetWorkspace); | |||
| $this->publishingService->publishNodes($this->publishingService->getUnpublishedNodes($workspace), $targetWorkspace); | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Wouldn't it be better to use Workspace.publish() instead also in the WorkspaceController? Publishing a workspace should be something you do on the workspace, and if needed that can use a service to actually do the task.
Or is there a reason other than "make it like in the WorkspaceController for this change?
There was a problem hiding this comment.
@aertmann just shortly discussed this with Karsten: the end result of both calls should be the same, but I really suggest to use $workspace->publish().
Even though both variants currently load all unpublished nodes into memory and then publish them one-by-one, we have the option to optimize $workspace->publish() at some point and come up with a solution which is more memory efficient.
So, if there aren't any other reasons speaking against it, I'd rather use $workspace->publish() everywhere where we want to publish a whole workspace.
There was a problem hiding this comment.
Hey thanks for the feedback. Not sure if you read the commit message? c1f8f72
Anyway sure that would be great, except that the publish method in the workspace model has it's own logic that doesn't use the publishing service which is needed for those signals to be fired.. hence the change. Now I'd rather change it like this to fix the buggy behavior than start rewriting the publish method in the workspace model to use the publishing service since I wouldn't know who's using it and what side-effect it would lead to.
The whole publishing stuff is a bit messy with some logic in the publishing service and some in workspace model. One example is that there are different signals in the workspace models publish method than the publishing services have.
There was a problem hiding this comment.
@aertmann okay, I see. What a mess! But I agree that we should rather fix this bug first and then make sure to find a cleaner solution for the publishing mechanism. How ever it happened that we have two of them, with their individual signals :-/
|
Can you provide a short hint how to reproduce the bug? |
|
yeah sorry, it can be produced by creating a new page and then discarding it again and then trying to create a new page again with the same name, which will give an 404 instead of displaying the new page the other issue is pretty straight forward, but not sure of an easy was to test it other than looking at the code or doing a debug ensuring the signals wired up in Package.php are fired.. |
|
thanks! |
