Added fee ids to MasterPricer #36
Conversation
|
Well, this is wonderful, thanks for contributing! :) I'll go through the code |
There was a problem hiding this comment.
Please see the remarks I made inline.
On top of that, I'd also like to have a sample in the docs (https://github.com/amabnl/amadeus-ws-client/blob/master/docs/samples/masterpricertravelboard.rst).
| /** | ||
| * Set fee ids if needed | ||
| * | ||
| * @param string|null $currency |
There was a problem hiding this comment.
Looks like this PHPDoc needs some love.
| class MPFeeId extends LoadParamsFromArray | ||
| { | ||
| /** | ||
| * Fee Type |
There was a problem hiding this comment.
I'd like to have all possible Fee Types as constants "TYPE_*" here so users know what values are possible (and supported by Amadeus).
This is analogous what I do in other places, for example here
Here's the list of possible fee types:
Fare_MasterPricerTravelBoardSearch/fareOptions/feeIdDescription/feeId/feeType
Attribute type identification, coded codesets
Value Description
ARP Arp number
FBA Free Baggage Allowance
FFI Fare Family Information
NPS No PNR Split
PTRAM Price To Reach amount type
RTC Half round trip combination
SBF Search by FBA
SORT Sorting option (FEE, NOFEE)
TOKEN Token
UPB Upsell per bound
UPH Homogenous upsell
As seen here: https://webservices.amadeus.com/extranet/structures/viewMessageStructure.do?id=5769&serviceVersionId=4507&isQuery=true
| @@ -62,8 +62,9 @@ class FareOptions | |||
| * @param array $corpCodesUniFares list of Corporate codes for Corporate Unifares | |||
| * @param bool $tickPreCheck Do Ticketability pre-check? | |||
| * @param string|null $currency Override Currency conversion | |||
| * @param array|null $flightOptions List of FeeIds | |||
There was a problem hiding this comment.
I'd prefer to have the typed objects here, e.g. FeeId[]
| @@ -33,4 +33,20 @@ class FeeId | |||
| public $feeType; | |||
There was a problem hiding this comment.
$feeType and $feeIdNumber need phpdocs.
Also here I'd like to have the same list of constants of possible feeTypes.
|
@DerMika Hey, I believe I did all changes you requested :) Tell me please if I missed something. |
There was a problem hiding this comment.
Looks good! There's only one small change I'd like: that you used the FeeId::TYPE_* constants in your sample in the docs.
That gives whoever is reading the docs the information that there are constants available to choose from. Otherwise they'd have to dig in the code to find that out.
|
@DerMika Updated. |
|
Wonderful, Thanks again for your help! I've merged your PR. |
…AN-755-switch-to-json-log-format-of-nginx to master * commit '150a8e6c2eb85062546720896176bde13e79335e': (change) field type of some properties access log (change) log format of nginx access log to json
Hey, I felt bad about just asking so I wanted to contribute a little bit.
Here is option to add FeeIds when searching in MasterPricer.
Possible options I saw in Amadeus.
Tell me if you want me to change anything!
Cheers, Mike.