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

Catch event stop during create/update #2385

Merged
merged 1 commit into from
Feb 9, 2015

Conversation

amenophis
Copy link
Contributor

My bad for the PR on splitted repo ...

Related to #2022 and Sylius/SyliusResourceBundle#75

Is it OK ?

@Shine-neko
Copy link
Contributor

👍

@amenophis
Copy link
Contributor Author

@pjedrzejewski @stloyd is it OK for you ?

@stloyd
Copy link
Contributor

stloyd commented Jan 27, 2015

As said before, looks good.

@amenophis
Copy link
Contributor Author

wait for @pjedrzejewski feedback ?

@amenophis
Copy link
Contributor Author

ping @pjedrzejewski

@Shine-neko
Copy link
Contributor

Yah ping @pjedrzejewski

@stloyd
Copy link
Contributor

stloyd commented Feb 3, 2015

@pjedrzejewski ping... again...

@pjedrzejewski
Copy link
Member

Does not sound right to me, 500 when you stop the saving/deletion? It's not always case of an error, no?

@amenophis
Copy link
Contributor Author

@pjedrzejewski ok, how can i manage it in a better way ?
@stloyd @Shine-neko any suggestion ?

@amenophis amenophis force-pushed the api-event branch 3 times, most recently from 65171fa to d8ed93f Compare February 3, 2015 19:06
@amenophis
Copy link
Contributor Author

@pjedrzejewski I made the errorCode configurable by setter and stop method on ResourceEvent object.
Is it better for you ?

@amenophis
Copy link
Contributor Author

@pjedrzejewski Better or not ? Can be merge soon ?

@pjedrzejewski
Copy link
Member

I'm not sure I like the fact that domain manager returns resource event ... It sounds a bit hacky to me. However, I have no idea how to make it better. What is your current usecase? (I see you need it fast J)

@amenophis
Copy link
Contributor Author

I need it to check if customer has enough credit to do the operation during the creation process inside an event and stop event if not enough credit. All the operation are done with REST API process.

Do you understand the usecase ?

@amenophis
Copy link
Contributor Author

@pjedrzejewski ?

@amenophis
Copy link
Contributor Author

@pjedrzejewski what do you think ? we really need a functionallity like this !

@pjedrzejewski
Copy link
Member

OK, I understand, but don't you think it is weird that in case of API call returing ResourceEvent means errors, while in case of normal call, it redirects to index?

@amenophis
Copy link
Contributor Author

In API call, the fact we get a ResourceEvent will allow to throw the exception. In WEB call, if an error occur, the browser will be redirected to the index with a flash message. So in the two cases, we are informed that something went wrong. Maybe in WEB call we need to redirect to current page ?

Do you have a better solution ?

pjedrzejewski pushed a commit that referenced this pull request Feb 9, 2015
Catch event stop during create/update
@pjedrzejewski pjedrzejewski merged commit 78e3ab4 into Sylius:master Feb 9, 2015
@pjedrzejewski
Copy link
Member

Now I get it! :) Thanks Jeremy! This needs to be reworked, but I will include this behavior!

@amenophis amenophis deleted the api-event branch February 9, 2015 20:52
@amenophis
Copy link
Contributor Author

OK thanks, take me in touch if you want some help

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Catch event stop during create/update
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Catch event stop during create/update
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.

None yet

4 participants