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

Profile endpoints extracted from PR #336 #383

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

m0veax
Copy link
Contributor

@m0veax m0veax commented Feb 2, 2020

Hi There,

i found PR #336 and started extracting the Profile Endpoints.

I removed duplicate Code, refactored Typehints and Typos. My next step is to write the needed tests and add it to the docs afterwards.

Hope anyone can use it :)

@coveralls
Copy link

coveralls commented Feb 2, 2020

Coverage Status

Coverage decreased (-0.8%) to 99.059% when pulling cc88b57 on proappers:profileEndpoints into b072260 on amabnl:master.

@bimusiek
Copy link
Collaborator

Hey @m0veax, thanks for contributing.
Do you think is it possible to add tests for coverage and fix scrutinizer issues?

I would love to have your code merged. 👍

@m0veax
Copy link
Contributor Author

m0veax commented Feb 12, 2020

Hi,

yeah i have to, otherwise it wouldn't get merged and I loose my update ability in my own usage.

But I decided to focus on Readprofile and search by name and will rearrange the pull request that way.

Getting the class structure converted to the needed xml is new for me and some kind of PITA right now. Maybe the docs of this lib should be extended about this topic?

@m0veax
Copy link
Contributor Author

m0veax commented Feb 12, 2020

Can someone point me to the place where the class Structures get converted to XML?

I have some converting Issues and can't really find a manual or some other informations.

@DerMika
Copy link
Collaborator

DerMika commented Feb 12, 2020

Unfortunately that's the magic of PHP's SoapClient library, which uses the C library libxml2 underneath..

@m0veax
Copy link
Contributor Author

m0veax commented Feb 12, 2020

Ah, so it is a native PHP feature and not something developed in your library. I tried to find some, but do you know any documentation about how that happens?

@DerMika
Copy link
Collaborator

DerMika commented Feb 12, 2020

I know you can provide a classpath map to explicitly try to map objects from the WSDL to PHP classes, but there's a lot of trial and error in the whole process I'm afraid... My resources were mainly Google & Stackoverflow :)

@m0veax
Copy link
Contributor Author

m0veax commented Feb 12, 2020

Thanks for your blazing fast replies. I will dive into it. Hopefully I get this done until the end of next month. I don't have much spare time at the moment, but I want to learn more about tests and your PR rules are fitting that case for me :)

I will commit again when I build a valid soap message to find profiles by name.

Please don't add a milestone yet

@m0veax
Copy link
Contributor Author

m0veax commented Feb 24, 2020

I feel like i am near to get reading a profile by freetext name search done.

But how should i interpret this Error in the PHP SOAP context?

Object of class Amadeus\Client\RequestOptions\Profile\FreeTextSearchOptions could not be converted to string

I'm sure that i should not add a toString magic method.

@DerMika
Copy link
Collaborator

DerMika commented May 28, 2020

FreeTextSearchOptions is a requestOptions object, which is something that should be used to BUILD the message (messages should be namespace Amadeus\Client\Struct*)

so you probably did something wrong converting the request options to the structs which are used to build the XML document.

@m0veax
Copy link
Contributor Author

m0veax commented May 28, 2020

Hey,

thanks for your input. I found a way to send a valid message and get a valid response. I am soon working on my next commit to this PR.

So, this is still alive and i'm still working on it.

@DerMika
Copy link
Collaborator

DerMika commented May 28, 2020

allright, thanks!

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