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

Space settings: use alias for vote and proposal #3265

Closed
wants to merge 25 commits into from

Conversation

zzuziak
Copy link
Contributor

@zzuziak zzuziak commented Oct 24, 2022

Fixes Issue #2844

Background

Currently voting and creating new proposals requires a signature each time. The goal of this PR is to add space setting enabling using the alias for both actions (one setting enables both).

Related PRs

Changes in this PR:

  • update interfaces and queries with new aliased field for spaces.filters and spaces.voting
  • add inputs and locale for aliased proposal and voting
  • check alias and pass alias data in useClient composable if space settings are enabled

Screenshots & Rec

Voting setting (currently in voting section, doesn't mention proposals):
image

Voting without alias:
unaliased

Aliased proposal & vote:
aliased

How to test and review this PR?

  1. Go to fabien.eth settings and enable aliased voting (requires merging two PRs mentioned above)
  2. Create a new proposal for the space - if you have connected your wallet, there shouldn't be a request for signature
  3. New proposal should be created
  4. Vote on proposal - shouldn't get a request for signature and vote should go through ✔️

@vercel
Copy link

vercel bot commented Oct 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
snapshot ✅ Ready (Inspect) Visit Preview Nov 11, 2022 at 9:20PM (UTC)

@bonustrack
Copy link
Member

@zzuziak Just a quick comment, on this repo (and most of our repo) we use Yarn, this PR shouldn't have a package-lock.json file

@zzuziak
Copy link
Contributor Author

zzuziak commented Oct 24, 2022

Hey @bonustrack, thanks 🙏 I was trying to fetch this branch with npm as a dependency to test it, must have generated the file then.

I can't test the new schema updated on two branches (snapshot.js and hub) - what's your approach to test the repos together before merging PRs?

@zzuziak
Copy link
Contributor Author

zzuziak commented Oct 24, 2022

Updated with a single param for space.voting, currently the setting lies within the SettingsVotingBlock:

image

To make it more readable I can move it to an individual block and mention that it also allows aliased proposal creation.

@bonustrack
Copy link
Member

bonustrack commented Oct 24, 2022

@zzuziak You can use yarn link to get Snapshot.js in the UI. To test the whole flow you would need to run the hub locally but I recommend focus on the UI integration, we will do some change on the hub to get settings with the new aliased param accepted

@zzuziak zzuziak marked this pull request as ready for review October 24, 2022 14:42
@zzuziak
Copy link
Contributor Author

zzuziak commented Oct 24, 2022

Ok, ready for first review 👌

I didn't use aliasing for the usePersonalSign, I assumed Gnosis should always require the signature but correct me if I wrong, still digging into it 🙈

src/composables/useClient.ts Show resolved Hide resolved
src/locales/default.json Outdated Show resolved Hide resolved
Zuza Zuber added 2 commits October 27, 2022 20:17
src/locales/default.json Outdated Show resolved Hide resolved
src/locales/default.json Outdated Show resolved Hide resolved
@samuveth
Copy link
Contributor

samuveth commented Oct 28, 2022

We should allow alias actions only for these actions: "proposal", "vote" and "delete-proposal"

I think we have a miss understanding, I was referring to this comment from Fabien. I don't see this handled anywhere in your update.

In the current implementation in this PR it will also try to update settings with alias

@zzuziak
Copy link
Contributor Author

zzuziak commented Oct 28, 2022

I think we have a miss understanding, I was referring to this comment from Fabien. I don't see this handled anywhere in your update.

In the current implementation in this PR it will also try to update settings with alias

@samuveth I tagged you in the comments to clarify, let me know if it's still confusing 🙏

@zzuziak
Copy link
Contributor Author

zzuziak commented Jan 30, 2023

Hey @samuveth could you have a look at this PR again and see if there is something left to be done?

@samuveth
Copy link
Contributor

@zzuziak yes, there are things to be done outside of this PR first and the useClient code need to be updated since we have depreciated personal sign since this PR was made. I'll take care of it from here and I'll assign it to myself.

@samuveth samuveth self-assigned this Jan 31, 2023
@snapshot-labs snapshot-labs deleted a comment from Anon79 Feb 2, 2023
@snapshot-labs snapshot-labs deleted a comment from Anon79 Feb 2, 2023
@samuveth samuveth marked this pull request as draft February 12, 2023 05:56
@samuveth samuveth added wontfix This will not be worked on enhancement New feature or request labels Mar 18, 2023
@samuveth samuveth removed the enhancement New feature or request label May 8, 2023
@stale
Copy link

stale bot commented Sep 6, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issues that are old and soon to be closed label Sep 6, 2023
@stale stale bot closed this Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues that are old and soon to be closed wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a space setting to enable vote and proposal action with alias
4 participants