Conversation
… Together, these provide a way to run polls using gno.land.
github-project-automation
bot
added this to
💪 Bounties & Worx and 🧙♂️Gno.land development
github-actions
bot
added
the
🧾 package/realm
Gno2D2
added
the
review/triage-pending
🛠 PR Checks Summary🔴 Pending initial approval by a review team member, or review from tech-staff Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🔴 Pending initial approval by a review team member, or review from tech-staff ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
|
Hi @wyhaines . The API allows to edit option text. This means that the poll owner can do: Then let a bunch of people vote. Then switch the options: If this was a vote on an important topic, then this could be a security risk. What do you think? Should there be a limit to editing the poll options after voting starts? |
|
@jefft0 that is a good catch. I'll add a guard to prevent changing of the poll options after the first vote is cast. That seems reasonable to me. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
jefft0
left a comment
jefft0
left a comment
There was a problem hiding this comment.
LGTM. My security concern was addressed about not editing the poll after voting starts. Good test coverage.
Gno2D2
removed
the
review/triage-pending
|
Hey @petar-dambovaliev @leohhhn do you have any reviews for this? |
There was a problem hiding this comment.
Sorry for the slow turnaround 🙏
Left some comments; I think that there's a few best practices we can put into this code to make it better.
Txlinks would be a great addition to the app, especially now since we have the Adena integration. Could you please post a few screenshots of how the app would look like
| _, exists := p.Votes.Get(addr.String()) | ||
| return exists |
| } | ||
|
|
||
| // VoterCount returns how many unique addresses have cast votes. | ||
| func (p *Poll) VoterCount() int { |
There was a problem hiding this comment.
this func is readonly so no need for a pointer receiver
| func (p *Poll) VoterCount() int { | |
| func (p Poll) VoterCount() int { |
| //---------------------------------------- | ||
| // Rendering |
There was a problem hiding this comment.
let's move this to a render.gno file
| func renderSinglePoll(b *bytes.Buffer, id string, p *poll.Poll) { | ||
| b.WriteString(ufmt.Sprintf("# Poll %s: %s\n\n", id, p.Title)) | ||
| b.WriteString(ufmt.Sprintf("**Description:** %s\n\n", p.Description)) | ||
| b.WriteString(ufmt.Sprintf("**Deadline:** %s\n", p.Deadline.Format(time.RFC1123))) | ||
| b.WriteString(ufmt.Sprintf("**Creator:** %s\n", p.Creator)) | ||
| b.WriteString(ufmt.Sprintf("**MaxChoices:** %d\n", p.MaxChoices)) |
There was a problem hiding this comment.
Not sure what your opinion is but I much more prefer that functions do not mutate their args. So ideally we'd just return a simple string here
There was a problem hiding this comment.
Also code is more readable with out := "" instead of WriteString, and we don't care too much about the saving on computation since this function is queried
| path = strings.TrimSpace(path) | ||
| if path == "" || strings.HasPrefix(path, "page=") { | ||
| return renderPollList(&b, path) | ||
| } | ||
|
|
||
| val, exists := polls.Get(path) | ||
| if !exists { | ||
| return renderPollList(&b, "") | ||
| } |
There was a problem hiding this comment.
we could use p/demo/mux here. https://gno.land/r/docs/routing
| func init() { | ||
| polls = avl.NewTree() | ||
| } |
There was a problem hiding this comment.
we can just init this at the var block
| var ( | ||
| polls *avl.Tree // TODO: This would be better as a btree; no need to key off a string, then. | ||
| pollIDCounter seqid.ID | ||
| pageSize = 5 |
| newP := poll.NewPoll(title, description, []string{}, maxChoices, deadline, caller) | ||
| polls.Set(id, newP) |
There was a problem hiding this comment.
I found it generally easier to set the value to the address instead of the value itself; this allows you to modify the value a bit more easily down the line (ie directly modify after getting it from the tree, and not having to set again)
| newP := poll.NewPoll(title, description, []string{}, maxChoices, deadline, caller) | ||
| polls.Set(id, newP) | ||
|
|
||
| return ufmt.Sprintf("Successfully created poll #%s!", id) |
There was a problem hiding this comment.
Is this string useful for anything?
I'd much rather have a std.Emit(), and a good render UI with txlinks for specific polls
| if p.Creator != std.OriginCaller() { | ||
| panic("only the poll creator can add options") | ||
| } |
There was a problem hiding this comment.
Same thing for OriginCaller
I only discovered txlink last week. This is a big issue, in general, for development with gno.land. Discoverability for the libs that we have is difficult. I'll take a look at how it could make the gnoweb based UI better. |
|
This PR is stale because it has been open 3 months with no activity. Remove stale label or comment or this will be closed in 3 months. |
|
Closing as stale; @wyhaines please feel free to reopen if you want to keep working on this :) |
5 participants
This is a simple polling package and realm. It allows the creation of multi-option polls with the ability to vote for more than one option.
It was based off of Leon's simple polling realm in the docs, but expanded to handle any number of polls with any number of options.
This branch supercedes kh.add-poll-package-and-realm, and this PR replaces #3776. It was far simpler to just make a completely clean new branch with the code than to try to clean up an old rebase.