[NodeBundle] save on (un)publish #1803
Conversation
deZinc
commented
Feb 1, 2018
•
deZinc
commented
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Fixed tickets | closes #1581 |
deZinc
force-pushed
the
fix/issue-1581-merged
branch
from
430ee25
to
5828cdd
Compare
deZinc
changed the title
|
@deZinc Why don't we put the helper methods inside the NodeAdminPublisher server? There is a service for publishing nodes, so maybe this helper can be moved to the service? |
|
@sandergo90 I thought about that too Sander, but decided to keep the methods in the controller as it's the only place they are used. Moving them however is of course a possibility. |
|
I think that it can be moved. It's the right place for it and is not an action that should be in the controller. |
|
Ok, I'll look into it. |
deZinc
force-pushed
the
fix/issue-1581-merged
branch
2 times, most recently
from
28dcd12
to
dcd5a04
Compare
|
@sandergo90 I made the requested changes. |
| @@ -91,6 +91,7 @@ protected function init(Request $request) | |||
| $this->authorizationChecker = $this->get('security.authorization_checker'); | |||
| $this->user = $this->getUser(); | |||
| $this->aclHelper = $this->get('kunstmaan_admin.acl.helper'); | |||
| $this->nodePublisher = $this->get('kunstmaan_node.admin_node.publisher'); | |||
There was a problem hiding this comment.
Add a protected variable for the class.
| @@ -57,13 +67,17 @@ public function __construct( | |||
| TokenStorageInterface $tokenStorage, | |||
| AuthorizationCheckerInterface $authorizationChecker, | |||
| EventDispatcherInterface $eventDispatcher, | |||
| CloneHelper $cloneHelper | |||
| CloneHelper $cloneHelper, | |||
There was a problem hiding this comment.
Adding extra constructor parameters is BC breaking.
There was a problem hiding this comment.
True, this should now go into 6
| $request->get('pub_date') . ' ' . $request->get('pub_time') | ||
| ); | ||
| $this->publishLater($nodeTranslation, $date); | ||
| $this->session->getFlashBag()->add( |
There was a problem hiding this comment.
The session can be retrieved from the Request
| $this->publishLater($nodeTranslation, $date); | ||
| $this->session->getFlashBag()->add( | ||
| FlashTypes::SUCCESS, | ||
| $this->translator->trans('kuma_node.admin.publish.flash.success_scheduled') |
There was a problem hiding this comment.
Maybe pass the translator from the NodeAdminController
There was a problem hiding this comment.
I'll have a look into this, because it means this can be done without a BC break then.
But, I think this is a bit cleaner, so'll make a different PR to go into the 6 branch.
| <a href="{{ path('KunstmaanNodeBundle_nodes_publish', { 'id': node.id}) }}" class="btn btn-danger btn--raise-on-hover"> | ||
| {{ 'kuma_node.modal.publish.now.button'|trans() }} | ||
| </a> | ||
| <button name="publishing" type="submit" class="btn btn-danger btn--raise-on-hover">{{ 'kuma_node.modal.publish.now.button'|trans() }}</button> |
There was a problem hiding this comment.
Why are you removing the form action here ?
There was a problem hiding this comment.
Because this needs to become part of the bigger form in order for the new functionality to work.
deZinc
force-pushed
the
fix/issue-1581-merged
branch
from
dcd5a04
to
be48479
Compare
|
@sandergo90 Ready, refactored in order to prevent a BC break |
There was a problem hiding this comment.
Hi @deZinc, your PR needs some changes
- Your answer if this PR is has deprecations seems to be incorrect.
- This PR seems to need a milestone of a patch release.
There was a problem hiding this comment.
Hi @deZinc, your PR passed all our requirements.
Thank you for contributing!
* 5.0: recompiled assets update changelog refactor HeaderPagePart entity test (#2024) [NodeBundle] fixed issue 1581, save on (un)publish (#1803) Replace deprecated twig raw tag in scss file (#1987) [KunstmaanAdminBundle]: fix js for collections (#1989) [Docs] set correct paths (#1990) [AdminBundle] fixed create user command #1995 (#1996)
* master: (24 commits) added missing translations to main language (#2038) bump node version (#2035) recompile assets after upmerge recompiled assets update changelog [NodeBundle] Split off logic of NodeAdminController in helper class (#2027) refactor HeaderPagePart entity test (#2024) [AllBundle] move old phpunit style to codeception with fixed coverage (#1955) [Docs] remove vagrant references and put some small snippet about docker in (#1953) Add target blank to download link (#1963) [NodeBundle] fixed issue 1581, save on (un)publish (#1803) Replace deprecated twig raw tag in scss file (#1987) [KunstmaanAdminBundle]: fix js for collections (#1989) [Docs] set correct paths (#1990) [AdminBundle] fixed create user command #1995 (#1996) [AdminBundle] Deprecate service container usage in commands (#2014) [AdminBundle] updated aclApplyCommand to use AclManager (#2003) Replace getParameter calls so our code doesn't trigger deprecations (#2012) [AdminBundle][MultidomainBundle] Move parameter checks and service definition changes to compiler pass (#2018) Mark extra services public (#2013) ...
