Skip to content

Commit

Permalink
[bugfix] poll vote count fixes (#2444)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
NyaaaWhatsUpDoc committed Dec 12, 2023
1 parent 2191c7d commit ac48192
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 67 deletions.
78 changes: 52 additions & 26 deletions internal/ap/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions internal/api/model/poll.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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?
//
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions internal/router/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"path/filepath"
"strings"
"time"
"unsafe"

"github.com/gin-gonic/gin"
apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
Expand Down Expand Up @@ -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,
Expand All @@ -185,5 +193,6 @@ func LoadTemplateFunctions(engine *gin.Engine) {
"emojify": emojify,
"acctInstance": acctInstance,
"increment": increment,
"isNil": isNil,
})
}
81 changes: 43 additions & 38 deletions internal/typeutils/internaltofrontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)

Expand All @@ -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.

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
}

// When this is status author, or hide counts
// is disabled, set the counts known per vote,
// and accumulate all the vote totals.
// 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{
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions web/template/status_poll.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 -}}
Expand All @@ -55,6 +58,7 @@
{{- $pollOption.VotesCount }} votes
{{- end -}}
</span>
{{- end -}}
</div>
</li>
{{- end }}
Expand Down

0 comments on commit ac48192

Please sign in to comment.