Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

(Fix) Duplicated Owner Addresses on Safe Creation#1666

Merged
dasanra merged 8 commits intodevelopmentfrom
fix/1661-duplicated-owner-addresses
Dec 9, 2020
Merged

(Fix) Duplicated Owner Addresses on Safe Creation#1666
dasanra merged 8 commits intodevelopmentfrom
fix/1661-duplicated-owner-addresses

Conversation

@fernandomg
Copy link
Contributor

This PR closes #1661, by:

  • adding a form-level validation function to properly check for duplicates
    address-already-introduced

Also fixes #1183, by:

  • fixing row deletion function
    remove-owner-row

- also fixed `calculateValuesAfterRemoving` function that removed an owner's row by clicking on the trash icon
@fernandomg fernandomg self-assigned this Nov 27, 2020
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Nov 27, 2020

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 3 0
Ignored 1 N/A
  • Result: ✅ success

  • Annotations: 3 total


[warning] @typescript-eslint/explicit-module-boundary-types

Require explicit return and argument types on exported functions' and classes' public class methods


Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Nov 27, 2020

Travis automatic deployment:
https://pr1666--safereact.review.gnosisdev.com/rinkeby/app

@ghost
Copy link

ghost commented Nov 27, 2020

Travis automatic deployment:
https://pr1666--safereact.review.gnosisdev.com/volta/app

@francovenica
Copy link
Contributor

This one looks good

A new ticket was created for the same issue in Address book and Settings (Adding/replacing owners)

@fernandomg
Copy link
Contributor Author

@Agupane, @nicosampler I just added a couple of tests for the 'remove-owner' related function.

I was thinking about splitting this into two separate PRs and add tests for the refactored function. But given it's already checked/tested, I did it here 😬

@nicosampler
Copy link
Contributor

@Agupane, @nicosampler I just added a couple of tests for the 'remove-owner' related function.
LGTM

@francovenica
Copy link
Contributor

I've never acknowledged that this ticket also fixes 1183.

@ghost
Copy link

ghost commented Dec 4, 2020

Travis automatic deployment:
https://pr1666--safereact.review.gnosisdev.com/rinkeby/app

@ghost
Copy link

ghost commented Dec 4, 2020

Travis automatic deployment:
https://pr1666--safereact.review.gnosisdev.com/volta/app

@ghost
Copy link

ghost commented Dec 4, 2020

Travis automatic deployment:
https://pr1666--safereact.review.gnosisdev.com/rinkeby/app

@ghost
Copy link

ghost commented Dec 4, 2020

Travis automatic deployment:
https://pr1666--safereact.review.gnosisdev.com/volta/app

@dasanra dasanra merged commit f161875 into development Dec 9, 2020
@dasanra dasanra deleted the fix/1661-duplicated-owner-addresses branch December 9, 2020 11:56
@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Safe creation - Duplicated owners addresses Create safe: a mess in the safe's owners list if the owner from the middle of the list is deleted

5 participants