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

Fixes #687 - Add ability to upload images in comments for an issue #703

Merged
merged 11 commits into from
Sep 21, 2015

Conversation

miketaylr
Copy link
Member

r? @karlcow for Python bits
r? @magsout for CSS + JS

@miketaylr
Copy link
Member Author

(I still need to fix the flaky tests, the failures are unrelated).

@@ -21,6 +21,7 @@
from webcompat import cache
from webcompat import github
from webcompat import limiter
from webcompat.helpers import get_comment_data
from webcompat.helpers import get_headers
from webcompat.helpers import get_request_headers
from webcompat.helpers import get_user_info

This comment was marked as abuse.

@karlcow
Copy link
Member

karlcow commented Sep 12, 2015

@miketaylr r+ for the python part. Tested. Code is working. I have a curiosity question up there, but that's just for curiosity.

@magsout
Copy link
Member

magsout commented Sep 15, 2015

hum need help I think @miketaylr

When I try to log in (localhost)

Something went wrong trying to sign into GitHub. :(

@karlcow
Copy link
Member

karlcow commented Sep 15, 2015

@magsout try to erase session.db first. Then restart

@magsout
Copy link
Member

magsout commented Sep 15, 2015

@karlcow same error :(

@miketaylr
Copy link
Member Author

Bummer @magsout. Two debug requests:

  1. Can you paste the HTTP logs (from the terminal) starting just before you click "Log in"? Linking a gist will be fine.
  2. Can you open the network panel of devtools and see if there are any 500 errors or something?

@magsout
Copy link
Member

magsout commented Sep 16, 2015

@miketaylr

capture d ecran 2015-09-16 a 07 23 17

@magsout
Copy link
Member

magsout commented Sep 16, 2015

@karlcow
Copy link
Member

karlcow commented Sep 16, 2015

@magsout you seem to have logged in ?
Did you log out of github too? Not only webcompat.

@magsout
Copy link
Member

magsout commented Sep 16, 2015

you seem to have logged in ?

@karlcow yes but no. I'm not logged

Did you log out of github too? Not only webcompat.

I'm logged on Github. I tried to logged out and logged in...

@miketaylr
Copy link
Member Author

Man, webapps are hard. @magsout, I don't recall if I shared the id and secret for the local testing app (that's what will correspond to the part from config.py below) with you or not. I just reset all user tokens there, but if you made your own can you try to "Revoke all users tokens"?

# We're running on localhost, use the test application
    GITHUB_CLIENT_ID = os.environ.get('FAKE_ID') or "<secrets!>"
    GITHUB_CLIENT_SECRET = os.environ.get('FAKE_SECRET') or "<secrets!>"
    GITHUB_CALLBACK_URL = "http://localhost:5000/callback"

@miketaylr
Copy link
Member Author

And after doing that, rm Session.db and then try to restart the app and login.

@magsout
Copy link
Member

magsout commented Sep 16, 2015

I don't recall if I shared the id and secret for the local testing app

Heu no, I sent to you my config by email.

just reset all user tokens there, but if you made your own can you try to "Revoke all users tokens"?

Done, and still same error :(

@@ -95,6 +94,126 @@ issues.TextAreaView = Backbone.View.extend({
}, 250, {maxWait: 1500})
});

issues.ImageUploadView = Backbone.View.extend({
tagName: 'div',
className: 'wc-Form-group',

This comment was marked as abuse.

@magsout
Copy link
Member

magsout commented Sep 17, 2015

@miketaylr

Except my comment LGTM r+

@miketaylr
Copy link
Member Author

Thanks for the review @magsout! Will make those changes (and spend some time looking at the issue that @karlcow raised).

@miketaylr
Copy link
Member Author

But I was wondering why the double dance dict -> json -> dict

You're awesome @karlcow. 🎈

That's just a mistake on my part, I forgot that raw_request returns a Requests Response object, not a Flask one. So I was just doing extra work for no good reason. Will fix.

miketaylr pushed a commit that referenced this pull request Sep 21, 2015
Fixes #687 - Add ability to upload images in comments for an issue
@miketaylr miketaylr merged commit b452a38 into master Sep 21, 2015
@miketaylr miketaylr deleted the issues/687/1 branch September 21, 2015 22:49
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.

3 participants