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

Safer check of .appStore vs .testFlight build, and enables TESTFLIGHT deployment #76

Merged
merged 2 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions Utilities/Utilities/_Utils/Installation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,30 @@ public class Installation {
case appStore
case jailBroken // potentially side-loaded
}

// This is private because the use of 'appConfiguration' is preferred.
private static let isTestFlight = Bundle.main.appStoreReceiptURL?.lastPathComponent == "sandboxReceipt"


private static let isAppStore = {
// Keep this code for reference. In case "sandboxReceipt" changes in later iOS
if let receipt: URL = Bundle.main.appStoreReceiptURL {
var error: NSError?
if (receipt as NSURL).checkResourceIsReachableAndReturnError(&error), error == nil {
return true
}
}
return false
}()

// This can be used to add debug statements.
static var isDebug: Bool {
#if DEBUG
return true
return true
#else
return false
return false
#endif
}

private static var isJailBroken: Bool = {
#if targetEnvironment(simulator)
return false
Expand All @@ -54,9 +65,8 @@ public class Installation {
}
#endif
}()

public static var source: Source {
return .appStore
if isJailBroken {
return .jailBroken
} else if isDebug {
Expand All @@ -67,7 +77,7 @@ public class Installation {
return .appStore
}
}

public static var isSimulator: Bool = {
#if targetEnvironment(simulator)
return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ public class CompositeFeatureFlagsProvider: NSObject & FeatureFlagsProtocol {

public func flag(feature: String?) -> Any? {
switch Installation.source {
case .appStore, .jailBroken, .testFlight:
case .appStore, .jailBroken:
return remote?.flag(feature: feature)
case .debug:
case .debug, .testFlight:
if let localFlag = local?.flag(feature: feature) {
return localFlag
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ public final class AbacusStateManager: NSObject {
switch Installation.source {
case .appStore:
deployment = "MAINNET"
case .testFlight, .debug, .jailBroken:
case .debug, .jailBroken:
deployment = "TESTNET"
case .testFlight:
deployment = "TESTFLIGHT"
Comment on lines +158 to +159
Copy link
Contributor

@mike-dydx mike-dydx Jan 30, 2024

Choose a reason for hiding this comment

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

what is the value/benefit of TESTFLIGHT ?

I am assuming the TESTFLIGHT deployment contains mainnet and testnet environments

If app store review build results in testFlight (we are not certain), then they will see those as the default selectable environments. App review might say "you cannot expose beta/test environments to users" and reject. I think apple might like to see that test environment access is behind special submission instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TESTFLIGHT deployment would have both mainnet and testnet, and default to mainnet, so unless reviewer enables debug and then switch to testnet environment, he is on mainnet.

It should be something like:

"TESTFLIGHT": {
         "environments": [
            "dydxprotocol-mainnet"
            "dydxprotocol-testnet"
         ],
         "default": "dydxprotocol-mainnet"
      },

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right. I am forgetting that the menu is hidden unless the Debug flag is enabled via QR code. I will get this updated in v4-web as well.

Copy link
Contributor Author

@johnqh johnqh Jan 30, 2024

Choose a reason for hiding this comment

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

I am doing a PR on v4-web for this

}
appConfigs = AppConfigs.companion.forApp
}
Expand Down
Loading