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

Added support for oEmbed. Needs testing. #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

throoze
Copy link
Contributor

@throoze throoze commented Nov 18, 2015

Hi! i really don't know how to test this code, but it should work. If you could please test it and let me know if it works, it would be nice! If it doesn't, i think it should require just small corrections.

A better option could be for you to tell me how to test it. I would really appreciate it, and I would let you know when its working.

Thanks!

@mileszim
Copy link
Owner

Awesome work! One quick way to test it would be to just open a ruby console, require the documentcloud main lib file, and then try creating a client and then running through the oembed methods you introduced. If it throws an unexpected error when you run a command, then you have something to debug!

I do want to add an actual test suite to the thing, hopefully in the next few days.

I'll leave this PR open until you respond saying you tried it out and think it's good to go!

Thanks again @throoze, you rock

@throoze
Copy link
Contributor Author

throoze commented Nov 18, 2015

I'm having problems to authenticate. I did:

DocumentCloud.configure do |config|
  # with my own credentials, obviously
  config.email    = '[email protected]'
  config.password = 'my_secret_password'
end

in irb console, but i'm always getting RestClient::Forbidden: 403 Forbidden. Whats the proper way to configure credentials in irb console? I'm sorry. Although I have some time working with rails, i'm not very good at plain ruby, and don't really understand how it works. Thanks for your help.

@mileszim
Copy link
Owner

Verify the email/password is correct. Then, you can use it more easily in the command line by use invoking your own DocumentCloud::Client instance in IRB:
client = DocumentCloud::Client.new(email: "[email protected]", password: "password123")

Then all the commands you would normally use DocumentCloud.whatever() to call, you can just do client.whatever(some_params: "here")

See if that works

@throoze
Copy link
Contributor Author

throoze commented Nov 21, 2015

Thanks! already on that. An extra step is needed, since email and password are not escaped by default.

client = DocumentCloud::Client.new(email: "[email protected]", password: "password123")

client.configure do |config|
        config.email    = DOCUMENTCLOUD_EMAIL
        config.password = DOCUMENTCLOUD_PASSWORD
end

# and then you can do something like client.whatever(some_params: "here")

I'll change that :) but I noticed this code on client initialization:

instance_variable_set(:"@#{key}", options[key] || DocumentCloud.instance_variable_get(:"@#{key}"))

Should every key be escaped? or only email and password? So far, I've only noticed these two keys being used in DocumentCloud::Client, but its just to be sure.

Also, since it may break rails expected behavior, ill set an @escaped flag in DocumentCloud::Client.initialize, and make DocumentCloud::Configurable.configure check for it, in case keys are already set.

Please let me know any thoughts on this. I'll let you know when changes are pushed.

Kind Regards,

@width = attrs[:width]
@html = attrs[:html]
end

Copy link
Owner

Choose a reason for hiding this comment

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

remove the empty spaces before the "end" statements and I'm good with merging this :)

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