[BITAU-149] Add Setting to Turn On Authenticator Syncing for an Account #957
Conversation
|
No New Or Fixed Issues Found |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #957 +/- ##
=======================================
Coverage 88.76% 88.77%
=======================================
Files 638 638
Lines 40087 40129 +42
=======================================
+ Hits 35585 35625 +40
- Misses 4502 4504 +2 ☔ View full report in Codecov by Sentry. |
| XCTAssertEqual(publishedValues[0].userId, "1") | ||
| XCTAssertEqual(publishedValues[0].shouldSync, true) | ||
| XCTAssertEqual(publishedValues[1].userId, "1") | ||
| XCTAssertEqual(publishedValues[1].shouldSync, false) |
There was a problem hiding this comment.
🎨 You can use == operator to reduce this a bit more:
| XCTAssertEqual(publishedValues[0].userId, "1") | |
| XCTAssertEqual(publishedValues[0].shouldSync, true) | |
| XCTAssertEqual(publishedValues[1].userId, "1") | |
| XCTAssertEqual(publishedValues[1].shouldSync, false) | |
| XCTAssertTrue(publishedValues[0] == (userId: "1", shouldSync: false)) | |
| XCTAssertTrue(publishedValues[1] == (userId: "1", shouldSync: true)) |
There was a problem hiding this comment.
The downside of doing XCTAssertTrue( == ) is that the failure message just says "they're not equal", whereas if you use XCTAssertEqual the failure message indicates the value of the variable, so you can better figure out why a test might be failing. IMO you should aim for the XCTAssertEquals, XCTAssertLessThan, and XCTAssertGreaterThan functions as much as possible for this reason.
There was a problem hiding this comment.
Ah, good call. Update all of them to this style in latest. 👍
There was a problem hiding this comment.
Sorry, didn't see @KatherineInCode's comment when I updated. Let me know which one y'all prefer. I'm good either way.
There was a problem hiding this comment.
@brant-livefront Let's go with the XCTAssertEquals approach. Depending on how often we need to check the tuple in tests, a custom assertion function might also be helpful.
There was a problem hiding this comment.
@KatherineInCode Sounds good, I'll roll back to using explicit XCTAssertEquals.
There was a problem hiding this comment.
@KatherineInCode this is updated in the latest commit. 👍
|
@fedemkr @KatherineInCode Just a reminder to let me know which direction you want to go on this (XCTAssertTrue vs XCTAssertEqual). I have another PR that I'd like to put up, but it's dependent on the stuff in this PR. So I'm waiting to put that up until we solve this. Thanks! |
🎟️ Tracking
[BITAU-149]
📔 Objective
This PR adds a
syncToAuthenticatorsetting that will allow the sync to be turned on/off for specific accounts in the BW PM app. It was very closely modeled after all of theconnectToWatchpieces. There is no UI to turn this on (that will be added last), but putting this in now allows us to begin hooking off the setting when doing the sync.⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes