feat: pending recovery widget #2772

iamacook · 2023-11-08T14:00:58Z

What it solves

Resolves #2754

How this PR fixes it

A pending recovery widget has been added to the top of the dashboard according to the designs:

When a pending recovery has not expired and is invalid, a countdown is shown.
When a pending recovery has not expired and is valid, a warning is shown. (Note: this will be adjusted when [Recovery] Proposal request modal/widget #2758 is implemented)

Full test coverage has been included for UI elements and associated logic.

How to test it

Open a Safe that has a pending recovery attempt, e.g. gor:0xAecDFD3A19f777F0c03e6bf99AAfB59937d6467b and observe the new banner.

Screenshots

Checklist

I've tested the branch on mobile 📱
I've documented how it affects the analytics (if at all) 📊
I've written a unit/e2e test for it (if applicable) 🧑‍💻

github-actions · 2023-11-08T14:01:13Z

Branch preview

✅ Deploy successful!

https://pending_recovery_widget--walletweb.review-wallet-web.5afe.dev

github-actions · 2023-11-08T14:05:25Z

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

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

github-actions · 2023-11-08T14:12:53Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	75.28% (+0.05% 🔼)	10279/13654
🔴	Branches	50.05% (+0.2% 🔼)	2099/4194
🔴	Functions	57.94% (+0.17% 🔼)	1533/2646
🟡	Lines	76.85% (+0.08% 🔼)	9301/12103

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / index.tsx	89.36%	85.71%	72.73%	97.56%
🟢	`...` / useBlockTimestamp.ts	100%	75%	100%	100%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / transaction-calldata.ts	95.65% (-4.35% 🔻)	100%	100%	100%
🔴	`...` / index.tsx	53.13% (-1.42% 🔻)	0%	0%	58.62% (-1.38% 🔻)

Test suite run success

1150 tests passing in 161 suites.

Report generated by 🧪jest coverage report action from f941c8b

github-actions · 2023-11-14T17:33:42Z

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

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

usame-algan

On mobile this widget can look a bit cramped but I also saw that there are no mobile designs yet so possibly something we can fix later.

public/images/common/recovery-pending.svg

src/components/dashboard/RecoveryInProgress/index.test.tsx

src/components/dashboard/RecoveryInProgress/index.tsx

usame-algan · 2023-11-20T09:17:28Z

src/components/dashboard/RecoveryInProgress/index.tsx

+    return null
+  }
+
+  const nonExpiredTxs = recovery


Somehow these queue items contain a lot more properties than what the type says e.g. functions like getBlock etc. but they are also not visible in the Redux store, only in the console.

Redux only shows serializable data, hence it not showing in the DevTools. As we are not persisting the data, nor relying on these "hidden" properties, I think we can ignore these.

src/components/dashboard/RecoveryInProgress/index.tsx

usame-algan · 2023-11-20T09:44:13Z

src/hooks/useBlockTimestamp.ts

+
+import { useWeb3ReadOnly } from './wallets/web3'
+
+const INTERVAL = 1_000


The Countdown only has minute precision. Why do we need to update the block timestamp every second which rerenders the RecoveryInProgress component? Would it not suffice to do it every 60 seconds?

The block timestamp is in seconds and this hook is not necessarily recovery-specific.

I understand but wherever we use this hook it would result in rerenders/side effects and possibly expensive computations every second. For the dashboard widget it is only necessary to be updated every 60s as far as I understand. We could pass in the interval as a prop to solve this instead of hard-coding it here.

I've implemented your suggestion in f941c8b.

usame-algan · 2023-11-20T09:49:59Z

src/hooks/useBlockTimestamp.ts

+  useEffect(() => {
+    if (!block) {
+      return
+    }
+
+    setTimestamp(block.timestamp)
+
+    const timeout = setInterval(() => {
+      setTimestamp((prev) => {
+        return prev ? prev + 1 : block.timestamp
+      })
+    }, INTERVAL)
+
+    return () => {
+      clearInterval(timeout)
+    }
+  }, [block])


I am not sure I understand this logic. Is it that we initially want to fetch the block timestamp once but then increase it by 1 every second? Is it possible to move this logic inside the useAsync block and get rid of the timestamp state?

Yes, by increasing the timestamp locally we don't need to fetch the it again. We cannot move it inside useAsync due to the cleanup.

usame-algan · 2023-11-20T09:56:20Z

src/components/dashboard/RecoveryInProgress/index.tsx

+import { FEATURES } from '@/utils/chains'
+
+export function RecoveryInProgress(): ReactElement | null {
+  const blockTimestamp = useBlockTimestamp()


I noticed that it can take a few seconds for the blockTimestamp to be there. I even navigated away when I first opened the dashboard because it didn't show up for a while. I would suggest adding a loading state and a Skeleton.

I've added a Skeleton in 9d5270e.

Can we adjust it so the Skeleton is only displayed if the safe has recovery enabled? I can see it flashing now even in safes where no recovery module is enabled.

Loading recovery takes longer than the timestamp so we do not know whether it is enabled. I would sooner suggest we opt for no loader, but dispatch a notification as well in case the user navigates away. What do you think @TanyaEfremova?

(Due to capacity, I will remove this for now and adjust it in a followup PR as to remove blockers.)

github-actions · 2023-11-20T10:50:46Z

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

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

iamacook added 8 commits November 7, 2023 17:58

feat: poll recovery state of Safe

2cecd77

fix: remove builders

93eb672

fix: add timestamp and validity to state

acd5a83

fix: types

de9dfe5

feat: pending recovery widget

d4bb9fb

fix: gap

a350f0d

fix: add flag check

a5dc313

Merge branch 'recover-state' into pending-recovery-widget

fecaecc

iamacook requested a review from usame-algan November 8, 2023 14:00

iamacook linked an issue Nov 8, 2023 that may be closed by this pull request

[Recovery] Pending recovery widget #2754

Closed

1 task

iamacook added 2 commits November 8, 2023 15:15

fix: don't use Date.now

e6b1f9e

fix: use fake timers

234d5ff

usame-algan added the recovery label Nov 8, 2023

Base automatically changed from recover-state to recovery-epic November 13, 2023 15:57

iamacook added 2 commits November 13, 2023 17:04

Merge branch 'recovery-epic' into pending-recovery-widget

8193ca7

Merge branch 'recovery-epic' into pending-recovery-widget

d89f41e

fix: test

debf834

usame-algan approved these changes Nov 20, 2023

View reviewed changes

fix: tests, logo + add Skeleton

9d5270e

iamacook added 2 commits November 20, 2023 11:55

fix: test

8b331b3

fix: reduce interval + remove Skeleton

f941c8b

iamacook merged commit a759ff4 into recovery-epic Nov 20, 2023
9 of 11 checks passed

iamacook deleted the pending-recovery-widget branch November 20, 2023 11:21

github-actions bot locked and limited conversation to collaborators Nov 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: pending recovery widget #2772

feat: pending recovery widget #2772

iamacook commented Nov 8, 2023

github-actions bot commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 14, 2023 •

edited

Loading

usame-algan left a comment

usame-algan Nov 20, 2023

iamacook Nov 20, 2023

usame-algan Nov 20, 2023

iamacook Nov 20, 2023

usame-algan Nov 20, 2023

iamacook Nov 20, 2023

usame-algan Nov 20, 2023

iamacook Nov 20, 2023

usame-algan Nov 20, 2023

iamacook Nov 20, 2023

usame-algan Nov 20, 2023

iamacook Nov 20, 2023

github-actions bot commented Nov 20, 2023 •

edited

Loading


		import { useWeb3ReadOnly } from './wallets/web3'

		const INTERVAL = 1_000

feat: pending recovery widget #2772

feat: pending recovery widget #2772

Conversation

iamacook commented Nov 8, 2023

What it solves

How this PR fixes it

How to test it

Screenshots

Checklist

github-actions bot commented Nov 8, 2023 • edited Loading

Branch preview

github-actions bot commented Nov 8, 2023 • edited Loading

ESLint Summary View Full Report

github-actions bot commented Nov 8, 2023 • edited Loading

Coverage report

Test suite run success

github-actions bot commented Nov 14, 2023 • edited Loading

ESLint Summary View Full Report

usame-algan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 20, 2023 • edited Loading

ESLint Summary View Full Report

github-actions bot commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 20, 2023 •

edited

Loading