From 1cf860c03e41be1cc24469aba5046e9851f04ae8 Mon Sep 17 00:00:00 2001
From: decentraliser <decentraliser@gmail.com>
Date: Sat, 23 May 2020 17:14:40 +0800
Subject: [PATCH] cosignatory add / remove behaviour, fixes #384

---
 .../MultisigCosignatoriesDisplay.spec.ts      | 175 ++++++++++++++++++
 .../MultisigCosignatoriesDisplay.vue          |  26 ++-
 .../MultisigCosignatoriesDisplayTs.ts         | 113 +++++------
 3 files changed, 259 insertions(+), 55 deletions(-)
 create mode 100644 __tests__/components/MultisigCosignatoriesDisplay.spec.ts

diff --git a/__tests__/components/MultisigCosignatoriesDisplay.spec.ts b/__tests__/components/MultisigCosignatoriesDisplay.spec.ts
new file mode 100644
index 000000000..2aea86339
--- /dev/null
+++ b/__tests__/components/MultisigCosignatoriesDisplay.spec.ts
@@ -0,0 +1,175 @@
+/*
+ * Copyright 2020 NEM Foundation (https://nem.io)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and limitations under the License.
+ *
+ */
+import { shallowMount } from '@vue/test-utils'
+// @ts-ignore
+import MultisigCosignatoriesDisplay from '@/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplay'
+import { MultisigAccountInfo, NetworkType, PublicAccount } from 'symbol-sdk'
+import i18n from '@/language/index'
+
+const networkType = NetworkType.MAIN_NET
+const account1 = PublicAccount.createFromPublicKey(
+  'B694186EE4AB0558CA4AFCFDD43B42114AE71094F5A1FC4A913FE9971CACD21D',
+  networkType,
+)
+const account2 = PublicAccount.createFromPublicKey(
+  'CF893FFCC47C33E7F68AB1DB56365C156B0736824A0C1E273F9E00B8DF8F01EB',
+  networkType,
+)
+const account3 = PublicAccount.createFromPublicKey(
+  'DAB1C38C3E1642494FCCB33138B95E81867B5FB59FC4277A1D53761C8B9F6D14',
+  networkType,
+)
+const account4 = PublicAccount.createFromPublicKey(
+  '1674016C27FE2C2EB5DFA73996FA54A183B38AED0AA64F756A3918BAF08E061B',
+  networkType,
+)
+const multisigInfo = new MultisigAccountInfo(account1, 1, 1, [account2, account3], [])
+
+describe('MultisigCosignatoriesDisplay', () => {
+  test('Getters should return correct values when no modifications', () => {
+    const wrapper = shallowMount(MultisigCosignatoriesDisplay, {
+      i18n,
+      propsData: {
+        multisig: multisigInfo,
+        modifiable: true,
+        cosignatoryModifications: {},
+      },
+    })
+
+    const component = wrapper.vm as MultisigCosignatoriesDisplay
+
+    expect(component.addModifications).toEqual([])
+    expect(component.removeModifications).toEqual([])
+    expect(component.cosignatories).toEqual([
+      { publicKey: account2.publicKey, address: account2.address.pretty() },
+      { publicKey: account3.publicKey, address: account3.address.pretty() },
+    ])
+
+    wrapper.destroy()
+  })
+
+  test('Getters should return correct values when there are modifications', () => {
+    const wrapper = shallowMount(MultisigCosignatoriesDisplay, {
+      i18n,
+      propsData: {
+        multisig: multisigInfo,
+        modifiable: true,
+        cosignatoryModifications: {
+          [account4.publicKey]: { cosignatory: account4, addOrRemove: 'add' },
+          [account3.publicKey]: { cosignatory: account3, addOrRemove: 'remove' },
+        },
+      },
+    })
+
+    const component = wrapper.vm as MultisigCosignatoriesDisplay
+
+    expect(component.addModifications).toEqual([{ publicKey: account4.publicKey, address: account4.address.pretty() }])
+    expect(component.removeModifications).toEqual([
+      { publicKey: account3.publicKey, address: account3.address.pretty() },
+    ])
+    expect(component.cosignatories).toEqual([{ publicKey: account2.publicKey, address: account2.address.pretty() }])
+
+    wrapper.destroy()
+  })
+
+  test('Should dispatch an error when adding a cosigner that is already one', () => {
+    const mockStoreDispatch = jest.fn()
+
+    const wrapper = shallowMount(MultisigCosignatoriesDisplay, {
+      i18n,
+      propsData: {
+        multisig: multisigInfo,
+        modifiable: true,
+        cosignatoryModifications: {},
+      },
+      mocks: {
+        $store: {
+          dispatch: mockStoreDispatch,
+        },
+      },
+    })
+
+    const component = wrapper.vm as MultisigCosignatoriesDisplay
+
+    component.onAddCosignatory(account2)
+
+    expect(mockStoreDispatch).toHaveBeenCalledWith('notification/ADD_WARNING', 'warning_already_a_cosignatory')
+    wrapper.destroy()
+  })
+
+  test('Should dispatch an error when adding a cosigner has already been added', () => {
+    const mockStoreDispatch = jest.fn()
+
+    const wrapper = shallowMount(MultisigCosignatoriesDisplay, {
+      i18n,
+      propsData: {
+        multisig: multisigInfo,
+        modifiable: true,
+        cosignatoryModifications: {
+          [account4.publicKey]: { cosignatory: account4, addOrRemove: 'add' },
+        },
+      },
+      mocks: {
+        $store: {
+          dispatch: mockStoreDispatch,
+        },
+      },
+    })
+
+    const component = wrapper.vm as MultisigCosignatoriesDisplay
+
+    component.onAddCosignatory(account4)
+
+    expect(mockStoreDispatch).toHaveBeenCalledWith('notification/ADD_WARNING', 'warning_already_a_cosignatory')
+    wrapper.destroy()
+  })
+
+  test('Should emit when adding a cosigner', () => {
+    const wrapper = shallowMount(MultisigCosignatoriesDisplay, {
+      i18n,
+      propsData: {
+        multisig: multisigInfo,
+        modifiable: true,
+        cosignatoryModifications: {},
+      },
+    })
+
+    const component = wrapper.vm as MultisigCosignatoriesDisplay
+
+    component.onAddCosignatory(account4)
+    expect(wrapper.emitted('add')).toBeTruthy()
+    expect(wrapper.emitted().add[0]).toEqual([account4])
+    wrapper.destroy()
+  })
+
+  test('Should emit when removing a cosigner', () => {
+    const wrapper = shallowMount(MultisigCosignatoriesDisplay, {
+      i18n,
+      propsData: {
+        multisig: multisigInfo,
+        modifiable: true,
+        cosignatoryModifications: {},
+      },
+    })
+
+    const component = wrapper.vm as MultisigCosignatoriesDisplay
+
+    component.onRemoveCosignatory(account2)
+    expect(wrapper.emitted('remove')).toBeTruthy()
+    expect(wrapper.emitted().remove[0]).toEqual([account2])
+    wrapper.destroy()
+  })
+})
diff --git a/src/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplay.vue b/src/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplay.vue
index b16b14aa4..e68d039ad 100644
--- a/src/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplay.vue
+++ b/src/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplay.vue
@@ -31,7 +31,13 @@
             <div class="cosignatory-address-container">
               <span>{{ address }}</span>
             </div>
-            <span v-if="modifiable" class="delete-icon" @click="onRemoveCosignatory(publicKey)" />
+            <Icon
+              v-if="modifiable"
+              type="md-trash"
+              size="21"
+              class="icon-button"
+              @click="onRemoveCosignatory(publicKey)"
+            />
             <span v-else>&nbsp;</span>
           </div>
         </template>
@@ -58,7 +64,7 @@
             <div class="cosignatory-address-container">
               <span class="cosignatory-removed">{{ address }}</span>
             </div>
-            <span class="delete-icon" @click="onUndoRemoveModification(publicKey)" />
+            <Icon type="ios-undo" size="21" class="icon-button" @click="onUndoModification(publicKey)" />
           </div>
         </template>
       </FormRow>
@@ -84,7 +90,7 @@
             <div class="cosignatory-address-container">
               <span>{{ address }}</span>
             </div>
-            <span class="delete-icon" @click="onRemoveModification(publicKey)" />
+            <Icon type="md-trash" size="21" class="icon-button" @click="onUndoModification(publicKey)" />
           </div>
         </template>
       </FormRow>
@@ -102,3 +108,17 @@ import { MultisigCosignatoriesDisplayTs } from './MultisigCosignatoriesDisplayTs
 
 export default class MultisigCosignatoriesDisplay extends MultisigCosignatoriesDisplayTs {}
 </script>
+
+<style lang="less" scoped>
+@import '../../views/resources/css/variables.less';
+
+.icon-button {
+  cursor: pointer;
+  color: @blackLight;
+  justify-self: left;
+}
+
+.icon-button:hover {
+  color: @red;
+}
+</style>
diff --git a/src/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplayTs.ts b/src/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplayTs.ts
index 80e483dee..de5abd297 100644
--- a/src/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplayTs.ts
+++ b/src/components/MultisigCosignatoriesDisplay/MultisigCosignatoriesDisplayTs.ts
@@ -15,102 +15,111 @@
  */
 // external dependencies
 import { Component, Prop, Vue } from 'vue-property-decorator'
-import { mapGetters } from 'vuex'
-import { Address, MultisigAccountInfo, NetworkType, PublicAccount } from 'symbol-sdk'
+import { MultisigAccountInfo, PublicAccount } from 'symbol-sdk'
+
 // child components
-import { ValidationProvider } from 'vee-validate'
 // @ts-ignore
 import FormRow from '@/components/FormRow/FormRow.vue'
 // @ts-ignore
 import AddCosignatoryInput from '@/components/AddCosignatoryInput/AddCosignatoryInput.vue'
 
 // custom types
+type AddOrRemove = 'add' | 'remove'
+
 interface Modification {
   cosignatory: PublicAccount
-  addOrRemove: 'add' | 'remove'
+  addOrRemove: AddOrRemove
 }
 
+type Cosignatories = { publicKey: string; address: string }[]
+
 @Component({
   components: {
-    ValidationProvider,
     FormRow,
     AddCosignatoryInput,
   },
-  computed: {
-    ...mapGetters({
-      networkType: 'network/networkType',
-    }),
-  },
 })
 export class MultisigCosignatoriesDisplayTs extends Vue {
   @Prop({ default: null }) multisig: MultisigAccountInfo
-
   @Prop({ default: false }) modifiable: boolean
-
   @Prop({ default: {} }) cosignatoryModifications: Record<string, Modification>
 
-  private networkType: NetworkType
+  /**
+   * Whether the add cosignatory form input is visible
+   */
+  protected isAddingCosignatory = false
 
-  public isAddingCosignatory: boolean = false
-  public addedActors: { publicKey: string; address: string }[] = []
-  public removedActors: { publicKey: string; address: string }[] = []
-
-  get cosignatories(): { publicKey: string; address: string }[] {
-    if (!this.multisig) return []
+  /**
+   * Cosignatories to add
+   * @type {Cosignatories}
+   */
+  protected get addModifications(): Cosignatories {
+    return this.getFilteredModifications('add')
+  }
 
-    return Object.values(this.multisig.cosignatories)
-      .filter((c) => {
-        return undefined === this.removedActors.find((m) => m.publicKey === c.publicKey)
-      })
-      .map((cosignatory) => ({
-        publicKey: cosignatory.publicKey,
-        address: cosignatory.address.pretty(),
-      }))
+  /**
+   * Cosignatories to remove
+   * @type {Cosignatories}
+   */
+  protected get removeModifications(): Cosignatories {
+    return this.getFilteredModifications('remove')
   }
 
-  get addModifications(): { publicKey: string; address: string }[] {
+  /**
+   * Internal helper to get filtered cosignatory modifications
+   * @param {AddOrRemove} addOrRemoveFilter
+   * @returns {Cosignatories}
+   */
+  private getFilteredModifications(addOrRemoveFilter: AddOrRemove): Cosignatories {
     return Object.values(this.cosignatoryModifications)
-      .filter(({ addOrRemove }) => addOrRemove === 'add')
+      .filter(({ addOrRemove }) => addOrRemove === addOrRemoveFilter)
       .map(({ cosignatory }) => ({
         publicKey: cosignatory.publicKey,
         address: cosignatory.address.pretty(),
       }))
   }
 
-  get removeModifications(): { publicKey: string; address: string }[] {
-    return Object.values(this.cosignatoryModifications)
-      .filter(({ addOrRemove }) => addOrRemove === 'remove')
-      .map(({ cosignatory }) => ({
-        publicKey: cosignatory.publicKey,
-        address: cosignatory.address.pretty(),
-      }))
+  /**
+   * The multisig account cosignatories after modifications
+   * @type {{ publicKey: string; address: string }[]}
+   */
+  protected get cosignatories(): { publicKey: string; address: string }[] {
+    if (!this.multisig) return []
+
+    return this.multisig.cosignatories
+      .filter(({ publicKey }) => !this.cosignatoryModifications[publicKey])
+      .map(({ publicKey, address }) => ({ publicKey, address: address.pretty() }))
   }
 
-  public onAddCosignatory(publicAccount: PublicAccount) {
-    const exists = this.cosignatories.find((a) => a.publicKey === publicAccount.publicKey)
-    const existsMod = this.addedActors.find((a) => a.publicKey === publicAccount.publicKey)
-    if (exists !== undefined || existsMod !== undefined) {
+  /**
+   * Hook called when a cosignatory is added
+   * @param {PublicAccount} publicAccount
+   */
+  protected onAddCosignatory(publicAccount: PublicAccount): void {
+    const { publicKey } = publicAccount
+    const isCosignatory = this.cosignatories.find((a) => a.publicKey === publicKey)
+
+    if (isCosignatory || this.cosignatoryModifications[publicKey]) {
       this.$store.dispatch('notification/ADD_WARNING', 'warning_already_a_cosignatory')
       return
     }
 
     this.$emit('add', publicAccount)
-    this.isAddingCosignatory = false
   }
 
-  public onRemoveCosignatory(publicKey: string) {
-    this.removedActors.push({
-      publicKey,
-      address: Address.createFromPublicKey(publicKey, this.networkType).pretty(),
-    })
+  /**
+   * Hook called when a cosignatory is removed
+   * @param {string} publicKey
+   */
+  protected onRemoveCosignatory(publicKey: string): void {
     this.$emit('remove', publicKey)
   }
 
-  public onRemoveModification(publicKey: string) {
-    this.$emit('undo', publicKey)
-  }
-
-  public onUndoRemoveModification(publicKey: string) {
-    this.$emit('undo', publicKey)
+  /**
+   * Hook called when a cosignatory modification is undone
+   * @param {string} thePublicKey
+   */
+  protected onUndoModification(thePublicKey: string): void {
+    this.$emit('undo', thePublicKey)
   }
 }