parse-community / Parse-Swift

The Swift SDK for Parse Platform (iOS, macOS, watchOS, tvOS, Linux, Android, Windows)
https://parseplatform.org
MIT License
306 stars 69 forks source link

fix: sessionToken nil if preventLoginWithUnverifiedEmail=1 on server #331

Closed Climbatize closed 2 years ago

Climbatize commented 2 years ago

New Pull Request Checklist

Issue Description

When Parse server is configured with preventLoginWithUnverifiedEmail session token after signup is nil and causes a parsing crash, caught and returned as unknown error.

Related issue: #313

Approach

What I do there is setting the sessionToken variable in signup response to optional to prevent parsing crash. Then, if the token is nil, I throw a 209 so developers on SDK can identify easily the ParseError and provide the good message to their users.

TODOs before merging

parse-github-assistant[bot] commented 2 years ago

Thanks for opening this pull request!

vdkdamian commented 2 years ago

Can I first try this with my project before merging? Just to make sure this isn't breaking anything. I'll try to do this within the next few days, after @cbaker6 approves these changes.

vdkdamian commented 2 years ago

Can you also put this on a different branch?

codecov[bot] commented 2 years ago

Codecov Report

Merging #331 (0a1ff59) into main (96232f3) will decrease coverage by 0.38%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #331      +/-   ##
==========================================
- Coverage   85.11%   84.72%   -0.39%     
==========================================
  Files         114      114              
  Lines       12116    12078      -38     
==========================================
- Hits        10312    10233      -79     
- Misses       1804     1845      +41     
Impacted Files Coverage Δ
Sources/ParseSwift/API/Responses.swift 91.25% <ø> (ø)
Sources/ParseSwift/Objects/ParseUser+async.swift 100.00% <ø> (ø)
Sources/ParseSwift/Objects/ParseUser+combine.swift 95.27% <100.00%> (ø)
Sources/ParseSwift/Objects/ParseUser.swift 82.66% <100.00%> (+0.03%) :arrow_up:
Sources/ParseSwift/Extensions/Data.swift 40.00% <0.00%> (-60.00%) :arrow_down:
Sources/ParseSwift/Storage/ParseFileManager.swift 81.14% <0.00%> (-6.86%) :arrow_down:
Sources/ParseSwift/LiveQuery/ParseLiveQuery.swift 72.92% <0.00%> (-0.60%) :arrow_down:
Sources/ParseSwift/Types/ParseAnalytics.swift 98.34% <0.00%> (-0.38%) :arrow_down:
Sources/ParseSwift/Objects/ParseObject.swift 81.98% <0.00%> (-0.31%) :arrow_down:
... and 2 more

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 96232f3...0a1ff59. Read the comment docs.

cbaker6 commented 2 years ago

Can you provide appropriate testcases along with fixing the tests that are failing?

You also need to lint your branch https://github.com/parse-community/Parse-Swift/blob/main/CONTRIBUTING.md#setting-up-your-local-machine

What needs to be investigated is if the server or client is responsible for throwing the error. It seems like something like this should come from the server as the client isn’t aware of the server config.

Climbatize commented 2 years ago

Can you also put this on a different branch?

What is your branching model?

Climbatize commented 2 years ago

Can you provide appropriate testcases along with fixing the tests that are failing?

You also need to lint your branch https://github.com/parse-community/Parse-Swift/blob/main/CONTRIBUTING.md#setting-up-your-local-machine

What needs to be investigated is if the server or client is responsible for throwing the error. It seems like something like this should come from the server as the client isn’t aware of the server config.

I provided test cases and tests are running well locally, do you know what's happening on Github Action?

image

cbaker6 commented 2 years ago

I'm not able to replicate your issue based on what's been provided so far. IMO, this doesn't seem like the right direction. I recommend following the directions here: https://github.com/parse-community/Parse-Swift/issues/313#issuecomment-1020310987

I provided test cases and tests are running well locally, do you know what's happening on Github Action?

Not sure, but if actions is showing these type of errors, there's most likely something wrong with the PR as oppose to actions. Particularly since the errors look the same across multiple OS's.

From your screenshot, it looks like you opened the Package.swift file to run your tests, but the failures are in the Parse.xcworkspace, you can try opening that and running your tests.

Climbatize commented 2 years ago

solved by #332 Seems like it's not today you'll get another contributor from the community :). Thanks @cbaker6 and @vdkdamian for your help

cbaker6 commented 2 years ago

@Climbatize you found out there's an issue, so you are definitely a contributor!

I'm sure there's something you will add soon to the SDK!

Thanks for reporting!!!