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

Created auxiliary functioon to check if a module is enabled #2570

Merged

Conversation

juampibermani
Copy link
Contributor

What it solves

Resolves #2549

How this PR fixes it

We check for the module to be available in stead of checking that there is any spending limit

How to test it

  • Create a spending limit
  • Delete that spending limit
  • Create another spending limit
    It shouldn't create the transaction to enable the spending limit module

@juampibermani juampibermani added this to the 3.11.0 milestone Jul 21, 2021
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Jul 21, 2021

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

@github-actions
Copy link

@github-actions
Copy link

@francovenica
Copy link
Contributor

Tried the issue of not being able to create a new allowance for an user once the module was previously enabled.
Just in case check the full workflow (creating an allowance, using it, editing, using it again, removing the user, adding it again..)

https://pr2570--safereact.review.gnosisdev.com/rinkeby/app/#/safes/0xc8578beE36bA92B045720448393180947E9A472c/transactions

Looks good to me

@dasanra dasanra requested a review from mmv08 July 26, 2021 07:11
@dasanra dasanra removed this from the 3.11.0 milestone Jul 26, 2021
@dasanra dasanra requested a review from mmv08 July 26, 2021 15:43
@@ -64,3 +65,7 @@ export const enableModuleTx = ({
notifiedTransaction: TX_NOTIFICATION_TYPES.SETTINGS_CHANGE_TX,
}
}

export const isModuleEnabled = (modules: string[] | undefined | null, moduleAddress: string): boolean => {
Copy link
Member

@mmv08 mmv08 Jul 27, 2021

Choose a reason for hiding this comment

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

why the type here for modules is so broad? I believe this function works correctly only if string[] is passed

@dasanra dasanra requested a review from mmv08 July 28, 2021 08:50
Copy link
Member

@mmv08 mmv08 left a comment

Choose a reason for hiding this comment

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

🎉

@dasanra dasanra merged commit 035ef54 into dev Jul 28, 2021
@dasanra dasanra deleted the fix/2549-creating-new-spending-limit-tries-to-add-module-again branch July 28, 2021 09:33
@github-actions github-actions bot locked and limited conversation to collaborators Jul 28, 2021
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.

Spending limit - Creating a spending limit is trying to add the module again
4 participants