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

Add RSVP library support for Meetup.com #179

Merged
merged 1 commit into from Dec 30, 2019

Conversation

@jamietanna
Copy link
Contributor

jamietanna commented Dec 29, 2019

As part of snarfed/bridgy/issues/873 we want to add Meetup.com support
to Brid.gy.

Before we can do this, we need to onboard it to granary.

For now, all were adding is RSVP support, so we can't add that in the
Granary webapp, just the library.

Unfortunately we don't get a permalink / ID back from the RSVP endpoint,
so similar to the way that Granary's Facebook likes work, we create our
own "permalink".

granary/meetup.py Outdated Show resolved Hide resolved
'urlname': urlname,
'event_id': event_id,
}
params = 'response=%(response)s' % {

This comment has been minimized.

Copy link
@jamietanna

jamietanna Dec 29, 2019

Author Contributor

is this overkill? thinking ahead to when we may want to change it in the future, but likely not helpful

This comment has been minimized.

Copy link
@snarfed

snarfed Dec 30, 2019

Owner

seems fine to me. you'd actually use obj['content'] instead of the method param, right? go for it!

This comment has been minimized.

Copy link
@jamietanna

jamietanna Dec 30, 2019

Author Contributor

Sorry what do you mean?

Copy link
Owner

snarfed left a comment

nice! this looks great. i've added comments but nothing major

granary/meetup.py Outdated Show resolved Hide resolved
'urlname': urlname,
'event_id': event_id,
})
return util.urlopen(urllib.request.Request(

This comment has been minimized.

Copy link
@snarfed

snarfed Dec 30, 2019

Owner

reuse oauth_dropins.meetup.urlopen_bearer_token()?

This comment has been minimized.

Copy link
@jamietanna

jamietanna Dec 30, 2019

Author Contributor

Looks like that code may only work for a GET, not a POST, though? 🤔 https://github.com/snarfed/oauth-dropins/blob/master/oauth_dropins/meetup.py#L42

This comment has been minimized.

Copy link
@snarfed

snarfed Dec 30, 2019

Owner

it will pass through data=..., which i think automatically makes urlopen() POST, right? if not, feel free to parameterize the method.

This comment has been minimized.

Copy link
@jamietanna

jamietanna Dec 30, 2019

Author Contributor

When updating to use it, it now gives me some errors around setting up the mocks, would you be able to help? Python isn't my strong point!

======================================================================
FAIL: test_create_rsvp_yes_with_www (granary.tests.test_meetup.MeetupTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/app/local3/lib/python3.7/site-packages/mox3/mox.py", line 2139, in new_method
    func(self, *args, **kwargs)
  File "/app/granary/tests/test_meetup.py", line 66, in test_create_rsvp_yes_with_www
    created = self.meetup.create(rsvp)
  File "/app/granary/meetup.py", line 33, in create
    return self._create(obj, False, include_link, ignore_formatting)
  File "/app/granary/meetup.py", line 92, in _create
    resp = self.post_rsvp(urlname, event_id, response)
  File "/app/granary/meetup.py", line 51, in post_rsvp
    return meetup.urlopen_bearer_token(url, self.access_token, data=params)
  File "/app/oauth_dropins/meetup.py", line 38, in urlopen_bearer_token
    return util.urlopen(urllib.request.Request(url, headers=headers), **kwargs)
  File "/app/oauth_dropins/webutil/util.py", line 1415, in urlopen
    return urllib.request.urlopen(url_or_req, *args, **kwargs)
  File "/app/local3/lib/python3.7/site-packages/mox3/mox.py", line 814, in __call__
    return mock_method(*params, **named_params)
  File "/app/local3/lib/python3.7/site-packages/mox3/mox.py", line 1113, in __call__
    expected_method = self._VerifyMethodCall()
  File "/app/local3/lib/python3.7/site-packages/mox3/mox.py", line 1180, in _VerifyMethodCall
    raise UnexpectedMethodCallError(self, expected)
mox3.mox.UnexpectedMethodCallError: Unexpected method call.  unexpected:-  expected:+
- function.__call__(<urllib.request.Request object at 0x7f3b306b4a90>, data=b'response=yes', timeout=15) -> None
+ function.__call__(<function TestCase.expect_urlopen.<locals>.check_request at 0x7f3b306b5b00>, timeout=15) -> <oauth_dropins.webutil.testutil.UrlopenResult object at 0x7f3b306b4ad0>

This comment has been minimized.

Copy link
@snarfed

snarfed Dec 30, 2019

Owner

oh looks like you maybe need to pass data=... to the Request constructor, not to urlopen()?

if that's not it, you may need to check for bytes value instead of str in the check fn, or pass str instead of bytes.

This comment has been minimized.

Copy link
@jamietanna

jamietanna Dec 30, 2019

Author Contributor

OK I've raised snarfed/oauth-dropins#26 as a dependency to this - once that's in we'll hopefully be able to use that call to urlopen and that'll work 👍

granary/meetup.py Show resolved Hide resolved
granary/meetup.py Outdated Show resolved Hide resolved
granary/meetup.py Outdated Show resolved Hide resolved
@snarfed

This comment has been minimized.

Copy link
Owner

snarfed commented Dec 30, 2019

thanks for the thorough tests btw!

@jamietanna

This comment has been minimized.

Copy link
Contributor Author

jamietanna commented Dec 30, 2019

Thanks @snarfed I've updated with your comments, plus replied to the ones I've not yet resolved!

@jamietanna jamietanna force-pushed the jamietanna:feature/meetup branch from e0e359f to c0a894a Dec 30, 2019
jamietanna added a commit to jamietanna/oauth-dropins that referenced this pull request Dec 30, 2019
As found with snarfed/granary#179, we need a convenient way to POST to
Meetup.com using the Bearer token.

The easiest way to do this is to add a new method which allows for
passing through the request body `data`.
@snarfed

This comment has been minimized.

Copy link
Owner

snarfed commented Dec 30, 2019

looks great otherwise!

jamietanna added a commit to jamietanna/oauth-dropins that referenced this pull request Dec 30, 2019
As found with snarfed/granary#179, we need a convenient way to POST to
Meetup.com using the Bearer token.

The easiest way to do this is to amend our existing method and allow
passing through the request body `data`.
jamietanna added a commit to jamietanna/oauth-dropins that referenced this pull request Dec 30, 2019
As found with snarfed/granary#179, we need a convenient way to POST to
Meetup.com using the Bearer token.

The easiest way to do this is to amend our existing method and allow
passing through the request body `data`.
jamietanna added a commit to jamietanna/oauth-dropins that referenced this pull request Dec 30, 2019
As found with snarfed/granary#179, we need a convenient way to POST to
Meetup.com using the Bearer token.

The easiest way to do this is to amend our existing method and allow
passing through the request body `data`.
snarfed added a commit to snarfed/oauth-dropins that referenced this pull request Dec 30, 2019
As found with snarfed/granary#179, we need a convenient way to POST to
Meetup.com using the Bearer token.

The easiest way to do this is to amend our existing method and allow
passing through the request body `data`.
@jamietanna

This comment has been minimized.

Copy link
Contributor Author

jamietanna commented Dec 30, 2019

Looks like we're good - just cleaning up history then will mark as ready for review

@jamietanna jamietanna force-pushed the jamietanna:feature/meetup branch from 5ce039a to e5e8040 Dec 30, 2019
@jamietanna jamietanna marked this pull request as ready for review Dec 30, 2019
@jamietanna jamietanna changed the title WIP: Add Meetup.com as a silo Add RSVP library support for Meetup.com Dec 30, 2019
@jamietanna

This comment has been minimized.

Copy link
Contributor Author

jamietanna commented Dec 30, 2019

@snarfed ready for your review!

As part of snarfed/bridgy/issues/873 we want to add Meetup.com support
to Brid.gy.

Before we can do this, we need to onboard it to granary.

For now, all were adding is RSVP support, so we can't add that in the
Granary webapp, just the library.

Unfortunately we don't get a permalink / ID back from the RSVP endpoint,
so similar to the way that Granary's Facebook likes work, we create our
own "permalink".
@jamietanna jamietanna force-pushed the jamietanna:feature/meetup branch from e5e8040 to f2392af Dec 30, 2019
@jamietanna jamietanna mentioned this pull request Dec 30, 2019
@snarfed snarfed merged commit be802e4 into snarfed:master Dec 30, 2019
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
Details
@snarfed

This comment has been minimized.

Copy link
Owner

snarfed commented Dec 30, 2019

🎉

@jamietanna jamietanna deleted the jamietanna:feature/meetup branch Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.