-
Notifications
You must be signed in to change notification settings - Fork 363
Fix/2549 creating spending limit is trying to add the module again #2559
Fix/2549 creating spending limit is trying to add the module again #2559
Conversation
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
I gave this ticket a look now since Juan Pablo told me is testable. The main issue of not being able to add a new allowance with the module enabled and not any other allowance created was fixed It looks good |
} | ||
const safeInfoModules = (remoteSafeInfo.modules || []).map(({ value }) => value) | ||
|
||
if (safeInfoModules.length) { | ||
safeInfo.modules = buildModulesLinkedList(safeInfoModules) | ||
safeInfo.spendingLimitEnabled = | ||
safeInfoModules?.some((module) => sameAddress(module, SPENDING_LIMIT_MODULE_ADDRESS)) ?? false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need optional chaining here because you already check the length property on line 80.
address: safeAddress = '', | ||
spendingLimits, | ||
currentVersion: safeVersion = '', | ||
spendingLimitEnabled: isSpendingLimitEnabled = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need to rename the variable?
Closing since we are choosing a different approach |
Closing since we are choosing a different approach