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

#4344 Breaks Facebook posting. #4361

Closed
liamn opened this issue Aug 7, 2013 · 14 comments · Fixed by #4383
Closed

#4344 Breaks Facebook posting. #4361

liamn opened this issue Aug 7, 2013 · 14 comments · Fixed by #4383

Comments

@liamn
Copy link
Contributor

liamn commented Aug 7, 2013

The Pull Request #4344 breaks Facebook posting, the users facebook cannot be added to the users account.

Trace is:

NoMethodError (undefined method `[]' for nil:NilClass):
  app/controllers/services_controller.rb:91:in `header_hash'
  app/controllers/services_controller.rb:60:in `abort_if_read_only_access'
  lib/rack/chrome_frame.rb:39:in `call'
  lib/unicorn_killer.rb:35:in `call' 
@oliverbarnes
Copy link
Contributor

I'll look into it. If possible, would you add a breaking spec replicating it please?

@oliverbarnes
Copy link
Contributor

@liamn please let me know if you still get this issue

@jhass jhass closed this as completed in 452301b Aug 8, 2013
jhass added a commit that referenced this issue Aug 8, 2013
Fix #4361 twitter access level check breaking facebook addition to user
@liamn
Copy link
Contributor Author

liamn commented Aug 8, 2013

Facebook's now working, but Twitter is broken now. - I get to the screen where I can authorise the application to use my twitter account, and after authorising it I get a 500.

@oliverbarnes
Copy link
Contributor

Sweet hahah... on it

On Thu, Aug 8, 2013 at 5:00 AM, Liam Nicholson [email protected]:

Facebook's now working, but Twitter is broken now. - I get to the screen
where I can authorise the application to use my twitter account, and after
authorising it I get a 500.


Reply to this email directly or view it on GitHubhttps://github.com//issues/4361#issuecomment-22313688
.

@oliverbarnes
Copy link
Contributor

@liamn can you send me the logs/stack trace please? I'm setting things up to manually test this (I know, shame on me for not having done it before), but while I do it the log might help

@oliverbarnes
Copy link
Contributor

@MrZyx is it possible to test this locally? I just tried creating a twitter app on dev.twitter, but it doesn't like localhost, unlike Facebook apps.

@jhass
Copy link
Member

jhass commented Aug 8, 2013

I don't think so :/ Well you could setup DynDNS and all that.

@oliverbarnes
Copy link
Contributor

ok... heroku time :)

On Thu, Aug 8, 2013 at 2:27 PM, Jonne Haß [email protected] wrote:

I don't think so :/


Reply to this email directly or view it on GitHubhttps://github.com//issues/4361#issuecomment-22348932
.

@liamn
Copy link
Contributor Author

liamn commented Aug 9, 2013

Trace:

Completed 500 Internal Server Error in 10ms

NoMethodError (undefined method `[]' for #<OAuth::AccessToken:0x000000076ad040>):
  app/controllers/services_controller.rb:100:in `twitter_header_present?'
  app/controllers/services_controller.rb:94:in `twitter_header'
  app/controllers/services_controller.rb:59:in `abort_if_read_only_access'
  lib/rack/chrome_frame.rb:39:in `call'
  lib/unicorn_killer.rb:35:in `call' 

@oliverbarnes
Copy link
Contributor

@liamn would you please test this fix? I deployed it and got the correct message when trying to add twitter without write access. When testing facebook I'm getting a oauth error ("missing client id"), but it might be because the facebook app I created isn't yet properly working, so it'd be good to check with yours.

@MrZyx not ready to merge yet, I'm just waiting on manual tests being fully done to fix breaking specs

@oliverbarnes
Copy link
Contributor

also just tested with read-write access, and it's working

@oliverbarnes
Copy link
Contributor

just fixed the tests. what's left is testing facebook manually. I still have the missing client id issue, so @liamn if you can confirm it works on your end, then it's something with my facebook app, and we can close this issue

@liamn
Copy link
Contributor Author

liamn commented Aug 11, 2013

I've already confirmed on your pull request, but yeah, everything is working here so it must be your Facebook app.

@oliverbarnes
Copy link
Contributor

yeah, I had SERVICES_FACEBOOK_KEY instead of SERVICES_FACEBOOK_APP_ID in my env

oliverbarnes pushed a commit to oliverbarnes/diaspora that referenced this issue Aug 12, 2013
…on to user

Rewrite twitter access-level check

Fixed tests, still working on getting facebook up

Add heroku example to diaspora.example.yml
jhass added a commit that referenced this issue Aug 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants