From ac481925622df9bf8024d1b5726282d0214fd22b Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Tue, 12 Dec 2023 13:47:07 +0000 Subject: [PATCH] [bugfix] poll vote count fixes (#2444) * don't drop all vote counts if hideCounts is set, refactors poll option extraction slightly * omit voters_count when not set * make voters_count a ptr to ensure it is omit unless definitely needed * handle case of expires_at, voters_count and option.votes_count being nilable * faster isNil check * remove omitempty tags since mastodon API marks things as nullable but still sets them in outgoing json --- internal/ap/extract.go | 78 +++++++++++++++-------- internal/api/model/poll.go | 6 +- internal/router/template.go | 9 +++ internal/typeutils/internaltofrontend.go | 81 +++++++++++++----------- web/template/status_poll.tmpl | 4 ++ 5 files changed, 111 insertions(+), 67 deletions(-) diff --git a/internal/ap/extract.go b/internal/ap/extract.go index 987ff5e55..11c20494c 100644 --- a/internal/ap/extract.go +++ b/internal/ap/extract.go @@ -1102,19 +1102,11 @@ func ExtractPoll(poll Pollable) (*gtsmodel.Poll, error) { var closed time.Time // Extract the options (votes if any) and 'multiple choice' flag. - options, votes, multi, err := ExtractPollOptions(poll) + options, multi, hideCounts, err := extractPollOptions(poll) if err != nil { return nil, err } - // Check if counts have been hidden from us. - hideCounts := len(options) != len(votes) - - if hideCounts { - // Simply provide zeroed slice. - votes = make([]int, len(options)) - } - // Extract the poll closed time, // it's okay for this to be zero. closedSlice := GetClosed(poll) @@ -1138,53 +1130,87 @@ func ExtractPoll(poll Pollable) (*gtsmodel.Poll, error) { voters := GetVotersCount(poll) return >smodel.Poll{ - Options: options, + Options: optionNames(options), Multiple: &multi, HideCounts: &hideCounts, - Votes: votes, + Votes: optionVotes(options), Voters: &voters, ExpiresAt: endTime, ClosedAt: closed, }, nil } -// ExtractPollOptions extracts poll option name strings, and the 'multiple choice flag' property value from Pollable. -func ExtractPollOptions(poll Pollable) (names []string, votes []int, multi bool, err error) { +// pollOption is a simple type +// to unify a poll option name +// with the number of votes. +type pollOption struct { + Name string + Votes int +} + +// optionNames extracts name strings from a slice of poll options. +func optionNames(in []pollOption) []string { + out := make([]string, len(in)) + for i := range in { + out[i] = in[i].Name + } + return out +} + +// optionVotes extracts vote counts from a slice of poll options. +func optionVotes(in []pollOption) []int { + out := make([]int, len(in)) + for i := range in { + out[i] = in[i].Votes + } + return out +} + +// extractPollOptions extracts poll option name strings, the 'multiple choice flag', and 'hideCounts' intrinsic flag properties value from Pollable. +func extractPollOptions(poll Pollable) (options []pollOption, multi bool, hide bool, err error) { var errs gtserror.MultiError // Iterate the oneOf property and gather poll single-choice options. IterateOneOf(poll, func(iter vocab.ActivityStreamsOneOfPropertyIterator) { - name, count, err := extractPollOption(iter.GetType()) + name, votes, err := extractPollOption(iter.GetType()) if err != nil { errs.Append(err) return } - names = append(names, name) - if count != nil { - votes = append(votes, *count) + if votes == nil { + hide = true + votes = new(int) } + options = append(options, pollOption{ + Name: name, + Votes: *votes, + }) }) - if len(names) > 0 || len(errs) > 0 { - return names, votes, false, errs.Combine() + if len(options) > 0 || len(errs) > 0 { + return options, false, hide, errs.Combine() } // Iterate the anyOf property and gather poll multi-choice options. IterateAnyOf(poll, func(iter vocab.ActivityStreamsAnyOfPropertyIterator) { - name, count, err := extractPollOption(iter.GetType()) + name, votes, err := extractPollOption(iter.GetType()) if err != nil { errs.Append(err) return } - names = append(names, name) - if count != nil { - votes = append(votes, *count) + if votes == nil { + hide = true + votes = new(int) } + options = append(options, pollOption{ + Name: name, + Votes: *votes, + }) }) - if len(names) > 0 || len(errs) > 0 { - return names, votes, true, errs.Combine() + if len(options) > 0 || len(errs) > 0 { + return options, true, hide, errs.Combine() } - return nil, nil, false, errors.New("poll without options") + return nil, false, false, errors.New("poll without options") } // IterateOneOf will attempt to extract oneOf property from given interface, and passes each iterated item to function. diff --git a/internal/api/model/poll.go b/internal/api/model/poll.go index 3eb801998..5603ff222 100644 --- a/internal/api/model/poll.go +++ b/internal/api/model/poll.go @@ -28,7 +28,7 @@ type Poll struct { ID string `json:"id"` // When the poll ends. (ISO 8601 Datetime). - ExpiresAt string `json:"expires_at"` + ExpiresAt *string `json:"expires_at"` // Is the poll currently expired? Expired bool `json:"expired"` @@ -40,7 +40,7 @@ type Poll struct { VotesCount int `json:"votes_count"` // How many unique accounts have voted on a multiple-choice poll. - VotersCount int `json:"voters_count"` + VotersCount *int `json:"voters_count"` // When called with a user token, has the authorized user voted? // @@ -68,7 +68,7 @@ type PollOption struct { Title string `json:"title"` // The number of received votes for this option. - VotesCount int `json:"votes_count"` + VotesCount *int `json:"votes_count"` } // PollRequest models a request to create a poll. diff --git a/internal/router/template.go b/internal/router/template.go index 804f532bd..d5e36d6f2 100644 --- a/internal/router/template.go +++ b/internal/router/template.go @@ -24,6 +24,7 @@ "path/filepath" "strings" "time" + "unsafe" "github.com/gin-gonic/gin" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" @@ -172,6 +173,13 @@ func increment(i int) int { return i + 1 } +// isNil will safely check if 'v' is nil without +// dealing with weird Go interface nil bullshit. +func isNil(i interface{}) bool { + type eface struct{ _, data unsafe.Pointer } + return (*eface)(unsafe.Pointer(&i)).data == nil +} + func LoadTemplateFunctions(engine *gin.Engine) { engine.SetFuncMap(template.FuncMap{ "escape": escape, @@ -185,5 +193,6 @@ func LoadTemplateFunctions(engine *gin.Engine) { "emojify": emojify, "acctInstance": acctInstance, "increment": increment, + "isNil": isNil, }) } diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go index b91b2ad34..353c719b5 100644 --- a/internal/typeutils/internaltofrontend.go +++ b/internal/typeutils/internaltofrontend.go @@ -686,9 +686,9 @@ func (c *Converter) StatusToWebStatus( webPollOptions := make([]apimodel.WebPollOption, len(poll.Options)) for i, option := range poll.Options { var voteShare float32 - if totalVotes != 0 && - option.VotesCount != 0 { - voteShare = (float32(option.VotesCount) / float32(totalVotes)) * 100 + + if totalVotes != 0 && option.VotesCount != nil { + voteShare = float32(*option.VotesCount) / float32(totalVotes) * 100 } // Format to two decimal points and ditch any @@ -1432,11 +1432,11 @@ func (c *Converter) PollToAPIPoll(ctx context.Context, requester *gtsmodel.Accou var ( options []apimodel.PollOption totalVotes int - totalVoters int - voted *bool + totalVoters *int + hasVoted *bool ownChoices *[]int isAuthor bool - expiresAt string + expiresAt *string emojis []apimodel.Emoji ) @@ -1462,57 +1462,62 @@ func (c *Converter) PollToAPIPoll(ctx context.Context, requester *gtsmodel.Accou // Set choices by requester. ownChoices = &vote.Choices - // Update default totals in the - // case that counts are hidden. + // Update default total in the + // case that counts are hidden + // (so we just show our own). totalVotes = len(vote.Choices) - totalVoters = 1 - for _, choice := range *ownChoices { - options[choice].VotesCount++ - } } else { - // Requester is defined but hasn't made - // a choice. Init slice to serialize as `[]`. - ownChoices = util.Ptr(make([]int, 0)) + // Requester hasn't yet voted, use + // empty slice to serialize as `[]`. + ownChoices = &[]int{} } // Check if requester is author of source status. isAuthor = (requester.ID == poll.Status.AccountID) - // Requester is defined so voted should be defined too. - voted = util.Ptr((isAuthor || len(*ownChoices) > 0)) + // Set whether requester has voted in poll (or = author). + hasVoted = util.Ptr((isAuthor || len(*ownChoices) > 0)) } if isAuthor || !*poll.HideCounts { - // A remote status, - // the simple route! - // - // Pull cached remote values. - totalVoters = (*poll.Voters) + // Only in the case that hide counts is + // disabled, or the requester is the author + // do we actually populate the vote counts. - // When this is status author, or hide counts - // is disabled, set the counts known per vote, - // and accumulate all the vote totals. + if *poll.Multiple { + // The total number of voters are only + // provided in the case of a multiple + // choice poll. All else leaves it nil. + totalVoters = poll.Voters + } + + // Populate per-vote counts + // and overall total vote count. for i, count := range poll.Votes { - options[i].VotesCount = count + if options[i].VotesCount == nil { + options[i].VotesCount = new(int) + } + (*options[i].VotesCount) += count totalVotes += count } } if !poll.ExpiresAt.IsZero() { // Calculate poll expiry string (if set). - expiresAt = util.FormatISO8601(poll.ExpiresAt) + str := util.FormatISO8601(poll.ExpiresAt) + expiresAt = &str } - // Try to inherit emojis - // from parent status. - if pStatus := poll.Status; pStatus != nil { - var err error - emojis, err = c.convertEmojisToAPIEmojis(ctx, pStatus.Emojis, pStatus.EmojiIDs) - if err != nil { - // Fall back to empty slice. - log.Errorf(ctx, "error converting emojis from parent status: %v", err) - emojis = make([]apimodel.Emoji, 0) - } + var err error + + // Try to inherit emojis from parent status. + emojis, err = c.convertEmojisToAPIEmojis(ctx, + poll.Status.Emojis, + poll.Status.EmojiIDs, + ) + if err != nil { + log.Errorf(ctx, "error converting emojis from parent status: %v", err) + emojis = []apimodel.Emoji{} // fallback to empty slice. } return &apimodel.Poll{ @@ -1522,7 +1527,7 @@ func (c *Converter) PollToAPIPoll(ctx context.Context, requester *gtsmodel.Accou Multiple: (*poll.Multiple), VotesCount: totalVotes, VotersCount: totalVoters, - Voted: voted, + Voted: hasVoted, OwnVotes: ownChoices, Options: options, Emojis: emojis, diff --git a/web/template/status_poll.tmpl b/web/template/status_poll.tmpl index f6acecfa0..d26046283 100644 --- a/web/template/status_poll.tmpl +++ b/web/template/status_poll.tmpl @@ -47,6 +47,9 @@
Option {{ increment $index }}: {{ emojify .Emojis (noescape $pollOption.Title) -}}
+ {{- if isNil $pollOption.VotesCount }} + Results not yet published. + {{- else -}} {{- $pollOption.VoteShareStr -}}% {{- if eq $pollOption.VotesCount 1 -}} @@ -55,6 +58,7 @@ {{- $pollOption.VotesCount }} votes {{- end -}} + {{- end -}}
{{- end }}