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

Fixed issue #1549 #1550

Merged
merged 1 commit into from
May 23, 2014
Merged

Fixed issue #1549 #1550

merged 1 commit into from
May 23, 2014

Conversation

umpirsky
Copy link
Contributor

No description provided.

@arnolanglade
Copy link
Contributor

Now the build failed for PHP 5.4, I just restarted in case of.. but it should be green :)

@umpirsky
Copy link
Contributor Author

Thanks @Arn0d !

@arnolanglade
Copy link
Contributor

\o/ Now PHP5.4 is green but not PHP5.3 and PHP5.5 !

@umpirsky
Copy link
Contributor Author

OK, then we can merge :)

arnolanglade added a commit that referenced this pull request May 23, 2014
@arnolanglade arnolanglade merged commit f11759b into Sylius:master May 23, 2014
@arnolanglade
Copy link
Contributor

Thanks Saša!

@umpirsky umpirsky deleted the fix/issue-1549 branch May 23, 2014 14:04
@winzou
Copy link
Contributor

winzou commented May 23, 2014

Have you tested it? Not sure if the preflush is right here, I will have a look. Your issue was strange, did you find why exactly your number didn't get persisted?

@arnolanglade
Copy link
Contributor

Oups, the build was green. Perhaps, I click on merge quickly, sorry. Can you give us feedback ?

@umpirsky
Copy link
Contributor Author

I just know that it got flushed after I made this change.

@winzou
Copy link
Contributor

winzou commented May 24, 2014

@umpirsky your fix is not working. It seems that the unit of work returns nothing for scheduledEntityUpdates and scheduledEntityInsertions in the preFlush event => the listener doesn't go in the foreach and so no generator is called. I'm testing with the classic checkout workflow. Please revert or fix asap.

@pjedrzejewski
Copy link
Member

Right, it does not work and all number are empty now... We should probably have a scenario to cover the numbers generation to have this issue solved once and for all...

Btw. latest master demo is available at http://master.sylius.sylius.stage1.io/. (I will set demo.sylius.org to use stage1 soon)

@umpirsky
Copy link
Contributor Author

$this->listener->enableEntity($order); is never called. Isn't this maybe related to removing OrderNumberListener form config and finite does not doing its job @winzou ?

@winzou
Copy link
Contributor

winzou commented May 25, 2014

@umpirsky I'm on master and the enableEntity is well called.
In the preFlush method I do:

var_dump($this->entitiesEnabled);
var_dump($uow->getScheduledEntityUpdates());
var_dump($uow->getScheduledEntityInsertions());

And I get:

array (size=1)
  '000000000b52ca3800000000659e9ef9' => boolean true
array (size=0)
  empty
array (size=0)
  empty

So this is really this preFlush that is not working.
If I switch to dd694f243b which is just before the move from onFlush to preFlush then everything is working well.

@umpirsky
Copy link
Contributor Author

I am on master branch too, and I get no dumps, and $this->listener->enableEntity($order); is never called for me.

I am testing with:

$ ./app/console do:fi:lo --no-interaction

How do you test @winzou?

@winzou
Copy link
Contributor

winzou commented May 26, 2014

Same as you. Really strange that the entity is not enabled at your place...
But anyway, it is at mine, and then the preFlush is not working. I've reverted your change + added scenario: #1560

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.

4 participants