From f6b412613a724c3bacff3fd28f24f2a1eb06351f Mon Sep 17 00:00:00 2001
From: Edgar Goetzendorff <hello@dated.fun>
Date: Mon, 24 Feb 2020 22:11:55 +0100
Subject: [PATCH] refactor: set currentPage / currentPerPage only if different
 from current value (#1612)

* refactor: set currentPage / currentPerPage only if different from current value

* tests: add unit tests

* fix: reset page when perpage update

* test: adjust test for reset currentPage

Co-authored-by: Davi Figueiredo <davi.mello.28@gmail.com>
Co-authored-by: Alex Barnsley <8069294+alexbarnsley@users.noreply.github.com>
---
 .../WalletTransactions.spec.js                | 62 ++++++++++++++++++-
 .../WalletTransactions/WalletTransactions.js  | 21 +++----
 2 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/__tests__/unit/components/Wallet/WalletTransactions/WalletTransactions.spec.js b/__tests__/unit/components/Wallet/WalletTransactions/WalletTransactions.spec.js
index 284d9e3b17..9601ad35f6 100644
--- a/__tests__/unit/components/Wallet/WalletTransactions/WalletTransactions.spec.js
+++ b/__tests__/unit/components/Wallet/WalletTransactions/WalletTransactions.spec.js
@@ -54,7 +54,8 @@ const createWrapper = (component, gettersTransactions) => {
         getters: {
           get 'transaction/byAddress' () {
             return gettersTransactions
-          }
+          },
+          'session/transactionTableRowCount': 10
         }
       },
       session_profile: {
@@ -532,5 +533,64 @@ describe('WalletTransactions', () => {
         expect(wrapper.vm.newTransactionsNotice).toBe('test')
       })
     })
+
+    describe('onPageChange', () => {
+      it('should do nothing if currentPage has not changed', () => {
+        const spyParams = jest.spyOn(WalletTransactionsMixin.methods, '__updateParams').mockImplementation()
+        const spyTransactions = jest.spyOn(WalletTransactionsMixin.methods, 'loadTransactions').mockImplementation()
+        createWrapper()
+
+        wrapper.vm.onPageChange({ currentPage: 1 })
+        expect(spyParams).not.toHaveBeenCalled()
+        expect(spyTransactions).toHaveBeenCalledTimes(1) // gets called once in the created lifecycle hook
+
+        spyParams.mockRestore()
+        spyTransactions.mockRestore()
+      })
+
+      it('should update params and load transactions if currentPage has changed', () => {
+        const spyParams = jest.spyOn(WalletTransactionsMixin.methods, '__updateParams').mockImplementation()
+        const spyTransactions = jest.spyOn(WalletTransactionsMixin.methods, 'loadTransactions').mockImplementation()
+        createWrapper()
+
+        wrapper.vm.onPageChange({ currentPage: 10 })
+        expect(wrapper.vm.currentPage).toBe(10)
+        expect(spyParams).toHaveBeenCalledTimes(1)
+        expect(spyTransactions).toHaveBeenCalledTimes(2) // gets called once in the created lifecycle hook
+
+        spyParams.mockRestore()
+        spyTransactions.mockRestore()
+      })
+    })
+
+    describe('onPerPageChange', () => {
+      it('should do nothing if currentPerPage has not changed', () => {
+        const spyParams = jest.spyOn(WalletTransactionsMixin.methods, '__updateParams').mockImplementation()
+        const spyTransactions = jest.spyOn(WalletTransactionsMixin.methods, 'loadTransactions').mockImplementation()
+        createWrapper()
+
+        wrapper.vm.onPerPageChange({ currentPerPage: 10 })
+        expect(spyParams).not.toHaveBeenCalled()
+        expect(spyTransactions).toHaveBeenCalledTimes(1) // gets called once in the created lifecycle hook
+
+        spyParams.mockRestore()
+        spyTransactions.mockRestore()
+      })
+
+      it('should update params and load transactions if currentPerPage has changed', () => {
+        const spyParams = jest.spyOn(WalletTransactionsMixin.methods, '__updateParams').mockImplementation()
+        const spyTransactions = jest.spyOn(WalletTransactionsMixin.methods, 'loadTransactions').mockImplementation()
+        createWrapper()
+
+        wrapper.vm.currentPage = 5
+
+        wrapper.vm.onPerPageChange({ currentPerPage: 20 })
+        expect(spyParams).toHaveBeenCalledTimes(1)
+        expect(spyTransactions).toHaveBeenCalledTimes(2) // gets called once in the created lifecycle hook
+        expect(wrapper.vm.currentPage).toBe(1)
+        spyParams.mockRestore()
+        spyTransactions.mockRestore()
+      })
+    })
   })
 })
diff --git a/src/renderer/components/Wallet/WalletTransactions/WalletTransactions.js b/src/renderer/components/Wallet/WalletTransactions/WalletTransactions.js
index 304e3c3061..6e0fd6d5e9 100644
--- a/src/renderer/components/Wallet/WalletTransactions/WalletTransactions.js
+++ b/src/renderer/components/Wallet/WalletTransactions/WalletTransactions.js
@@ -185,23 +185,20 @@ export default {
     },
 
     onPageChange ({ currentPage }) {
-      if (!currentPage) {
-        return
+      if (currentPage && currentPage !== this.currentPage) {
+        this.currentPage = currentPage
+        this.__updateParams({ page: currentPage })
+        this.loadTransactions()
       }
-
-      this.currentPage = currentPage
-      this.__updateParams({ page: currentPage })
-      this.loadTransactions()
     },
 
     onPerPageChange ({ currentPerPage }) {
-      if (!currentPerPage) {
-        return
+      if (currentPerPage && currentPerPage !== this.transactionTableRowCount) {
+        this.transactionTableRowCount = currentPerPage
+        this.currentPage = 1
+        this.__updateParams({ limit: currentPerPage, page: 1 })
+        this.loadTransactions()
       }
-
-      this.__updateParams({ limit: currentPerPage, page: 1 })
-      this.loadTransactions()
-      this.transactionTableRowCount = currentPerPage
     },
 
     onSortChange ({ source, field, type }) {