[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
2023-12-12 13:47:07 +00:00 · 2023-12-12 13:47:07 +00:00 · ac48192562
parent 2191c7dee5
commit ac48192562
5 changed files with 111 additions and 67 deletions
--- 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 &gtsmodel.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.
--- 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.
--- a/internal/router/template.go
+++ b/internal/router/template.go
@ -24,6 +24,7 @@ import (
 	"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,
 	})
 }
--- 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,
--- a/web/template/status_poll.tmpl
+++ b/web/template/status_poll.tmpl
@ -47,6 +47,9 @@
 				<meter aria-hidden="true" id="poll-{{- $pollOption.PollID -}}-option-{{- increment $index -}}" min="0" max="100" value="{{- $pollOption.VoteShare -}}">{{- $pollOption.VoteShare -}}&#37;</meter>
 				<div class="sr-only">Option {{ increment $index }}:&nbsp;<span lang="{{ .LanguageTag.TagStr }}">{{ emojify .Emojis (noescape $pollOption.Title) -}}</span></div>
 				<div class="poll-vote-summary">
+					{{- if isNil $pollOption.VotesCount }}
+					Results not yet published.
+					{{- else -}}
 					<span class="poll-vote-share">{{- $pollOption.VoteShareStr -}}&#37;</span>
 					<span class="poll-vote-count">
 						{{- if eq $pollOption.VotesCount 1 -}}
@ -55,6 +58,7 @@
 							{{- $pollOption.VotesCount }} votes
 						{{- end -}}
 					</span>
+					{{- end -}}
 				</div>
 			</li>
 		{{- end }}