Swe 13 choose bank account #15

rrk12005 · 2019-04-07T21:51:10Z

This branch allows the user to choose the bank they want to use. Need to hook up to the makePayment API call

… type. Fetching bank account info and populating table with it. Updated getbankacctinfo request to grab the bank name and last 4 digits of bank account.

…type. Fetching bank account info and populating table with it. Updated getbankacctinfo request to grab the bank name and last 4 digits of bank account.

…g of account info, just able to display chosen bank name

…. new structure of storing default bank account in UseraDefaults

ahm11003 · 2019-04-08T01:43:58Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/BankAcctCell.swift

+import WatchKit
+
+class BankAcctCell: NSObject {
+    @IBOutlet weak var BankNameLabel: WKInterfaceLabel!


variable names should begin with a lowercase and follow camel case style.

ahm11003 · 2019-04-08T01:44:08Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/BankAcctCell.swift

+
+class BankAcctCell: NSObject {
+    @IBOutlet weak var BankNameLabel: WKInterfaceLabel!
+    @IBOutlet weak var Last4Label: WKInterfaceLabel!


same as above

ahm11003 · 2019-04-08T01:47:50Z

SynchronyFinancial/SynchronyFinancial/BankAcct.swift

+    }
+}
+
+extension UserDefaults {


This extension on UserDefaults should be in its own file, namely UserDefaults+Extensions.swift

ahm11003 · 2019-04-08T01:50:10Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/PaymentDetailInterfaceController.swift

+       paymentButton.setHidden(hide)
+       payFrom.setHidden(hide)
+       separate.setHidden(hide)
+       /*if(!hide){


debug code should be removed prior to merging into master

ahm11003 · 2019-04-08T01:53:48Z

...hronyFinancial/SynchronyFinancial WatchKit Extension/PaymentOptionsInterfaceController.swift

+
+    override func table(_ table: WKInterfaceTable, didSelectRowAt rowIndex: Int) {
+        let acct = bankAccts[rowIndex]
+        self.dictForDefault.updateValue(String(acct.acctType.rawValue), forKey: "acctType")


So this line is creating a new String from the acct.acctType.rawValue which will evaluate to either "1" or "0". Is that the intended behavior? Is there a reason we couldn't just use the rawValue which is int?

Also, for this line and the lines below, we don't need the explicit use of self

since it's a dictionary of strings, this value needs to be a string, therefore the conversion from int to string is necessary to store the value in the dict

So in that case we should change the raw value of AccountType to be String so we can avoid this conversion. The raw values of checking and savings can simply be string representations of those words

good point. I'll add that with my changes

ahm11003

Overall this PR looks good. A few changes need to be made before it is ready to merge. Also, is it possible to populate the bank account and only fire off the web service if the user wants to change that account?

ahm11003 · 2019-04-08T03:15:54Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/PaymentDetailInterfaceController.swift


    override func awake(withContext context: Any?) {
+        configureInterfaceObjects(true)
+        activityIndicator.configureForActivityIndicator()
        super.awake(withContext: context)


super.awake(withContext: context) should be the first call within this function

Also, I think you intend to show the activityIndicatorLabel here and then hide it again once web service returns

# Conflicts: # SynchronyFinancial/SynchronyFinancial WatchKit App/Base.lproj/Interface.storyboard # SynchronyFinancial/SynchronyFinancial WatchKit Extension/FetchData.swift # SynchronyFinancial/SynchronyFinancial WatchKit Extension/PaymentDetailInterfaceController.swift # SynchronyFinancial/SynchronyFinancial/Defaults.swift

ahm11003 · 2019-04-08T03:17:37Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/PaymentDetailInterfaceController.swift

@@ -44,14 +58,37 @@ class PaymentDetailInterfaceController: WKInterfaceController {
            paymentButton.setTitle("Pay Now")
        }
    }
+
+    @IBAction func bankAccountsTapped() {
+        self.pushController(withName: "PaymentOptions", context: self.dictForBankAcct)


unnecessary use of explicit self in function call as well as when passing in dictForBankAcct

ahm11003 · 2019-04-08T03:21:49Z

Additionally, when we currently make a payment we pass in the last 4 digits of the bank account to pay from. So, we should either update that function to retrieve the value stored in UserDefaults and remove that parameter, or we should update existing calls to the make payment function to retrieve the value from UserDefaults prior to making the call.

…o API request is called so it is fired upon only when user want to change default bank acct. User's default bank account ID is now being used in the makePayment API call.

ahm11003 · 2019-04-08T20:15:58Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/FetchData.swift

@@ -131,11 +130,11 @@ class FetchData {
                //"fetch_bank_account_details"
                dict["fetch_bank_account_details"]?.arrayValue.forEach {
                    if let bankId = $0["bank_account_id"].string,
+                        let bankName = $0["bank_name"].string,
+                        let last4Acct = $0["last4_bank_acct_number"].string,
                        let accttype: AccountType = $0["bank_account_type"].stringValue ==


So now that we have a raw value of String for AccountType we should just be able to say:
let accttype = AccountType(rawValue: $0["bank_account_type"].stringValue)

ahm11003 · 2019-04-08T20:17:03Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/PaymentDetailInterfaceController.swift

        }

+


we can remove this blank line and the line below

ahm11003 · 2019-04-08T20:19:48Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/PaymentDetailInterfaceController.swift

@@ -50,7 +54,11 @@ class PaymentDetailInterfaceController: WKInterfaceController {
                let type: PaymentType = self.paymentAmount == self.selectedAccount?.curBalance ? .currentBal : .minimumDue

                // process this payment using default bank account (9999) until ability to change account is implemented
-                FetchData.submitPayment(for: alias, type: type, amount: self.paymentAmount, bankID: "9999") { confirmationNum, paymentID, error in
+                if let bankacct = UserDefaults.standard.dictionary(forKey: "default_bank_acct") as? [String: String] {
+                    self.bankID = bankacct["bankAcctId"] ?? "9999"


So we shouldn't be falling back on "9999" as a "default" bank account id unless that is consistent across ALL user accounts.

ahm11003 · 2019-04-08T20:20:32Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/PaymentDetailInterfaceController.swift

+    @IBAction func bankAccountsTapped() {
+        configureInterfaceObjects(true)
+        activityIndicator.configureForActivityIndicator()
+        FetchData.getBankInfo { accts, error in guard error == nil else {


guard should be dropped down inside the closure

ahm11003 · 2019-04-08T20:21:47Z

...hronyFinancial/SynchronyFinancial WatchKit Extension/PaymentOptionsInterfaceController.swift

+
+    override func table(_ table: WKInterfaceTable, didSelectRowAt rowIndex: Int) {
+        let acct = bankAccts[rowIndex]
+        self.dictForDefault.updateValue(acct.acctType.rawValue, forKey: "acctType")


remove explicit use of self on this line and subsequent lines

ahm11003 · 2019-04-08T20:22:53Z

...hronyFinancial/SynchronyFinancial WatchKit Extension/PaymentOptionsInterfaceController.swift

+        }
+        bankAccts = accts
+        configureRows()
+    }


insert new line after this closing brace

ahm11003 · 2019-04-08T20:23:03Z

...hronyFinancial/SynchronyFinancial WatchKit Extension/PaymentOptionsInterfaceController.swift

+    override func willActivate() {
+        // This method is called when watch view controller is about to be visible to user
+        super.willActivate()
+    }


insert new line after this closing brace

ahm11003 · 2019-04-08T20:23:15Z

...hronyFinancial/SynchronyFinancial WatchKit Extension/PaymentOptionsInterfaceController.swift

+    override func didDeactivate() {
+        // This method is called when watch view controller is no longer visible
+        super.didDeactivate()
+    }


insert new line after this closing brace

ahm11003 · 2019-04-09T19:50:04Z

SynchronyFinancial/SynchronyFinancial WatchKit Extension/PaymentDetailInterfaceController.swift

@@ -50,12 +52,17 @@ class PaymentDetailInterfaceController: WKInterfaceController {
                let type: PaymentType = self.paymentAmount == self.selectedAccount?.curBalance ? .currentBal : .minimumDue

                // process this payment using default bank account (9999) until ability to change account is implemented
-                FetchData.submitPayment(for: alias, type: type, amount: self.paymentAmount, bankID: "9999") { confirmationNum, paymentID, error in
+                guard let bankacct = UserDefaults.standard.dictionary(forKey: "default_bank_acct") as? [String: String],  let bankID = bankacct["bankAcctId"] else{


Our extension on UserDefaults stores a BankAcct object under this same key, can we utilize those functions we wrote here? Like:
guard let bankacct = UserDefaults.getDefaultBankAccount() as? BankAcct
and then pass in bankacct.bankID to the call to submitPayment

# Conflicts: # SynchronyFinancial/SynchronyFinancial WatchKit Extension/TransactionsInterfaceController.swift # SynchronyFinancial/SynchronyFinancial.xcodeproj/project.pbxproj

…ctInfo from UserDefaults in updating the selected bank account label as well as passing bankId in submit payment

rrk12005 added 5 commits April 5, 2019 14:15

Created PaymentOptions interface and controller for selecting payment…

63bb56b

… type. Fetching bank account info and populating table with it. Updated getbankacctinfo request to grab the bank name and last 4 digits of bank account.

reated PaymentOptions interface and controller for selecting payment …

6ffbf64

…type. Fetching bank account info and populating table with it. Updated getbankacctinfo request to grab the bank name and last 4 digits of bank account.

SWF-13 updating default payment option

18c29c8

SWF-13 able to choose bank that user would like to pay for. No passin…

0b6fb67

…g of account info, just able to display chosen bank name

SWF-13 finished addition of ability to choose bank account to pay for…

76ef026

…. new structure of storing default bank account in UseraDefaults

rrk12005 requested review from ahm11003, jod13004 and tiw15001 April 7, 2019 21:51

ahm11003 reviewed Apr 8, 2019

View reviewed changes

ahm11003 requested changes Apr 8, 2019

View reviewed changes

ahm11003 reviewed Apr 8, 2019

View reviewed changes

rrk12005 added 2 commits April 8, 2019 14:09

Fixed errors from code review. Additionally, moved where fetchBankInf…

520d31a

…o API request is called so it is fired upon only when user want to change default bank acct. User's default bank account ID is now being used in the makePayment API call.

hotfix for passing default bank acct id

cc16cdf

ahm11003 reviewed Apr 8, 2019

View reviewed changes

SWE-13 PR Requested changes

3594b68

ahm11003 reviewed Apr 9, 2019

View reviewed changes

ahm11003 and others added 3 commits April 9, 2019 15:53

SWE-13 [swiftlint] resolve swiftlint warnings

0c9a242

Merge branch 'master' into SWE-13-choose-bank-account

8b78622

# Conflicts: # SynchronyFinancial/SynchronyFinancial WatchKit Extension/TransactionsInterfaceController.swift # SynchronyFinancial/SynchronyFinancial.xcodeproj/project.pbxproj

SWE-13 Integration of UserDefaults+Extension as the getter for bankAc…

d60bd62

…ctInfo from UserDefaults in updating the selected bank account label as well as passing bankId in submit payment

ahm11003 approved these changes Apr 10, 2019

View reviewed changes

ahm11003 merged commit 8d33cc1 into master Apr 10, 2019

Swe 13 choose bank account #15

Swe 13 choose bank account #15

rrk12005 commented Apr 7, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

rrk12005 Apr 8, 2019

ahm11003 Apr 8, 2019

rrk12005 Apr 8, 2019

ahm11003 left a comment

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019 •

edited

ahm11003 commented Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 8, 2019

ahm11003 Apr 9, 2019 •

edited

Swe 13 choose bank account #15

Swe 13 choose bank account #15

Conversation

rrk12005 commented Apr 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahm11003 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahm11003 Apr 8, 2019 • edited

Choose a reason for hiding this comment

ahm11003 commented Apr 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahm11003 Apr 9, 2019 • edited

Choose a reason for hiding this comment

ahm11003 Apr 8, 2019 •

edited

ahm11003 Apr 9, 2019 •

edited