Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed order of handle-route (update before create) #99

Merged
merged 1 commit into from
Mar 14, 2017

Conversation

wachterjohannes
Copy link
Member

@wachterjohannes wachterjohannes commented Mar 14, 2017

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets none
Related issues/PRs none
License MIT
Documentation PR none

What's in this PR?

The order of update and create route is important if you want to create an article with an existing route.

Additional changes:

  • added route_path to create
  • fixed delete selected in list

@wachterjohannes wachterjohannes added this to the 0.3.0 milestone Mar 14, 2017
@wachterjohannes wachterjohannes force-pushed the bugix/route-generation-on-create branch 2 times, most recently from 922d268 to 1b1167e Compare March 14, 2017 09:03
$article->setAuthorId($author->getId());
}
if ($document->getAuthor() && $author = $this->contactRepository->findById($document->getAuthor())) {
$article->setAuthorFullName($author->getFullName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wachterjohannes Why you are not setting the author id anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have only written it in another format - to make it more readable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have changed the function...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following line is missing:

$article->setAuthorId($author->getId());

$article->setChangerContactId($changer->getContact()->getId());
}
if ($document->getChanger() && $changer = $this->userManager->getUserById($document->getChanger())) {
$article->setChangerFullName($changer->getFullName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here (:

$article->setCreatorContactId($creator->getContact()->getId());
}
if ($document->getCreator() && $creator = $this->userManager->getUserById($document->getCreator())) {
$article->setCreatorFullName($creator->getFullName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here (:

@wachterjohannes wachterjohannes force-pushed the bugix/route-generation-on-create branch from 1b1167e to 9f23a33 Compare March 14, 2017 09:11
@wachterjohannes
Copy link
Member Author

@trickreich fixed.

@@ -102,7 +102,7 @@ public static function getSubscribedEvents()
{
return [
Events::HYDRATE => [['handleHydrate', -500]],
Events::PERSIST => [['handleRoute', 0], ['handleRouteUpdate', 0], ['handleScheduleIndex', -500]],
Events::PERSIST => [['handleRouteUpdate', 1], ['handleRoute', 0], ['handleScheduleIndex', -500]],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we make here bigger steps..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is here important that this is near to each other and in the right order. if you use a bigger priority you have no route - if you use a smaller you have one.

@@ -277,7 +277,10 @@ define([
},

deleteItems: function(ids) {
this.sandbox.util.save('/admin/api/articles?ids=' + ids.join(','), 'DELETE').then(function() {
this.sandbox.util.save(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use here the ArticleManager

@wachterjohannes wachterjohannes force-pushed the bugix/route-generation-on-create branch from 9f23a33 to 084b4d6 Compare March 14, 2017 09:19
@wachterjohannes
Copy link
Member Author

@trickreich fixed

@trickreich
Copy link
Contributor

ack

@alexander-schranz alexander-schranz merged commit 21f0e75 into develop Mar 14, 2017
@wachterjohannes wachterjohannes deleted the bugix/route-generation-on-create branch March 14, 2017 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants