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

fix: disabled batch sidebar buttons when not owner #2454

Merged
merged 3 commits into from
Aug 29, 2023
Merged

Conversation

iamacook
Copy link
Member

@iamacook iamacook commented Aug 28, 2023

What it solves

Resolves #2404

How this PR fixes it

The batch sidebar buttons are now disabled if the user is not connected or not an owner.

How to test it

Disabled

As a non-owner open the sidebar and observe the disabled buttons with tooltip when a batch doesn't exists and when one does.

Enabled

As an owner or non-owner open the sidebar and observe the enabled buttons tooltip when a batch doesn't exists and when one does.

Screenshots

batch sidebar buttons

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) 🧑‍💻

@iamacook iamacook self-assigned this Aug 28, 2023
@github-actions
Copy link

github-actions bot commented Aug 28, 2023

Branch preview

✅ Deploy successful!

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

@github-actions
Copy link

github-actions bot commented Aug 28, 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

It looks good for non-owners and disconnected users, they see the buttons grayed out.

I dissagree with the spending limit owners being able to use the feature.

A person with an allowance cannot do any of this things: Put a transaction that use the allowance in a batch, nor confirm a batch since none of those listed tx will ever be a spending limit. So both actions "Add tx to batch" and "Confirm batch" cannot be used by a user with a spending limit

@iamacook
Copy link
Member Author

I dissagree with the spending limit owners being able to use the feature.

A person with an allowance cannot do any of this things: Put a transaction that use the allowance in a batch, nor confirm a batch since none of those listed tx will ever be a spending limit. So both actions "Add tx to batch" and "Confirm batch" cannot be used by a user with a spending limit

Thanks for the thorough testing! I removed access to non-owner spending limit beneficaries in 4c9f083.

@francovenica
Copy link
Contributor

Looks good now

The non-owners with allowance cannot use the feature anymore.

@iamacook iamacook merged commit c40ac35 into dev Aug 29, 2023
8 of 9 checks passed
@iamacook iamacook deleted the batch-sidebar-buttons branch August 29, 2023 13:53
@github-actions github-actions bot locked and limited conversation to collaborators Aug 29, 2023
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 to batch] non-owners and disconnected users issues
3 participants