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 Null check to prevent crash for TrailHead Go Android #2633

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

Copy link

salesforce-cla bot commented Nov 4, 2024

Thanks for the contribution! It looks like @umashankar-sf is an internal user so signing the CLA is not required. However, we need to confirm this.

@mobilesdk-bot
Copy link

1 Warning
⚠️ No Lint Results.

Tests results for SalesforceSDK

Generated by 🚫 Danger

@mobilesdk-bot
Copy link

1 Warning
⚠️ No Lint Results.

Tests results for SmartStore

Generated by 🚫 Danger

@mobilesdk-bot
Copy link

1 Warning
⚠️ No Lint Results.

Tests results for SalesforceHybrid

Generated by 🚫 Danger

@mobilesdk-bot
Copy link

1 Warning
⚠️ No Lint Results.

Tests results for MobileSync

Generated by 🚫 Danger

@mobilesdk-bot
Copy link

1 Error
🚫 Tests have failed, see below for more information.
1 Warning
⚠️ No Lint Results.

Tests:

Name Classname Time
test[testCleanResyncGhosts] com.salesforce.androidsdk.reactnative.ReactMobileSyncTest 132.474

Tests results for SalesforceReact

Generated by 🚫 Danger

@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 58.82%. Comparing base (8a67d31) to head (2d8a8c0).
Report is 5 commits behind head on dev.

Files with missing lines Patch % Lines
.../com/salesforce/androidsdk/rest/ClientManager.java 0.00% 1 Missing and 1 partial ⚠️
...m/salesforce/androidsdk/security/AppLockManager.kt 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #2633      +/-   ##
============================================
- Coverage     58.93%   58.82%   -0.12%     
+ Complexity     2456     2450       -6     
============================================
  Files           189      189              
  Lines         15475    15477       +2     
  Branches       1998     2000       +2     
============================================
- Hits           9120     9104      -16     
- Misses         5426     5439      +13     
- Partials        929      934       +5     
Flag Coverage Δ
MobileSync 82.02% <ø> (ø)
SalesforceHybrid 57.36% <ø> (ø)
SalesforceReact 51.38% <ø> (-0.98%) ⬇️
SalesforceSDK 45.02% <0.00%> (-0.14%) ⬇️
SmartStore 78.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...m/salesforce/androidsdk/security/AppLockManager.kt 96.96% <0.00%> (-3.04%) ⬇️
.../com/salesforce/androidsdk/rest/ClientManager.java 45.02% <0.00%> (-0.40%) ⬇️

... and 3 files with indirect coverage changes

}

fun getGlobalPrefs(): SharedPreferences {
val ctx = SalesforceSDKManager.getInstance().appContext
return ctx.getSharedPreferences(policyKey, Context.MODE_PRIVATE)
}

fun getPolicy(account: UserAccount): Policy {
fun getPolicy(account: UserAccount?): Policy {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect. There are two places where this is API is used and both check the account is not null first.

Copy link
Contributor

@brandonpage brandonpage left a comment

Choose a reason for hiding this comment

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

I don't understand what scenario this change is for. When would the options Bundle ever be null?

If you are launching the LoginActivity yourself (which I do not recommend) you need to supply the correct values so we can use them here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants