(Feature) [Spending Limit] Send funds by fernandomg · Pull Request #1276 · 5afe/safe-react

fernandomg · 2020-08-25T20:38:08Z

This PR closes #693, by:

enabling using spending limit amount when sending funds
tweaking send form to use spending limit or safe's funds
signing tx (despite this worked with the first attempt, I'm still curious how the Safe will be aware of the module's tx)

- this allows to display data hash when there's no info to display - also helps to decode data that has not being decoded by the backend and the frontend can do

…ormation - same layout as the Remove tx modal

several things going on here. - The txs is under the `custom tx` umbrella, instead of `Modify settings` one. - This is a WIP as there's still missing SpendingLimitTxExecute information - also, I'm not ok with how the components are structured

github-actions · 2020-08-25T20:38:23Z

CLA Assistant Lite All Contributors have signed the CLA.

github-actions · 2020-08-25T20:43:14Z

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

Result: ✅ success
Annotations: 1 total

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

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

src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx Line 59 - Object pattern argument should be typed.

^{Report generated by eslint-plus-action}

ghost · 2020-08-25T20:50:21Z

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

…ist-SpendingLimit

…ndFunds-SpendingLimit

lukasschor · 2020-08-26T16:55:23Z

We should display Spending limit modifications in the transaction list with a gear icon and "Spending Limit" instead of "Contract Interactions"

When a spending limit is added / changed / removed, we should say "Modify Spending Limit" instead of "New Spending Limit".

lukasschor · 2020-08-26T16:59:03Z

Let's remove the "tx" in all Spending limit - related notifications

ghost · 2020-08-27T18:33:51Z

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

…dFunds-SpendingLimit # Conflicts: # src/routes/safe/store/actions/transactions/utils/multiSendDecodedDetails.ts

- close modal once the user confirms the tx

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

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

…ndFunds-SpendingLimit

ghost · 2020-09-01T15:09:38Z

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

dasanra · 2020-09-01T15:16:46Z

src/logic/contracts/methodIds.ts

+export const SPENDING_LIMIT_METHOD_TO_ID = {
+  '0xe71bdf41': 'addDelegate',
+  '0xbeaeb388': 'setAllowance',
+  '0x4515641a': 'executeAllowanceTransfer',


Here we are using string instead of const defined in SPENDING_LIMIT_METHOD_NAMES.

I think you are trying to use same pattern used in
src/routes/safe/store/models/types/transactions.d.ts SAFE_METHOD_NAMES -> METHOD_TO_ID

done, thanks for the catch

dasanra · 2020-09-01T16:53:04Z

src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx

                    />
                  </Col>
                </Row>
+                {tokenSpendingLimit && (


This component is getting really big

Should we plan to extract some fragments into smaller child components?

honestly, I didn't want to jump into the refactor hell. So I decided, at the time, to keep everything in the same place.

But anyway, extracted the radioButtons-related row to a SpendingLimitRow component.

Any feedback is welcome.

I think it's fine to keep it as is in this PR but address it in a different one.

Appreciate that some of the new additions were split on a new component.

I'm fine with it as long as a task of tech debt is added to backlog

ghost · 2020-09-02T09:54:40Z

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

- also set missing types

ghost · 2020-09-02T18:52:47Z

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

dasanra · 2020-09-03T15:55:22Z

src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx

                    />
                  </Col>
                </Row>
+                {tokenSpendingLimit && (


Appreciate that some of the new additions were split on a new component.

I'm fine with it as long as a task of tech debt is added to backlog

fernandomg added 8 commits August 25, 2020 09:28

add a way to identify SpendingLimit methods and its data

b41ac8e

add data hash to MultiSendDetails

e10dbb2

- this allows to display data hash when there's no info to display - also helps to decode data that has not being decoded by the backend and the frontend can do

add NewSpendingLimitDetails component to display SL transaction inf…

50d3931

…ormation - same layout as the Remove tx modal

fix: avoid overflow of long strings inside multiSend details

79281f8

refactor: make it easy to identify what Tx is being displayed

4a4d9bf

move NewSpendingLimitDetails under TxDescription umbrella

b984c3d

WIP: first attempt to send funds using spending limit module

5939520

fernandomg self-assigned this Aug 25, 2020

fernandomg added 2 commits August 26, 2020 12:29

Merge branch 'feature/#1039-list-SpendingLimit' into feature/#691-txL…

c466c95

…ist-SpendingLimit

Merge branch 'feature/#691-txList-SpendingLimit' into feature/#693-se…

ba7e8d3

…ndFunds-SpendingLimit

fix: avoid signature request

7bb8bae

fernandomg added 4 commits August 28, 2020 20:23

Merge branch 'feature/#1039-list-SpendingLimit' into feature/#693-sen…

946b01d

…dFunds-SpendingLimit # Conflicts: # src/routes/safe/store/actions/transactions/utils/multiSendDecodedDetails.ts

fixes after merge

36d96ec

fix send funds flow

bdb2c0a

- close modal once the user confirms the tx

remove "tx" from spending limit notifications

43a3150

fernandomg requested review from Agupane, dasanra, mmv08 and nicosampler August 28, 2020 23:50

fernandomg marked this pull request as ready for review August 28, 2020 23:51

Merge branch 'feature/#413-SpendingLimit-in-app' into feature/#693-se…

a6fcf86

…ndFunds-SpendingLimit

Update style initialization

0e52cf5

dasanra suggested changes Sep 2, 2020

View reviewed changes

fernandomg added 2 commits September 2, 2020 15:22

fix: use SPENDING_LIMIT_METHODS_NAME values

809d301

- also set missing types

refactor: extract SpendingLimitRow to its own component

4bfd280

fernandomg requested a review from dasanra September 2, 2020 18:42

dasanra approved these changes Sep 3, 2020

View reviewed changes

fernandomg mentioned this pull request Sep 3, 2020

Refactor SendFunds component #1317

Closed

fernandomg merged commit e3ddb1f into feature/#413-SpendingLimit-in-app Sep 3, 2020

fernandomg deleted the feature/#693-sendFunds-SpendingLimit branch September 3, 2020 18:04

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

Conversation

fernandomg commented Aug 25, 2020

Uh oh!

github-actions bot commented Aug 25, 2020

Uh oh!

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

Uh oh!

ESLint Summary View Full Report

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

Uh oh!

ghost commented Aug 25, 2020

Uh oh!

lukasschor commented Aug 26, 2020

Uh oh!

lukasschor commented Aug 26, 2020

Uh oh!

ghost commented Aug 27, 2020

Uh oh!

ghost commented Aug 29, 2020

Uh oh!

ghost commented Sep 1, 2020

Uh oh!

dasanra Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

fernandomg Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

dasanra Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

fernandomg Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

mmv08 Sep 3, 2020

Choose a reason for hiding this comment

Uh oh!

dasanra Sep 3, 2020

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 2, 2020

Uh oh!

ghost commented Sep 2, 2020

Uh oh!

dasanra Sep 3, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Aug 25, 2020 •

edited

Loading