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

Epic: Address Book v2#2280

Merged
katspaugh merged 21 commits into
developmentfrom
address-book-v2
Jun 7, 2021
Merged

Epic: Address Book v2#2280
katspaugh merged 21 commits into
developmentfrom
address-book-v2

Conversation

@katspaugh

@katspaugh katspaugh commented May 10, 2021

Copy link
Copy Markdown
Contributor

An epic PR to track the progress of the Address Book v2 implementation.

The QA will happen on this branch too.

⚠️ Please DO NOT update this branch from development via GitHub ⚠️

What to test

  • Create safe – safe name is stored in the address book
  • Load safe - safe name is stored in the address book
  • Add an owner - check name in address book
  • Replace an owner - check name in address book
  • Edit safe name in settings
  • Edit owner name in settings
  • Directly create an address book entry
  • Directly edit an address book entry
  • Check that address book name is shown on transaction details
  • Name stored in address book it's used over known address name from client-gateway
  • Deleting a safe using settings button don't delete the address from the address book
  • Name shown in assets "send" is updated
  • Name shown in assets "receive" is updated
  • Name shown in transaction details for owners is correct
  • Name shown in address book "send" is correct
  • Names shown in settings > spending limits are correct

* Add import/export buttons to addressBook

* Change textSize to icons
@katspaugh katspaugh added the Epic label May 10, 2021
@katspaugh katspaugh added this to the Address Book v2 milestone May 10, 2021
@github-actions

Copy link
Copy Markdown

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions

github-actions Bot commented May 10, 2021

Copy link
Copy Markdown

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 0 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

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

matextrem and others added 2 commits May 17, 2021 16:55
* Add export feature

* Improve Address book export modal

* Use styled-components instead of material-ui styles

* Add comment

* Improve filename + set loading state false when error
* Remove UNKNOWN from owner list

* Fetch Safe Name from address book

* Update Safe name in address book when updating from settings > safe details

* Fix lint issue
Daniel Sanchez and others added 2 commits May 18, 2021 17:56
* Added import modal and import functionality

* Added modal refactor

* Added entry validations

* Added message after loading file

* Reset values on modal close

* Minor formal changes

* Entry list validations on separate functions

* Minor changes

* Text corrections

* Check if isAddress instead of checksum

* add styles on import

* fix sidebar label for ADDRESS BOOK (with a space)

* fix modal title in lowecase

* replace icons on AB list

* Clear upload feedback on modal close

* add styles to drag and drop area

* add link to Help Center article

* remove unused styles

* change color when is an error

* change error color

* fix error color and align file size

* Modal refactor

* fix button styles on modal

* fix long file names

Co-authored-by: Agustín Longoni <agustin.longoni@altoros.com>
@katspaugh katspaugh modified the milestones: Address Book v2, 3.7.0 May 25, 2021
* WIP: add `chainId` with default value to `AddressBookEntry`

* WIP: first approach to the new AB structure and data persistence in localStorage

* migrate AB

* migrate AB actions

* migrate AB reducers

* update AB selectors

* update AB middlewares

* update AB utils

* update AB model file

* migrate AB usage to the new structure

* change AB notification key name

* fixes after rebase

* add `addressBookBatchLoad` action

* create a selector for owners with AB data

`safeOwnersWithAddressBookDataSelector`

* migrate safe->owners structure

* make `name` and `address` mandatory for `makeAddressBookEntry`

* filter out invalid AB names before storing them

* review Load & Open flow for owner's names

* update owners' reducers and actions

- `editSafeOwner` no longer necessary as there's no name change in the owners' list

* create AB map-like selector

chainId -> address -> name

* auto-complete owner's name if it exist in the AB

* update replace owner modal and remove REPLACE_SAFE_OWNER

- no longer optimistically update the owner's list if threshold === 1, as it behaves erratically with the constant update from data provided by the services
- `REPLACE_SAFE_OWNER` no longer necessary

* update remove owner modal and remove REMOVE_SAFE_OWNER

- no longer optimistically update the owner's list if threshold === 1, as it behaves erratically with the constant update from data provided by the services
- `REMOVE_SAFE_OWNER` no longer necessary

* update add owner modal and remove ADD_SAFE_OWNER

- no longer optimistically update the owner's list if tx is signed, as it behaves erratically with the constant update from data provided by the services
- `ADD_SAFE_OWNER` no longer necessary

* update yarn.lock after rebase

* fix AddressBookMap type

* create a migration function for safe's names

* create selectors that require safe's name

* remove `name` from the safe model and update related code

* JSON.parse if JSON.parsed string returned a string instead of a JSON

* remove commented lines

* rename BATCH_LOAD to SAFE_LOAD

* refactor AddressBookMap to `chainId`->`address`->`AddressBookEntry`

* make `chainId` optional for `getNameFromAddressBookSelector`

* use `addressBookMapSelector` for a more direct AddressBook entry name lookup

* remove `loadedSafeViaUrl` flag, and rely on address book entry instead

* replace `addAddressBookEntry` action with `addressBookAddOrUpdate`
* add back `loadedViaUrl` flag

* implicitly set `loadedViaUrl` to `false` when adding a safe via form or setting its name from Settings

* exclude safes whose `loadedViaUrl` flag is `true`

* hide "Remove Safe" button for safes `loadedViaUrl`

* add `loadedViaUrl` flag to mocked safe
@github-actions

github-actions Bot commented May 27, 2021

Copy link
Copy Markdown

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 8 0
Ignored 9 N/A
  • Result: ✅ success

  • Annotations: 8 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

katspaugh and others added 2 commits May 27, 2021 15:52
* Refactor AB migration

* Fix AB v2 and safes migration

* Don't migrate if already migrated

* Restore removeFromStorage
* Add loading state when click on download button

* Expand clickable area for link
Daniel Sanchez and others added 3 commits June 1, 2021 12:17
* Use ADDRESS_BOOK_DEFAULT_NAME instead of hard coded string

* Use address book name in Send Collectible review step

* Show name if known in contract interaction review

* Use string comparison util
Co-authored-by: katspaugh <katspaugh@users.noreply.github.com>
@tschubotz

Copy link
Copy Markdown
Contributor

Screenshot 2021-06-02 at 10 37 09

This link exists on both the export and import modals.

I created a brief help center article. Could this be linked there please? https://help.gnosis-safe.io/en/articles/5299068-address-book-export-and-import

Also, how about re-wording this to Learn about address book import and export? Would be less text.

@liliya-soroka

liliya-soroka commented Jun 2, 2021

Copy link
Copy Markdown
Member

Isssues related to addressbook
#2378
#2381 - closed as duplication
#2383
#2384
#2385
#2386 - May be critical for the addressbook release

@katspaugh

katspaugh commented Jun 2, 2021

Copy link
Copy Markdown
Contributor Author

Thanks @tschubotz, I'm on it.
Edit: fixed in #2380.

@liliya-soroka

Copy link
Copy Markdown
Member

@katspaugh , will "Ability to check/uncheck imported addresses in case of a conflict with existing addresses" be implemented in the next addressbook version ? Do we need a ticket for it ?

@katspaugh

Copy link
Copy Markdown
Contributor Author

@liliya-soroka we decided to skip it completely.

fernandomg and others added 4 commits June 2, 2021 15:55
…safe (#2374)

* avoid deleting addressBook entry when deleting a safe

* avoid deleting addressBook entry when deleting a safe
* Verify chainId column and called import reducer

* Added dispatch for addressbook import

* Fixed store reducer and notification

* Validate all entries on csv data validation

* Using utils to verify imported addresses

* Add props typing to ImportEntriesModal

* Convert chainId to number
* Allow to load new safe with empty owner names

* Avoid adding owners to addressbook if name is empty

* Remove unnecessary initialization
@francovenica

Copy link
Copy Markdown
Contributor

Here's an old issue that could be related to the AB: #2323

fernandomg and others added 2 commits June 3, 2021 16:49
* keep safes with no name for the list of safes

* prevent storing only those safes whose `loadedViaUrl` flag is true

* update AB before Safe store

* default safe name to empty string if it is not available
@francovenica

Copy link
Copy Markdown
Contributor

@francovenica

Copy link
Copy Markdown
Contributor

Didn't find anything that may stop the deployment. We will see how it goes in stg on Monday

@katspaugh katspaugh mentioned this pull request Jun 7, 2021
@katspaugh katspaugh merged commit 2d532f3 into development Jun 7, 2021
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants