search

mothership-ec / cog-mothership-returns

Mothership Returns Cogule
Other
0 stars 1 forks source link

Fix for issue where document IDs were not being saved against the return #165

Closed thomasjthomasj closed 10 years ago

thomasjthomasj commented 10 years ago

What does this do?

Fixes issue where the document ID was not being saved against the return, and therefore causing a fatal error when the user would click to print off their return slip: https://github.com/messagedigital/rsar/issues/31

This is not the cleanest fix but the code is so rancid at the moment that it's in need of a lot of cleaning up. Writing perfect code here would be like forcing people to use coasters in a barn, but I've raised an issue for it: https://github.com/messagedigital/cog-mothership-returns/issues/164

How should this be manually tested?

Create a return and check that you can view the document. Also create a standalone return in EPOS and ensure that it doesn't error

Related PRs / Issues / Resources?

Anything else to add? (Screenshots, background context, etc)

irisSchaffer commented 10 years ago

On EPOS standalone returns:

image

irisSchaffer commented 10 years ago

RE

Yes, I know the create decorator uses a transaction but it will have already been committed by this point. Don't judge me please! It's all Laurence's fault! He wrote all this nasty code and then went packing :(

Why did you not make it a transactional event and listen for CREATE_END instead if you're worried about the transaction? I am more worried about putting plain SQL into the Event Listener! Also this event listener is only used for building the navigation, which is why it seems a bit weird to have it in the same file. But we can raise an issue for this and at some point change it to be two Event Listeners (one EventListener and one MenuEventListener / MenuListener).