(Feature) [Spending Limit] List of beneficiaries by fernandomg · Pull Request #1261 · 5afe/safe-react

fernandomg · 2020-08-19T20:58:36Z

This PR closes #1039, by listing the beneficiaries and its spending limits.
Also, closes #692, by allowing the limit removal.
Also, will wait for this PR complete feedback to properly close #690.

~~Still WIP.~~

Refactored the code, for better organization. It's almost all contained in the src/routes/safe/components/Settings/SpendingLimit directory, as a first iteration I think it's ok and helps to find patterns and improvement points easily.

Some tweaks around tx creation: it'll be using multiSend txs only when it's needed (enable SpendingLimit module or adding a new delegate)

…w-SpendingLimit

- marked `isValid` as optional, with default `true` value - marked `initialValue` as optional - migrated to hooks for material-ui styles

- migrated to hooks for material-ui styles - make `label` parametrized and optional, with default value - make `setIsValidAddress` optional

- migrated to hooks for material-ui styles

…w-SpendingLimit # Conflicts: # src/routes/safe/components/Apps/index.tsx

Also: - fixed styles - handled form state while going back and forth between create and review - had to change `Review` Button as submitting wasn't triggered on the first click.

- also refactored `ScanQRWrapper` so it uses the specified icon

…w-SpendingLimit # Conflicts: # package.json # src/routes/safe/components/Apps/index.tsx # yarn.lock

- if the module is not enabled it will be enabled on the first tx - beneficiary is added to the delegate list in the contract - allowance is set Still several things to address, like informing the user about an existing allowance, code cleanup

- create - verify existent and inform the user

…dingLimit state issue

…w-SpendingLimit

…m multiSend txs - Added notifications messages (this is extremely verbose, not sure if I should only use the Settings messages)

ghost · 2020-08-24T15:10:20Z

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

ghost · 2020-08-24T15:40:04Z

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

ghost · 2020-08-24T15:48:09Z

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

…ist-SpendingLimit

ghost · 2020-08-24T18:31:23Z

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

lukasschor · 2020-08-24T19:00:29Z

Abbreviated addresses should have shortened with 4 leading and trailing characters, if possible. So this should be displayed as 0x83eC...0769.

fernandomg · 2020-08-24T19:39:23Z

Abbreviated addresses should have shortened with 4 leading and trailing characters, if possible. So this should be displayed as 0x83eC...0769.

@lukasschor, ah. I just tried to be aligned with the owners' table. Starting with the long-shortened view and shrinking to 4 chars when the viewport is smaller (also hides the token icons).

If it's not valid, then I'll definitively remove those behaviors.

lukasschor · 2020-08-24T19:43:27Z

We should generally move to standardize the addresses and either show full length or abbreviated with 4 chars (which should be the default, full address should only be an exception). We have started doing so here: #1174
It's not crucial to have this implemented everywhere immediately, but at least for new features, we should already take this into account.

ghost · 2020-08-25T12:41:28Z

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

lukasschor · 2020-08-25T15:18:35Z

We should not display "UNKNOWN" for addresses where there is no address book entry.

See this ticket for reference: #1068

UPDATE: It seems like this is already the case, right? At least when I tested it did not show UNKNOWN.

dasanra · 2020-08-26T13:31:39Z

src/utils/constants.ts

 export const TIMEOUT = process.env.NODE_ENV === 'test' ? 1500 : 5000
 export const ETHERSCAN_API_KEY = process.env.REACT_APP_ETHERSCAN_API_KEY
+export const SPENDING_LIMIT_MODULE_ADDRESS =
+  process.env.REACT_APP_SPENDING_LIMIT_MODULE_ADDRESS || '0x9e9Bf12b5a66c0f0A7435835e0365477E121B110'


This address should be named. I understand this is something will be removed later

exactly, this is there to make the feature work while on dev, as the variable is not set in the CI builds

I created a task in [Backlog] - Stability saying that this should be reviewed before deployment

ghost · 2020-08-26T13:43:38Z

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

ghost · 2020-08-26T15:38:11Z

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

…ist-SpendingLimit # Conflicts: # package.json # src/logic/safe/store/actions/fetchSafe.ts # src/routes/safe/components/Layout/index.tsx # yarn.lock

github-actions · 2020-08-28T23:55:00Z

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	0	0
Ignored	1	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

ghost · 2020-08-29T00:05:00Z

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

fernandomg added 30 commits July 30, 2020 16:47

Merge branch 'development' into feature/#413-SpendingLimit-in-app

9b3c973

fix: Card component requires className instead of styles

5e24cfc

update safe-react-components

f669a3a

change Spending Limit icon from allowances to fuelIndicator

2583e57

Merge branch 'feature/#413-SpendingLimit-in-app' into feature/#690-ne…

5fca120

…w-SpendingLimit

add types to TokenSelectField component

df6547a

- marked `isValid` as optional, with default `true` value - marked `initialValue` as optional - migrated to hooks for material-ui styles

refactor AddressBookInput

7616e8f

- migrated to hooks for material-ui styles - make `label` parametrized and optional, with default value - make `setIsValidAddress` optional

add types to GnoModal component

32e5d50

- migrated to hooks for material-ui styles

update safe-react-components

d0b2c7d

WIP: New Spending Limit form

cea0817

Merge branch 'feature/#413-SpendingLimit-in-app' into feature/#690-ne…

b603512

…w-SpendingLimit # Conflicts: # src/routes/safe/components/Apps/index.tsx

fix compatibility with new safe-react-components version

83a0478

update safe-react-components version

66c1e85

fix styles for TokenSelectField's placeholder

debf1ce

WIP: add review screen

7c109f0

Also: - fixed styles - handled form state while going back and forth between create and review - had to change `Review` Button as submitting wasn't triggered on the first click.

WIP: fix styles/layout

b854053

- also refactored `ScanQRWrapper` so it uses the specified icon

disable Spending Limit button for non-owners

3c9543d

Merge branch 'feature/#413-SpendingLimit-in-app' into feature/#690-ne…

ec4408e

…w-SpendingLimit # Conflicts: # package.json # src/routes/safe/components/Apps/index.tsx # yarn.lock

WIP: Add new spending limit

a3b9ee2

- if the module is not enabled it will be enabled on the first tx - beneficiary is added to the delegate list in the contract - allowance is set Still several things to address, like informing the user about an existing allowance, code cleanup

WIP: verify module is enabled and works

b5aadc8

WIP: complete New Spending Limit flow

1fea250

- create - verify existent and inform the user

WIP: properly handle tokens' decimals also fix typos and existentSpen…

4bd62a6

…dingLimit state issue

refactor: split code into components

53f9e4a

Merge branch 'feature/#413-SpendingLimit-in-app' into feature/#690-ne…

8307abd

…w-SpendingLimit

fix reference after merge

9be7cd7

add REACT_APP_SPENDING_LIMIT_MODULE_ADDRESS as .env variable

b465670

set return type for array map

9182621

add return value

e0a62ae

replace map with for..of loop

1000ffa

move MODULE_ADDRESS constant to .env.example

b81938a

fernandomg added 2 commits August 24, 2020 11:56

refactor: move away from Apps' sendTransactions method. Using custo…

bdee939

…m multiSend txs - Added notifications messages (this is extremely verbose, not sure if I should only use the Settings messages)

refactor: undo previousResetTime as a memoized value

d5bec40

fernandomg added 3 commits August 24, 2020 12:26

feature: use multiSend txs only when needed

b7f6083

fix: add support for ETH allowance modification

2fab251

fix: support non existent spendingLimits

a35be6d

Merge branch 'feature/#413-SpendingLimit-in-app' into feature/#1039-l…

b0890b6

…ist-SpendingLimit

fix: set fixed length for addresses

5550ca9

dasanra suggested changes Aug 26, 2020

View reviewed changes

Update React import

e8c8c34

fix: avoid UNKNOWN as contact name

073a065

fernandomg added 2 commits August 28, 2020 19:58

Merge branch 'feature/#413-SpendingLimit-in-app' into feature/#1039-l…

a634f18

…ist-SpendingLimit # Conflicts: # package.json # src/logic/safe/store/actions/fetchSafe.ts # src/routes/safe/components/Layout/index.tsx # yarn.lock

fixes after merge

813fa68

fernandomg requested a review from dasanra August 28, 2020 23:52

dasanra approved these changes Sep 1, 2020

View reviewed changes

fernandomg merged commit 1387ed4 into feature/#413-SpendingLimit-in-app Sep 1, 2020

fernandomg deleted the feature/#1039-list-SpendingLimit branch September 1, 2020 14:51

github-actions bot locked and limited conversation to collaborators Sep 1, 2020

Conversation

fernandomg commented Aug 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Aug 24, 2020

Uh oh!

ghost commented Aug 24, 2020

Uh oh!

ghost commented Aug 24, 2020

Uh oh!

ghost commented Aug 24, 2020

Uh oh!

lukasschor commented Aug 24, 2020

Uh oh!

fernandomg commented Aug 24, 2020

Uh oh!

lukasschor commented Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Aug 25, 2020

Uh oh!

lukasschor commented Aug 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dasanra Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

fernandomg Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

dasanra Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 26, 2020

Uh oh!

ghost commented Aug 26, 2020

Uh oh!

github-actions bot commented Aug 28, 2020

ESLint Summary View Full Report

Uh oh!

ghost commented Aug 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fernandomg commented Aug 19, 2020 •

edited

Loading

lukasschor commented Aug 24, 2020 •

edited

Loading

lukasschor commented Aug 25, 2020 •

edited

Loading