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 Twitter and Facebook login #97

Merged
merged 16 commits into from
Mar 21, 2021
Merged

Conversation

abs8090
Copy link
Contributor

@abs8090 abs8090 commented Mar 19, 2021

This PR adds Twitter and Facebook login via the ParseAuthenticatable protocol.
The user of ParseTwitter or ParseFacebook still needs to implement TwitterKit or FBSDKCoreKit to obtain authentication keys and pass them to ParseTwitter.logn() or ParseFacebook.login() to authenticate with ParseServer.

@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #97 (c619c27) into main (403a02b) will increase coverage by 0.90%.
The diff coverage is 100.00%.

❗ Current head c619c27 differs from pull request most recent head 55d6b7d. Consider uploading reports for the commit 55d6b7d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
+ Coverage   79.76%   80.66%   +0.90%     
==========================================
  Files          63       65       +2     
  Lines        5170     5411     +241     
==========================================
+ Hits         4124     4365     +241     
  Misses       1046     1046              
Impacted Files Coverage Δ
...Swift/Authentication/3rd Party/ParseFacebook.swift 100.00% <100.00%> (ø)
...arseSwift/Authentication/3rd Party/ParseLDAP.swift 100.00% <100.00%> (ø)
...eSwift/Authentication/3rd Party/ParseTwitter.swift 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 403a02b...55d6b7d. Read the comment docs.

@abs8090 abs8090 closed this Mar 19, 2021
@abs8090 abs8090 reopened this Mar 19, 2021
@cbaker6
Copy link
Contributor

cbaker6 commented Mar 19, 2021

Thanks @abs8090 for your contribution! I will look over soon.

In the mean time can you add an entry to the CHANGELOG.md file, something like (this should go right above ### 1.2.1):

__New features__
- Add ParseTwitter and ParseFacebook authentication ([#97](https://github.com/parse-community/Parse-Swift/pull/97)), thanks to [YOUR NAME](https://github.com/abs8090).

@cbaker6 cbaker6 marked this pull request as draft March 19, 2021 13:51
@cbaker6
Copy link
Contributor

cbaker6 commented Mar 20, 2021

I updated your branch with the latest main, you should should pull your branch and then proceed with the rest of the changes

@cbaker6
Copy link
Contributor

cbaker6 commented Mar 20, 2021

The PR is looking good! Be sure to "pull" your branch again before making anymore updates.

You had some inconsistencies in ParseTwitter with, twitterId, userId, and user. I switched them all to userId as it looks like that's what Twitter uses.

Things you need to do:

  • Install SwiftLint from the contributors guide and fix the warnings.
  • Verify you have all of the necessary keys for Facebook and Twitter and make sure your testcases are checking what they need to check

@cbaker6 cbaker6 marked this pull request as ready for review March 21, 2021 16:53
@cbaker6
Copy link
Contributor

cbaker6 commented Mar 21, 2021

This looks ready to go!

Thanks for making your first contribution!

@cbaker6 cbaker6 merged commit fa74a7c into parse-community:main Mar 21, 2021
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.

None yet

2 participants