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

Updates hideCvv flag for edit payment methods and new payment methods. #1109

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
1ece775
Updates hideCvv flag for edit payment methods and new payment methods.
wjames111 Oct 28, 2024
1a99566
Adds custom directive for CVV validation.
wjames111 Nov 5, 2024
75721b2
Pass function for disabling continue button when cvv is invalid.
wjames111 Nov 8, 2024
dd884e6
disable continue button when cvv is invalid.
wjames111 Nov 8, 2024
b6e6af9
Renames function.
wjames111 Nov 8, 2024
b34a965
Move template into directive.
wjames111 Nov 13, 2024
7ab82ad
Removes commented out code.
wjames111 Nov 13, 2024
eaed35e
Updates form and validators in directive.
wjames111 Nov 14, 2024
4a08f18
Remove console logs.
wjames111 Nov 14, 2024
9933f03
Uses existing validators rather then adding them into the directive.
wjames111 Nov 14, 2024
b12a6de
Adds directive to creditCardForm.component.
wjames111 Nov 14, 2024
fe82a59
Fixes broken tests.
wjames111 Nov 14, 2024
eb8d216
Adds test for cvv.
wjames111 Nov 18, 2024
4e0adf5
Update confusing function name.
wjames111 Nov 19, 2024
b4eac83
Adds cvv outside of payment methods loop. updates classNames for styl…
wjames111 Nov 19, 2024
100f09f
Updates cvv test.
wjames111 Nov 19, 2024
ad30a26
Clear securityCode viewValue on switchPayment.
wjames111 Nov 19, 2024
cfbb0e4
Remove console.log.
wjames111 Nov 19, 2024
9e19f3a
Fix lint issues.
wjames111 Nov 19, 2024
db41374
Add check for selected payment type before disabling continue button.
wjames111 Nov 20, 2024
3e705e0
Adds test for getContinueDisabled when selection is not credit-card.
wjames111 Nov 20, 2024
d4e68b1
Update to handle EFT.
wjames111 Nov 21, 2024
872c3a0
Add tests for new payment methods.
wjames111 Nov 21, 2024
ba4b235
Remove hideCvv from ng-if.
wjames111 Nov 21, 2024
375cfe9
Run linter.
wjames111 Nov 21, 2024
72beba9
Fix tests.
wjames111 Nov 21, 2024
2adbcf1
Fixup tests.
wjames111 Nov 22, 2024
4ce72f3
Test updates.
wjames111 Nov 22, 2024
4fa3d1a
Fix console error, refactor tests.
wjames111 Nov 25, 2024
5d09d9d
Fix broken test.
wjames111 Nov 25, 2024
4204964
Fix for flaky test.
wjames111 Nov 25, 2024
22c3206
Fix for needing to reenter CVV upon new payment
wjames111 Nov 25, 2024
aeccc63
cleanup tests.
wjames111 Nov 25, 2024
c4717db
Lint
wjames111 Nov 25, 2024
95ef487
Add additional test case.
wjames111 Nov 25, 2024
f58818e
Remove conditional, breakup test to multiple lines.
wjames111 Nov 25, 2024
0497010
Add CVV to sessionStorage on continue; Rename tests.
wjames111 Nov 26, 2024
8134562
lint.
wjames111 Nov 26, 2024
0709f6a
Fix broken tests.
wjames111 Nov 26, 2024
74a673b
Remove uneeded test.
wjames111 Nov 27, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,38 @@ import paymentMethodDisplay from 'common/components/paymentMethods/paymentMethod
import paymentMethodFormModal from 'common/components/paymentMethods/paymentMethodForm/paymentMethodForm.modal.component'
import coverFees from 'common/components/paymentMethods/coverFees/coverFees.component'

import * as cruPayments from '@cruglobal/cru-payments/dist/cru-payments'
import orderService from 'common/services/api/order.service'
import cartService from 'common/services/api/cart.service'
import { validPaymentMethod } from 'common/services/paymentHelpers/validPaymentMethods'
import giveModalWindowTemplate from 'common/templates/giveModalWindow.tpl.html'
import { SignInEvent } from 'common/services/session/session.service'

import creditCardCvv from '../../../../common/directives/creditCardCvv.directive'
import template from './existingPaymentMethods.tpl.html'

const componentName = 'checkoutExistingPaymentMethods'

class ExistingPaymentMethodsController {
/* @ngInject */
constructor ($log, $scope, orderService, cartService, $uibModal) {
constructor ($log, $scope, orderService, cartService, $uibModal, $window) {
this.$log = $log
this.$scope = $scope
this.orderService = orderService
this.cartService = cartService
this.$uibModal = $uibModal
this.paymentFormResolve = {}
this.validPaymentMethod = validPaymentMethod
this.sessionStorage = $window.sessionStorage

this.$scope.$on(SignInEvent, () => {
this.$onInit()
})
}

$onInit () {
this.enableContinue({ $event: false })
this.loadPaymentMethods()
this.waitForFormInitialization()
}

$onChanges (changes) {
Expand All @@ -52,6 +56,23 @@ class ExistingPaymentMethodsController {
}
}

waitForFormInitialization () {
const unregister = this.$scope.$watch('$ctrl.creditCardPaymentForm.securityCode', () => {
if (this.creditCardPaymentForm && this.creditCardPaymentForm.securityCode) {
unregister()
this.addCvvValidators()
}
})
}

addCvvValidators () {
this.$scope.$watch('$ctrl.creditCardPaymentForm.securityCode.$viewValue', (number) => {
this.creditCardPaymentForm.securityCode.$validators.minLength = cruPayments.creditCard.cvv.validate.minLength
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 causing an error on localhost. securityCode is undefined. This might work staging, butI have nly tested on local.

Copy link
Author

Choose a reason for hiding this comment

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

I did notice this error as well. I'll see if I can get a fix for this.

this.creditCardPaymentForm.securityCode.$validators.maxLength = cruPayments.creditCard.cvv.validate.maxLength
this.enableContinue({ $event: cruPayments.creditCard.cvv.validate.minLength(number) && cruPayments.creditCard.cvv.validate.maxLength(number) })
})
}

loadPaymentMethods () {
this.orderService.getExistingPaymentMethods()
.subscribe((data) => {
Expand Down Expand Up @@ -80,6 +101,7 @@ class ExistingPaymentMethodsController {
// Select the first payment method
this.selectedPaymentMethod = paymentMethods[0]
}

this.switchPayment()
}

Expand Down Expand Up @@ -130,6 +152,22 @@ class ExistingPaymentMethodsController {

switchPayment () {
this.onPaymentChange({ selectedPaymentMethod: this.selectedPaymentMethod })
if (this.selectedPaymentMethod?.['card-type'] && this.creditCardPaymentForm?.securityCode) {
const selectedUri = this.selectedPaymentMethod.self.uri
const storage = JSON.parse(this.sessionStorage.getItem('storedCvvs'))
const getSelectedCvv = storage ? storage[Object.keys(storage).filter((item) => item === selectedUri)] : false
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use '' instead of false you don't need the conditional below.

Copy link
Author

Choose a reason for hiding this comment

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

Oh nice I like that!


if (getSelectedCvv) {
// Set CVV to new credit card CVV
this.creditCardPaymentForm.securityCode.$setViewValue(getSelectedCvv)
} else {
// Clear CVV when switching between payment credit card payment methods
this.creditCardPaymentForm.securityCode.$setViewValue('')
}

this.creditCardPaymentForm.securityCode.$render()
}

if (this.selectedPaymentMethod?.['bank-name']) {
// This is an EFT payment method so we need to remove any fee coverage
this.orderService.storeCoverFeeDecision(false)
Expand All @@ -144,7 +182,8 @@ export default angular
paymentMethodFormModal.name,
coverFees.name,
orderService.name,
cartService.name
cartService.name,
creditCardCvv.name
])
.component(componentName, {
controller: ExistingPaymentMethodsController,
Expand All @@ -159,6 +198,7 @@ export default angular
brandedCheckoutItem: '<',
onPaymentFormStateChange: '&',
onPaymentChange: '&',
onLoad: '&'
onLoad: '&',
enableContinue: '&'
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Observable } from 'rxjs/Observable'
import 'rxjs/add/observable/of'
import 'rxjs/add/observable/throw'
import 'rxjs/add/operator/toPromise'
import * as cruPayments from '@cruglobal/cru-payments/dist/cru-payments'

import { SignInEvent } from 'common/services/session/session.service'

Expand All @@ -15,24 +16,39 @@ describe('checkout', () => {
beforeEach(angular.mock.module(module.name))
const self = {}

beforeEach(inject(($componentController, $timeout) => {
beforeEach(inject(($componentController, $timeout, $window) => {
self.$timeout = $timeout

self.controller = $componentController(module.name, {}, {
onLoad: jest.fn(),
onPaymentChange: jest.fn(),
enableContinue: jest.fn(),
onPaymentFormStateChange: jest.fn(),
cartData: { items: [] }
cartData: { items: [] },
creditCardPaymentForm: {
securityCode: {
$valid: true,
$validators: {
minLength: (value) => cruPayments.creditCard.cvv.validate.minLength(value),
maxLength: cruPayments.creditCard.cvv.validate.maxLength
},
$setViewValue: jest.fn(),
$render: jest.fn(),
}
}
})
self.$window = $window
self.$window.sessionStorage.clear()
}))


describe('$onInit', () => {
it('should call loadPaymentMethods', () => {
jest.spyOn(self.controller, 'loadPaymentMethods').mockImplementation(() => {})
jest.spyOn(self.controller, 'waitForFormInitialization').mockImplementation(() => {})
self.controller.$onInit()

expect(self.controller.loadPaymentMethods).toHaveBeenCalled()
expect(self.controller.waitForFormInitialization).toHaveBeenCalled()
})

it('should be called on sign in', () => {
Expand Down Expand Up @@ -329,6 +345,79 @@ describe('checkout', () => {
expect(self.controller.onPaymentChange).toHaveBeenCalledWith({ selectedPaymentMethod: undefined })
expect(self.controller.orderService.storeCoverFeeDecision).not.toHaveBeenCalled()
})

it('should reset securityCode viewValue', () => {
self.controller.creditCardPaymentForm.securityCode.$viewValue = '123'
self.controller.selectedPaymentMethod = { 'card-type': 'Visa', self: { type: 'cru.creditcards.named-credit-card', uri: 'selected uri' }, selectAction: 'some uri' }
self.controller.switchPayment()

expect(self.controller.creditCardPaymentForm.securityCode.$setViewValue).toHaveBeenCalledWith('')
expect(self.controller.creditCardPaymentForm.securityCode.$render).toHaveBeenCalled()
})

it('should add securityCode viewValue from sessionStorage', () => {
self.controller.creditCardPaymentForm.securityCode.$viewValue = '123'
self.controller.selectedPaymentMethod = { 'card-type': 'Visa', self: { type: 'cru.creditcards.named-credit-card', uri: '/paymentmethods/crugive/giydsnjqgi=' }, selectAction: 'some uri' }
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is a bit too long, let's break it up to multiple lines.

self.$window.sessionStorage.setItem('storedCvvs', '{"/paymentmethods/crugive/giydsnjqgi=":"456","/paymentmethods/crugive/giydsnjqgy=":"321"}')
self.controller.switchPayment()

expect(self.controller.creditCardPaymentForm.securityCode.$setViewValue).toHaveBeenCalledWith('456')
expect(self.controller.creditCardPaymentForm.securityCode.$render).toHaveBeenCalled()
})
})

describe('addCvvValidators', () => {
it('should add validator functions to creditCardPaymentForm.securityCode', () => {
wrandall22 marked this conversation as resolved.
Show resolved Hide resolved
jest.spyOn(self.controller, 'addCvvValidators').mockImplementation(() => {
self.controller.creditCardPaymentForm.securityCode.$validators = {
minLength: cruPayments.creditCard.cvv.validate.minLength,
maxLength: cruPayments.creditCard.cvv.validate.maxLength
}
})
delete self.controller.creditCardPaymentForm
self.controller.waitForFormInitialization()
self.controller.$scope.$digest()

expect(self.controller.addCvvValidators).not.toHaveBeenCalled()
self.controller.creditCardPaymentForm = {
$valid: true,
$dirty: false,
securityCode: {
$viewValue: '123',
}
}
self.controller.$scope.$digest()

expect(self.controller.addCvvValidators).toHaveBeenCalled()
expect(Object.keys(self.controller.creditCardPaymentForm.securityCode.$validators).length).toEqual(2)
expect(typeof self.controller.creditCardPaymentForm.securityCode.$validators.minLength).toBe('function')
expect(typeof self.controller.creditCardPaymentForm.securityCode.$validators.maxLength).toBe('function')

})

it('should call enableContinue when cvv is valid', () => {
self.controller.creditCardPaymentForm.securityCode.$viewValue = '123'
self.controller.addCvvValidators()
self.controller.$scope.$apply()

expect(self.controller.enableContinue).toHaveBeenCalledWith({ $event: true })
})

it('should call enableContinue when cvv is too long', () => {
self.controller.creditCardPaymentForm.securityCode.$viewValue = '12345'
self.controller.addCvvValidators()
self.controller.$scope.$apply()

expect(self.controller.enableContinue).toHaveBeenCalledWith({ $event: false })
})

it('should call enableContinue when cvv is too short', () => {
self.controller.creditCardPaymentForm.securityCode.$viewValue = '1'
self.controller.addCvvValidators()
self.controller.$scope.$apply()

expect(self.controller.enableContinue).toHaveBeenCalledWith({ $event: false })
})
})
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@
<button class="btn btn-xs btn-link" ng-click="$ctrl.openPaymentMethodFormModal(paymentMethod)" ng-if="paymentMethod['card-type']" translate>edit</button>
</label>
</div>

<div ng-if="$ctrl.selectedPaymentMethod['card-type']" class="row">
<div class="col-sm-4">
<form novalidate name="$ctrl.creditCardPaymentForm">
<credit-card-cvv ></credit-card-cvv>
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't seem like the cvv is getting stored in sessionStorage here like it does in other cases, which means it probably isn't getting sent to the server properly. This is due to the unsetting of the cvv in selectPayment but never re-setting it anywhere else (e.g. on hitting continue)

Copy link
Contributor

Choose a reason for hiding this comment

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

bump

</form>
</div>
</div>

<div class="panel panel-default tab-toggle mb0 mt"
ng-if="(($ctrl.cartData && $ctrl.cartData.items) || $ctrl.brandedCheckoutItem) && $ctrl.selectedPaymentMethod && $ctrl.selectedPaymentMethod['card-type']">
<div class="panel-title panel-heading" translate>{{'OPTIONAL'}}</div>
Expand Down
9 changes: 8 additions & 1 deletion src/app/checkout/step-2/step-2.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ class Step2Controller {
}
}

getContinueDisabled () {
isContinueDisabled () {
if (this.selectedPaymentMethod?.['card-type'] && typeof this.isCvvValid !== 'undefined' && !this.isCvvValid) {
Copy link
Contributor

Choose a reason for hiding this comment

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

so if isCvvValid is undefined we enable the continue button?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it that if there are no CC to ensure they are valid, we don't disable the button, so the user isn't blocked.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see on line 129 we ensure a payment method is selected.

Copy link
Author

Choose a reason for hiding this comment

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

I may be able to take that out now but initially I added typeof this.isCvvValid !== 'undefined' so that it's not disabled before enableContinue gets called.

return true
}
if (this.loadingPaymentMethods) {
return true
}
Expand All @@ -129,6 +132,10 @@ class Step2Controller {
}
return false
}

enableContinue (isCvvValid) {
this.isCvvValid = isCvvValid
wrandall22 marked this conversation as resolved.
Show resolved Hide resolved
}
}

export default angular
Expand Down
Loading