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

RF element added many times #68

Closed
bimusiek opened this issue Apr 26, 2017 · 11 comments
Closed

RF element added many times #68

bimusiek opened this issue Apr 26, 2017 · 11 comments

Comments

@bimusiek
Copy link
Collaborator

Hey, I see from the code that RF element is being added automatically.

$requestOptions->receivedFrom = $this->params->receivedFrom;

However, if you use AddMultiElements many times, then it should only be added once, thus I would suggest to allow users to specify when to send it and not add it automatically as you do it only on PNR creation.

I got warning from Amadeus when we do new certification that it should only be sent once.

What do you think? I can prepare PR for it.

@DerMika
Copy link
Collaborator

DerMika commented Apr 26, 2017

Well, the first problem I see now is backwards compatibility. Removing this now will cause things to break - because I know some people depend on it.

@bimusiek
Copy link
Collaborator Author

Then maybe

"receivedFrom" => IgnoreElement()

?

This way we could check while creating request to not include received from.

Or there could be another param ignoreReceivedFrom, no idea what is the best approach here.

@DerMika
Copy link
Collaborator

DerMika commented Apr 26, 2017

Did this issue cause you problems with your certification? Does Amadeus ask that you change this?

@DerMika
Copy link
Collaborator

DerMika commented Apr 26, 2017

I'll have to think about the best way to solve this.

@bimusiek
Copy link
Collaborator Author

Yes, we are doing ATC and LCC certification and while checking traces from LCC booking I got this info:

·         You are adding an RF element in both PNR_AddMultiElements queries. This only has to be added once for each time you are saving the PNR. This means that once it is added in sequence number 2, it will remain in the PNR until you send a status code 10 or 11. You are also adding an RF element when you are saving the PNR. Please remove at least 2 instances of this. Ideally this should be added along with option code 11 under sequence number 6.

@DerMika
Copy link
Collaborator

DerMika commented Apr 26, 2017

Ok, so maybe we could change the automagic of it to only add an RF when the actioncode is any kind of save action?

I don't think that would result in a BC break.

@DerMika
Copy link
Collaborator

DerMika commented Apr 26, 2017

But... The problem I see with that solution is that you don't know if an RF was added manually in a previous PNR_AddMultiElements while the PNR was still in context.

@bimusiek
Copy link
Collaborator Author

Plus, I can imagine using PNR_AddMultiElements while changing existing PNR, which would add RF element again. I understand the point of making API as easy to use for new users but it won't fly in certification 🏅

@DerMika
Copy link
Collaborator

DerMika commented Apr 26, 2017

So what I'm thinking of now - for backwards compatibility - is a second parameter when calling the message:

$client->pnrAddMultiElements($admultiopt, ['noAutoRf' => true]);

And then we can decide for version 2.0 of we want to continue the automatic adding of RF elements or if the user of the library is completely responsible for it.

@DerMika
Copy link
Collaborator

DerMika commented Apr 27, 2017

Ok, I've implemented a fix to override the automatic addition of an RF element when doing a PNR_AddMultiElements. I made it a property in the request options after all, because it made more sense.

Sample here: https://github.com/amabnl/amadeus-ws-client/blob/master/docs/samples/pnr-create-modify.rst#disable-automatically-adding-a-received-from-rf-element

@DerMika DerMika closed this as completed Apr 27, 2017
@bimusiek
Copy link
Collaborator Author

Thanks a lot! 🎉

@DerMika DerMika added this to the 1.4.0 milestone Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants