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

fix: add set/deleteGuard #3645

Merged
merged 5 commits into from
Mar 10, 2022
Merged

fix: add set/deleteGuard #3645

merged 5 commits into from
Mar 10, 2022

Conversation

iamacook
Copy link
Member

@iamacook iamacook commented Mar 9, 2022

What it solves

Resolves #3639

How this PR fixes it

The collapsed label and correct transaction info displays have been added for set/deleteGuard calls. A fallback for any future new SettingsInfo has been added that will simply show the type as well.

How to test it

Open rin:0x6cf158C37354d8da5396fb6d3e965318CCf119c1 on the production CGW and observe the correct labelling of the setGuard (in the history) and deleteGuard (in the queue). When expanding the transaction, the info should be displayed.

Screenshots

image
image

@github-actions
Copy link

github-actions bot commented Mar 9, 2022

CLA Assistant Lite All Contributors have signed the CLA.

@iamacook iamacook linked an issue Mar 9, 2022 that may be closed by this pull request
2 tasks
@iamacook
Copy link
Member Author

iamacook commented Mar 9, 2022

This isn't based off dev as I'm waiting on the other PR to be merged first.

@github-actions
Copy link

github-actions bot commented Mar 9, 2022

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
Copy link

github-actions bot commented Mar 9, 2022

Deployment links

🟠 Rinkeby Mainnet 🟣 Polygon 🟡 BSC Arbitrum 🟢 Gnosis Chain

@github-actions
Copy link

github-actions bot commented Mar 9, 2022

E2E Tests Failed
Check the results here: https://github.com/gnosis/safe-react-e2e-tests/actions/runs/1963243256

Failed tests:

  • ❌ Add an existing safe Add an existing safe
  • ❌ Read-only transaction creation and review Read-only transaction creation and review
  • ❌ Safe Apps List Safe Apps List
  • ❌ Safe Balances Safe Balances

Copy link
Member

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@liliya-soroka
Copy link
Member

I would expect to see the address of deleted Guard, as we have for module
image

@iamacook
Copy link
Member Author

iamacook commented Mar 9, 2022

I would expect to see the address of deleted Guard, as we have for module

The CGW does not return this information. Would this be feasible @jpalvarezl? It is not technically feasible.

@francovenica
Copy link
Contributor

francovenica commented Mar 10, 2022

It looks fine
Checked in that safe and the "Set guard" "delete guard" labels are there.
Given the comment regarding the delete guard not showing the address of expected.

Note: I don't understand why deleting a guard is actually called "Delete guard" but removing other modules like spending limit is "Disable module" and a complete different tx. Although they are both modules are they treated differently?

image

image

EDIT: I always thought that guards were a special case of a module, but it seems is something completely different, so is natural they are show differently in the tx's.

Base automatically changed from use-tx-token-type to dev March 10, 2022 12:53
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1963190451

  • 0 of 5 (0.0%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.03%) to 35.1%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts 0 2 0.0%
src/routes/safe/components/Transactions/TxList/TxInfoSettings.tsx 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts 1 1.75%
src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx 1 6.19%
Totals Coverage Status
Change from base Build 1963180825: -0.03%
Covered Lines: 3407
Relevant Lines: 8769

💛 - Coveralls

@iamacook iamacook merged commit 7995c19 into dev Mar 10, 2022
@iamacook iamacook deleted the add-set-delete-guard branch March 10, 2022 14:51
@github-actions github-actions bot locked and limited conversation to collaborators Mar 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SetGuard/DeleteGuard displays
8 participants