From a67ea160c1202b54ea40c83dfde91df3f64655f6 Mon Sep 17 00:00:00 2001 From: rafmaster7 Date: Tue, 22 Sep 2026 22:26:29 +0200 Subject: [PATCH] fix(badge): do not award a tiered badge below the tier already held ReachAnswerAcceptedAmount awards every badge whose threshold the user has passed. For single award badges that form a ladder this reads as a demotion: a user who was granted the top badge by an administrator receives the lowest one the moment their first answer is accepted, congratulations dialog included. Crossing several thresholds at once grants the whole ladder in one go as well. Single award badges behind this handler now yield only the highest tier reached, and only when that tier is above the one the user already holds. Multi award badges, which every post can earn on its own, are untouched. Unit tests cover picking the tier, the tier already held, and the split between single and multi award badges. --- internal/repo/badge/badge_event_rule.go | 81 ++++++++++++++++- internal/repo/badge/badge_event_rule_test.go | 91 ++++++++++++++++++++ 2 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 internal/repo/badge/badge_event_rule_test.go diff --git a/internal/repo/badge/badge_event_rule.go b/internal/repo/badge/badge_event_rule.go index 786d988ac..94d00e694 100644 --- a/internal/repo/badge/badge_event_rule.go +++ b/internal/repo/badge/badge_event_rule.go @@ -182,17 +182,94 @@ func (br *eventRuleRepo) ReachAnswerAcceptedAmount(ctx context.Context, return nil, errors.InternalServer(reason.DatabaseError).WithError(err).WithStack() } - for _, b := range badges { - // get badge requirement + // Single award badges behind this handler form a ladder: every one of them stands for a level + // the user has reached, so only the highest level reached is awarded, and only when it is above + // the level the user already holds (an administrator can grant one by hand). Without this a user + // who was given the top badge received the lowest one as soon as their first answer was + // accepted, and crossing several thresholds at once granted the whole ladder at a time. + // Multi award badges, which every post can earn on its own, keep the previous behaviour. + tiers, others := splitSingleAwardBadges(badges) + for _, b := range others { requirement := b.GetIntParam("amount") if requirement == 0 || amount < requirement { continue } awards = append(awards, br.createBadgeAward(event.AnswerUserID, event.AnswerID, b)) } + if best := highestTierReached(tiers, amount); best != nil { + held, err := br.highestHeldTier(ctx, event.AnswerUserID, tiers) + if err != nil { + return nil, err + } + if best.GetIntParam("amount") > held { + awards = append(awards, br.createBadgeAward(event.AnswerUserID, event.AnswerID, best)) + } + } return awards, nil } +// splitSingleAwardBadges separates the tiered (single award) badges from the rest +func splitSingleAwardBadges(badges []*entity.Badge) (tiers, others []*entity.Badge) { + for _, b := range badges { + if b.Single == entity.BadgeSingleAward { + tiers = append(tiers, b) + } else { + others = append(others, b) + } + } + return tiers, others +} + +// highestTierReached returns the highest tier whose requirement the user has reached +func highestTierReached(tiers []*entity.Badge, amount int64) (best *entity.Badge) { + for _, b := range tiers { + requirement := b.GetIntParam("amount") + if requirement == 0 || amount < requirement { + continue + } + if best == nil || requirement > best.GetIntParam("amount") { + best = b + } + } + return best +} + +// highestHeldTierOf returns the requirement of the highest tier the user already holds +func highestHeldTierOf(tiers []*entity.Badge, awarded map[string]bool) (held int64) { + for _, b := range tiers { + if !awarded[b.ID] { + continue + } + if requirement := b.GetIntParam("amount"); requirement > held { + held = requirement + } + } + return held +} + +// highestHeldTier reads which tiers the user already holds +func (br *eventRuleRepo) highestHeldTier(ctx context.Context, userID string, tiers []*entity.Badge) (int64, error) { + if userID == "" || len(tiers) == 0 { + return 0, nil + } + ids := make([]string, 0, len(tiers)) + for _, b := range tiers { + ids = append(ids, b.ID) + } + held := make([]*entity.BadgeAward, 0) + err := br.data.DB.Context(ctx).Where("user_id = ?", userID). + And("is_badge_deleted = ?", entity.IsBadgeNotDeleted). + In("badge_id", ids).Find(&held) + if err != nil { + return 0, errors.InternalServer(reason.DatabaseError).WithError(err).WithStack() + } + awarded := make(map[string]bool, len(held)) + for _, a := range held { + awarded[a.BadgeID] = true + } + return highestHeldTierOf(tiers, awarded), nil +} + // ReachAnswerVote reach answer vote func (br *eventRuleRepo) ReachAnswerVote(ctx context.Context, event *schema.EventMsg) (awards []*entity.BadgeAward, err error) { diff --git a/internal/repo/badge/badge_event_rule_test.go b/internal/repo/badge/badge_event_rule_test.go new file mode 100644 index 000000000..49b7a741a --- /dev/null +++ b/internal/repo/badge/badge_event_rule_test.go @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package badge + +import ( + "testing" + + "github.com/apache/answer/internal/entity" + "github.com/stretchr/testify/require" +) + +// a ladder of single award badges: thresholds 1, 6, 15, 80 and 200 accepted answers +func tierBadges() []*entity.Badge { + return []*entity.Badge{ + {ID: "10000000000000001", Name: "first", Single: entity.BadgeSingleAward, Param: `{"amount":"1"}`}, + {ID: "10000000000000002", Name: "second", Single: entity.BadgeSingleAward, Param: `{"amount":"6"}`}, + {ID: "10000000000000003", Name: "third", Single: entity.BadgeSingleAward, Param: `{"amount":"15"}`}, + {ID: "10000000000000004", Name: "fourth", Single: entity.BadgeSingleAward, Param: `{"amount":"80"}`}, + {ID: "10000000000000005", Name: "fifth", Single: entity.BadgeSingleAward, Param: `{"amount":"200"}`}, + } +} + +func TestHighestTierReached_OnlyTheTopTierIsHandedOut(t *testing.T) { + tiers := tierBadges() + + require.Nil(t, highestTierReached(tiers, 0), "no accepted answers, no badge") + require.Equal(t, "first", highestTierReached(tiers, 1).Name) + require.Equal(t, "second", highestTierReached(tiers, 6).Name) + require.Equal(t, "second", highestTierReached(tiers, 14).Name) + require.Equal(t, "third", highestTierReached(tiers, 15).Name, "crossing several thresholds at once gives one badge") + require.Equal(t, "fourth", highestTierReached(tiers, 199).Name) + require.Equal(t, "fifth", highestTierReached(tiers, 200).Name) +} + +func TestHighestHeldTierOf_KnowsTheTierTheUserAlreadyHas(t *testing.T) { + tiers := tierBadges() + + require.Equal(t, int64(0), highestHeldTierOf(tiers, map[string]bool{})) + require.Equal(t, int64(80), highestHeldTierOf(tiers, map[string]bool{"10000000000000004": true}), + "granted by an administrator") + require.Equal(t, int64(80), highestHeldTierOf(tiers, map[string]bool{ + "10000000000000004": true, "10000000000000001": true, + }), "the highest one held counts, not the last one granted") +} + +// The bug itself: a user holding the fourth tier was awarded the first one as soon as an answer of +// theirs was accepted. +func TestTierNotAwardedBelowTheTierAlreadyHeld(t *testing.T) { + tiers := tierBadges() + held := highestHeldTierOf(tiers, map[string]bool{"10000000000000004": true}) + + best := highestTierReached(tiers, 1) // their first answer is accepted + require.Equal(t, "first", best.Name) + require.False(t, best.GetIntParam("amount") > held, "the lower badge must not be awarded") + + best = highestTierReached(tiers, 200) // they reach the top threshold + require.Equal(t, "fifth", best.Name) + require.True(t, best.GetIntParam("amount") > held, "a higher tier is still awarded") + + // and the top tier is not awarded twice + heldTop := highestHeldTierOf(tiers, map[string]bool{"10000000000000005": true}) + require.False(t, highestTierReached(tiers, 200).GetIntParam("amount") > heldTop) +} + +func TestSplitSingleAwardBadges_MultiAwardKeepsOldBehaviour(t *testing.T) { + badges := append(tierBadges(), &entity.Badge{ + ID: "10000000000000006", Name: "per-post", Single: entity.BadgeMultiAward, Param: `{"amount":"3"}`, + }) + + tiers, others := splitSingleAwardBadges(badges) + require.Len(t, tiers, 5) + require.Len(t, others, 1) + require.Equal(t, "per-post", others[0].Name) +}