Skip to content

Conversation

@dance2die
Copy link

Hi, I have added a provider for Meetup.com.

Would you review the content and see if it's mergeable?
Please let me know should the merge does not conform to a guideline or a standard.

Test Steps

  1. Add .AddMeetup in Startup.cs
.AddMeetup(options =>
{
    options.ClientId = "CLIENT ID";
    options.ClientSecret = "CLIENT SECRET";
    options.Scope.Add("basic");
});

The list of Scope values can be found here.

  1. Create a Meetup OAuth consumer from here to get a ClientId (shown as Key) and ClientSecret (shown as Secret)
    2.1 and set the Redirect URL to "https://localhost:44318/"
    firefox_2018-02-23_20-42-11

Once you've added the Meetup provider, the test site will display "Connect using Meetup" as shown below
chrome_2018-02-23_20-35-51

  1. Login to Meetup.com

  2. Redirected page shows claims processed
    chrome_2018-02-23_20-38-51

@dance2die
Copy link
Author

@PinpointTownes Hi Kevin,

Would you be able to guide me on what I am doing wrong and how I can fix the issue?
(I've left a comment on the build failure on Gitter, just in case)

Thank you.

@kinosang
Copy link
Contributor

CI failed because it can't download KoreBuild from GitHub. No idea why it happened.

ClaimActions.MapJsonKey("urn:meetup:localized_country_name", "localized_country_name");
ClaimActions.MapJsonKey("urn:meetup:state", "state");

// Map user's photo information returned as a nested object
Copy link
Contributor

Choose a reason for hiding this comment

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

keep the indent of 12 spaces.

Copy link
Author

Choose a reason for hiding this comment

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

I am sorry about that.
My Visual Studio has tabs as a space by default. I will update the indentations.

ClaimActions.MapJsonKey(ClaimTypes.Email, "email");

// Map custom claims
ClaimActions.MapJsonKey("urn:meetup:name", "name");
Copy link
Contributor

Choose a reason for hiding this comment

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

See #216, it introduces ClaimTypes constants for the claims.

Copy link
Author

Choose a reason for hiding this comment

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

OK, let me refer to the #216

@dance2die
Copy link
Author

@kinosang
AppVeyor is still failing while Travis is passing.

According to GitHub issue on AspNet KoreBuild, zip version is not supported any more

@kevinchalet
Copy link
Member

According to GitHub issue on AspNet KoreBuild, zip version is not supported any more

We'll probably move to the new version at some point, but the error you're seeing on AppVeyor doesn't have anything to do with that: it's caused by the fact the GitHub.com folks removed support for TLS 1.0 and 1.1 2 days ago. The AppVeyor team is working on a global fix to force .NET apps to use TLS 1.2 in all their CI images.

In the meantime, let's just ignore the AppVeyor errors.

@dance2die
Copy link
Author

Thank you for looking into the AppVeyor issue @PinpointTownes.

@dance2die
Copy link
Author

I've rebased according to an advice in #224.
Would you review to see if the rebase is done properly? (My first time doing a rebase).
Thank you.

Copy link
Member

@martincostello martincostello left a comment

Choose a reason for hiding this comment

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

This needs re-syncing with the dev branch so as to not include the changes to the build scripts.

public const string PhotoBaseUrl = "urn:meetup:photo.base_url";

public const string PhotoType = "urn:meetup:photo.type";

Copy link
Member

Choose a reason for hiding this comment

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

Nit: remove blank line here.

@martincostello
Copy link
Member

Thanks for the PR @dance2die - as part of the preparation for ASP.NET Core 3.0 support, tests are being added to help make things easier to maintain and validate going forwards (see #292).

Once tests are merged into the dev branch, could you copy the approach to add tests for the new provider into this PR please?

@martincostello
Copy link
Member

#280 has been merged to dev now. If you rebase and update any tests as appropriate, we can look at merging this PR soon.

@martincostello martincostello removed this from the 2.1.0 milestone May 24, 2019
@martincostello
Copy link
Member

Closing due to inactivity and merge conflicts - please re-open and rebase if you would like to pursue adding a provider for Meetup.com in a future release.

@dance2die
Copy link
Author

@martincostello
Sorry about the late follow up.

I haven't been using .NET core for awhile and I will leave it to others to do a PR should this is need Meetup provider.

@dance2die dance2die deleted the meetup_test_branch branch May 28, 2019 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants