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

Initial support for X-EconomicAppIdentifier #42

Merged
merged 2 commits into from
Dec 9, 2015

Conversation

prognostikos
Copy link
Contributor

So here's a somewhat hacky solution.

I couldn't easily see how to test if request headers are set when mocking requests, but I have run this on a production batch job that uses connect_with_credentials and it does work for me.

@@ -8,6 +8,10 @@ class Economic::Endpoint
def_delegator "client.globals", :log_level, :log_level=
def_delegator "client.globals", :log, :log=

def initialize(app_identifier = nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some documentation about what app_identifier is meant to be? Has E-conomic written something somewhere we can copy/paste and/or link to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. My preference would be to add the link to the blog post in the commit message and add a note to the README - sound good?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note in the README sounds good. I'd prefer some rdoc-style documentation directly in the code above the method definition if possible. See for example #call below. This way the docs will appear on for example http://www.rubydoc.info/gems/rconomic/Economic/Endpoint

You can now supply an identifier for your application to both
`Economic::Session.new` and `#connect_with_credentials`.

Economic's blog post about this requirement can be found at
http://techtalk.e-conomic.com/e-conomic-soap-api-now-requires-you-to-specify-a-custom-x-economicappidentifier-header/
@prognostikos
Copy link
Contributor Author

@koppen OK, I think it's pretty well documented now.

@prognostikos
Copy link
Contributor Author

Er, well, Session is. Will add a comment for Endpoint

@koppen
Copy link
Member

koppen commented Dec 9, 2015

That's great, thanks!

koppen added a commit that referenced this pull request Dec 9, 2015
Initial support for X-EconomicAppIdentifier
@koppen koppen merged commit fef7498 into substancelab:master Dec 9, 2015
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.

2 participants