[23926] - Store & Show Challenges from Notification Extension. #188
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlejandroOrozco
requested review from
SergioFierro and
yafuquen
as code owners
AlejandroOrozco
changed the title
yafuquen
reviewed
Feb 15, 2022
| var bestAttemptContent: UNMutableNotificationContent? | ||
|
|
||
| private lazy var twilioVerify: TwilioVerify? = try? TwilioVerifyBuilder() | ||
| .setAccessGroup("group.company.identifier") |
There was a problem hiding this comment.
We could add a TODO for setting the customer's correct access group. Should we use the access group for the sample app in order to generate builds for testing?
There was a problem hiding this comment.
Added #warning instead, that would be more visible.
| #endif | ||
| twilioVerify = try builder.build() | ||
| twilioVerify = try builder | ||
| .setAccessGroup("group.company.identifier") |
There was a problem hiding this comment.
We could add a TODO for setting the customer's correct access group. Should we use the access group for the sample app in order to generate builds for testing?
There was a problem hiding this comment.
And we can include also where to update the access group for the extension
There was a problem hiding this comment.
Added #warning instead, that would be more visible. Also mentioned the NotificationExtension.
| import XCTest | ||
| @testable import TwilioVerifyDemoCache | ||
|
|
||
| final class TwilioVerifyDemoCacheTests: XCTestCase { |
| public var algorithm = kSecAttrKeyTypeECSECPrimeRandom as String | ||
| public var signatureAlgorithm = SecKeyAlgorithm.ecdsaSignatureMessageX962SHA256 | ||
| public var accessControl: SecAccessControl | ||
| public var accessGroup: String? |
There was a problem hiding this comment.
Please update the base branch to not see the changes for the SDK
AlejandroOrozco
force-pushed
the
aorozcobuiles/23926/StoreAndShowChallengesInNotificationServiceExtension
branch
from
625966d to
c5b2840
Compare
AuthyApps
reviewed
Feb 23, 2022
| } | ||
|
|
||
| func updateChallenge(withSid sid: String, withFactor factor: PushFactor, status: ChallengeStatus, success: @escaping EmptySuccessBlock, failure: @escaping TwilioVerifyErrorBlock) { | ||
| func updateChallenge( |
There was a problem hiding this comment.
Function should have complexity 10 or less: currently complexity equals 11cyclomatic_complexity PushChallengeProcessor.swift:54 |
AuthyApps
reviewed
Feb 23, 2022
| } | ||
|
|
||
| func updateChallenge(withSid sid: String, withFactor factor: PushFactor, status: ChallengeStatus, success: @escaping EmptySuccessBlock, failure: @escaping TwilioVerifyErrorBlock) { | ||
| func updateChallenge( |
There was a problem hiding this comment.
Function body should span 40 lines or less excluding comments and whitespace: currently spans 60 linesfunction_body_length PushChallengeProcessor.swift:54 |
AlejandroOrozco
force-pushed
the
aorozcobuiles/23926/StoreAndShowChallengesInNotificationServiceExtension
branch
from
c5b2840 to
a758bd6
Compare
mejiagarcia
approved these changes
Feb 23, 2022
AlejandroOrozco
force-pushed
the
aorozcobuiles/23926/StoreAndShowChallengesInNotificationServiceExtension
branch
from
a758bd6 to
2c6609e
Compare
AlejandroOrozco
force-pushed
the
aorozcobuiles/23926/StoreAndShowChallengesInNotificationServiceExtension
branch
from
2c6609e to
b1b2013
Compare
yafuquen
approved these changes
Feb 23, 2022
AlejandroOrozco
added a commit
that referenced
this pull request
Feb 24, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket
Description
Testing