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

Class methods should return objects with expected classes #103

Open
ronaldtse opened this issue Mar 23, 2017 · 4 comments
Open

Class methods should return objects with expected classes #103

ronaldtse opened this issue Mar 23, 2017 · 4 comments

Comments

@ronaldtse
Copy link
Collaborator

Hi @abunashir ,

When I was testing out the code I noticed that some responses do not return properly 'classed' objects.

For example,

Digicert::Product.all.first.class
=> Digicert::ResponseObject

But the expectation of someone running this should be Digicert::Product.

Could you help go through the code and see which calls are returning the generic ResponseObject and give them the proper classes?

Thanks!

@abunashir
Copy link
Member

Hi @ronaldtse, we are parsing any response to ResponseObject, which is basically parsing the json response to deeply nested OpenStruct object( click here to see the source). Like we spoke before, because of the inconsistent nature of the Digicert response and to make it more flexible we used it.

We can change it to return an Actual class but then we have to specify every attribute in each of our classes, we can try to do that now but I feel like that might be lot of headache to mange whenever there is a change in Digicert response. Personally, I feel that the path we will take once this client is little bit stable. Please let me know your thoughts

@ronaldtse
Copy link
Collaborator Author

@abunashir that's fine. We should probably implement Digicert::Order.all to allow calls like:

certificate = Digicert::Order.all.first.certificate
duplicated_certificate = Digicert::Order.all.first.duplicates.certificate

@ronaldtse
Copy link
Collaborator Author

@abunashir the client seems stable enough to move forward with this. What do you think?

@abunashir
Copy link
Member

@ronaldtse: Yap, that's on the top of the list right now. We are in a pretty much stable situation and I am hoping to move away from OpenStruct response type and include this one in the v1.0.

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

No branches or pull requests

2 participants