Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 1.19.0 #2546

Merged
merged 37 commits into from
Sep 28, 2023
Merged

Release 1.19.0 #2546

merged 37 commits into from
Sep 28, 2023

Conversation

katspaugh
Copy link
Member

@katspaugh katspaugh commented Sep 26, 2023

🎁 Features

🐞 Bug fixes

🛠️ Miscellaneous

iamacook and others added 30 commits August 31, 2023 17:32
* fix: don't track clicks on disabled elements

* fix: remove dependency

* fix: check whether any children are disabled
* Chore: track tx errors on Sentry

* Fix: error 805 for the sign flow
Adds user `fmrsabino` to the CLA allowlist
* Update default value for Relay Service URL (staging)

Updates the default value set for `SAFE_RELAY_SERVICE_URL_STAGING` from `https://safe-client-nest.staging.5afe.dev/v1/relay` to `https://safe-client.staging.5afe.dev/v1/relay`

* Update default value for Relay Service URL (production)

Updates the default value set for `NEXT_PUBLIC_SAFE_RELAY_SERVICE_URL_PRODUCTION` from `https://safe-client-nest.safe.global/v1/relay` to `https://safe-client.safe.global/v1/relay`
* welcome screen re-ordering enhancement for mobile view

* my safe accounts moved to top
* fix: use current Safe version for deployments

* fix: don't return deployments where none are
* Fix: [Dashboard] better layout for pending txs

* Order 1
* Refactor address book and balnces tests

* Fix tests
* Rfactoring of existing Cypress tests (P2)

* Refactoring of tests: create wallet, import/export, load safe, nfts, landing, pendig actions; general refactoring
* fix: reset nonce if outdated

resets nonce to recommended nonce if the currently selected nonce is lower than the Safe nonce.

* fix: set to safe.nonce instead of recommendedNonce

* refactor: move invalid nonce logic to SafeTxProvider

* fix: do not setNonce for default values, keep SafeTxProvider in sync with form

* fix: only update nonce on touch

* refactor: remove unused code

* fix: review comments

* chore: add comment to useAsync deps
* Tests: fix nonce editing in create_tx

* Fix batch_tx
* Add human readable transfers to history and queue

* Format token value to adhere to the style guide

* Fix failing tests

* Fix mobile view
* fix: close transaction flow if Safe changes

* refactor: use `handleModalClose`

* fix: use `safeAddress` variable
* Feat: actionable pending txs on the Dashboard

* Fix e2e test

* Rename vars

* Txs -> transactions
* fix: close transaction modal if already on `href`

* fix: Transaction Builder link
* Cypress tests refactoring: PO model, safe app tests

* Cypress tests refactoring: PO model, safe app tests

# Conflicts:
#	cypress/e2e/pages/safeapps.pages.js
#	cypress/support/constants.js

* Fix tests

* Add missing constants

* Remove unused import

* Fix tests, fix naming

* Update Ubuntu image

* Update Cypress version

* Update yarn .lock file to reflect requied changes due to cypress v update

* Fix tests: manage local storage, set test isolation setting

* Disable pendingactions tests, need to clarify logic

* Remove unused import

* Update cypress/e2e/pages/create_tx.pages.js

Co-authored-by: Aaron Cook <[email protected]>

* Update cypress/e2e/pages/create_tx.pages.js

Co-authored-by: Aaron Cook <[email protected]>

* Address comments in PR

* Update constants

* Disabling beamer test as it is not reliable

---------

Co-authored-by: Aaron Cook <[email protected]>
* fix: Update tx flow form when selecting a replacement nonce

* fix: Update recommended nonce in the form if it changes in the background

* fix: Skip initial validation

* fix: Move logic inside useRecommendedNonce
* fix: boolean env vars

* fix: remove fallback
* fix: allow safe apps to overwrite the off-chain signing setting

* chore: convert deployment branch name to lowercase

* fix: deploy script
* feat: implement Firebase Cloud Messaging

* fix: move to custom `next-pwa` worker

* Revert "fix: move to custom `next-pwa` worker"

This reverts commit 2654d76.

* fix: remove `next-pwa`

* fix: convert SW to TS + add notification text

* fix: improve notifications

* refactor: separate types + add test

* fix: improve UI + add partial tests

* fix: add more test coverage

* fix: list Safes

* fix: migrate to SDK

* feat: basic notification preferences

* fix: tests

* fix: implement designs

* feat: per-Safe preferences

* refactor: move registrations to IndexedDB

* fix: adjust style, add banner + registration list

* fix: split databases + finish design

* fix: database reactivity with `ExternalStore`

* fix: update tests

* fix: preferences, env vars, not in UI + tweaks

* feat: add deep link to notification

* fix: add tracking + rename to push notifications

* fix: test

* fix: condense preferences

* fix: address review comments + adjust design

* fix: test

* refactor: clean up code

* refactor: make `_parseWebhookNotification` a map

* fix: register for confirmation requests by default

* fix: adjust text + fix tests

* fix: reduce service worker bundle size

* fix: don't re-export

* wip: reimplement next-pwa

* fix: `next-pwa` with custom worker

* chore: convert env var to JSON

* refactor: abstract code from component + fix test

* fix: env issue

* refactor: leverage SDK

* feat: add cached tracking

* fix: test

* fix: mock response on Safe Apps share page

* revert: cached tracking

* refactor: restructure/rename + add error codes

* fix: address review comments

* fix: address comments

* fix: regenerate lockfile

* fix: address comments

* feat: add push notification tracking (#2500)

* feat: add push notification tracking

* fix: address review comments

* fix: error message

* feat: add close button to banner

* fix: remove unnecessary types + qa findings

* fix: texts, confirmation registration + enable all

* fix: mock

* fix: spacing

* fix: spacing

* fix: use EIP-191

* fix: test

* feat: add feature flag

* fix: test

* fix: add space + only show banner on added Safes

* fix: remove space

* fix: e2e
* fix: Add AB test for human-readable transactions

* fix: Adjust ab test key, set local storage for e2e tests

* fix: Slightly different column widths in history/queue
@github-actions
Copy link

github-actions bot commented Sep 26, 2023

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


4 out of 5 committers have signed the CLA.
@katspaugh
@udhaykumarbala
@mike10ca
@KaushikKC
@fmrsabino
You can retrigger this bot by commenting recheck in this Pull Request

@github-actions
Copy link

github-actions bot commented Sep 26, 2023

Branch preview

✅ Deploy successful!

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

@github-actions
Copy link

github-actions bot commented Sep 26, 2023

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

@francovenica
Copy link
Contributor

@iamacook
Issue notifications:
If you are not in the correct network The "Enable all" button in the top bar notification only makes me switch networks, but doesn't trigger the off-chain signature to actually enable notifications

* fix: adjust Ledger signatures for notifications

* fix: test
@francovenica
Copy link
Contributor

Issue:

In the global preferences for notifications, unchecking and saving only one checkbox from one network unchecks the rest of the checkboxes
checkbox unchecking

@francovenica
Copy link
Contributor

I'm leaving a note correcting something I said in the original Notifications EPIC:

The "Confirmation request" DOES work on safes 2/2.
Debugging with Aaron today, I've found out that Firefox is very pesky with notifications. It seems that enabling/disabling notifications constantly can create issues (service workers and the indexDB)
I decied to use chrome as the browser for control, and it seems to be much more consistent with the messages.

Another note that I didn't say in the original EPIC but is worth mentioning:
"Confirmation request" is the only type of notification that looks at the owners who are receiving it. So if an owner signed the enabling of notifications and then is kicked out of the safe, he will no longer receive notificatons about "Confirmation request", but it WILL receive notifications about "tx executions" and "Tokens received". The user can still, as a non owner, unsubscribre from these notifications types.

…signatures (#2551)

* fix: notification preferences + post-switch registration

* fix: sequentially sign for hardware wallets

* fix: default value

* fix: catch chain assertion rejections
@iamacook
Copy link
Member

I've found out that Firefox is very pesky with notifications. It seems that enabling/disabling notifications constantly can create issues (service workers and the indexDB) I decied to use chrome as the browser for control, and it seems to be much more consistent with the messages.

I can confirm that I get notifications in Firefox and Chrome, even after unregistering the service worker/clearing the IndexedDB. It is concerning that it is so flakey for you but I hope that the user won't have to touch either of these.

Another note that I didn't say in the original EPIC but is worth mentioning: "Confirmation request" is the only type of notification that looks at the owners who are receiving it. So if an owner signed the enabling of notifications and then is kicked out of the safe, he will no longer receive notificatons about "Confirmation request", but it WILL receive notifications about "tx executions" and "Tokens received". The user can still, as a non owner, unsubscribre from these notifications types.

We have mentioned this in the help article that will be linked to as per discussion with @kirkkonen and @TanyaEfremova.

* fix: only delete all preferences per chain

* fix: required sig count + test coverage
@francovenica
Copy link
Contributor

Issue:

In a brand new safe (or a safe that has never executed a transaction) the nonce field in the tx form shows the skeleton all the time and never shows the Nonce 0 as expected.
After executing that first transaction it works fine, so the issue is if the safe has to execute the nonce 0
image

@francovenica
Copy link
Contributor

Messages are broken, the text overlaps all together. Maybe is because human-readable
image

This safe has messages: (turn on Prod CGW)
https://release_1_19_0--walletweb.review-wallet-web.5afe.dev/transactions/messages?safe=gor:0x0EDc6697427E9e9d62A1eE1A2951AfA7bE9e75B7

@usame-algan
Copy link
Member

usame-algan commented Sep 28, 2023

In a brand new safe (or a safe that has never executed a transaction) the nonce field in the tx form shows the skeleton all the time and never shows the Nonce 0 as expected.

Messages are broken, the text overlaps all together. Maybe is because human-readable

Nice finds! I've fixed both in #2562 and merged into this Branch

@francovenica
Copy link
Contributor

The 2 issues reported where fixed

Nonce 0 is shown just fine now
The messages look fine as well

image
image

@francovenica
Copy link
Contributor

The rest of the ticket is fine.

I've checked the list in the description and that can be tested in the UI look good

I've done the regression:
Safe creation and adding
Tx creation / execution with Trezor, Ledger, MM extension, MM phone app via WC, Mobile pairing
Send funds, NFT's
Safe app (tx builder)
Execution un bulk
Message signing (using epi 1271 test app)
Owner edition (adding/removin/replacing)
Spending limit creation removal usage
Address book
Import/export
Settings in general
....

@usame-algan
Copy link
Member

recheck

@katspaugh katspaugh merged commit a44252b into main Sep 28, 2023
8 of 9 checks passed
@katspaugh katspaugh deleted the release/1.19.0 branch September 28, 2023 16:17
@github-actions github-actions bot locked and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants