From dff932c9b3c31e41b99382fcba8f78ab611a6ace Mon Sep 17 00:00:00 2001 From: rafmaster7 Date: Tue, 22 Sep 2026 22:33:43 +0200 Subject: [PATCH] fix(notification): the badge congratulations dialog can always be dismissed Closing the dialog calls PUT /notification/read/state, which returned success while doing nothing whenever the notification row behind the alert was gone or belonged to somebody else: the guard returned early and the alert stayed in the red dot cache, so /notification/status kept serving the same badge_award and the dialog came back on every page load with no way to close it. A revoked badge deletes its notification, which is exactly how the rows go missing. The alert now leaves the cache before the notification row is looked up. The cache key is built from the caller's own user id, so a request can only ever clear the caller's own alert. The test fails on the current code and passes with the fix. --- .../service/notification/badge_alert_test.go | 77 +++++++++++++++++++ .../notification/notification_service.go | 14 ++-- 2 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 internal/service/notification/badge_alert_test.go diff --git a/internal/service/notification/badge_alert_test.go b/internal/service/notification/badge_alert_test.go new file mode 100644 index 000000000..7b19c96bd --- /dev/null +++ b/internal/service/notification/badge_alert_test.go @@ -0,0 +1,77 @@ +/* + * 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 notification + +import ( + "context" + "fmt" + "testing" + + "github.com/apache/answer/internal/base/constant" + basedata "github.com/apache/answer/internal/base/data" + "github.com/apache/answer/internal/entity" + "github.com/apache/answer/internal/service/noticequeue" + notficationcommon "github.com/apache/answer/internal/service/notification_common" +) + +// badgeAlertTestRepo a notification repository where the row behind the alert is gone +type badgeAlertTestRepo struct { + notficationcommon.NotificationRepo +} + +func (r *badgeAlertTestRepo) GetById(ctx context.Context, id string) (*entity.Notification, bool, error) { + return &entity.Notification{}, false, nil +} + +// The alert that the badge dialog reads lives in the cache. When the notification row behind it is +// gone - a revoked badge deletes it - dismissing the dialog used to return success while leaving the +// alert in place, so the dialog came back on every page load and could not be closed. +func TestClearIDUnRead_DropsTheBadgeAlertWhenTheNotificationIsGone(t *testing.T) { + cache, cleanup, err := basedata.NewCache(&basedata.CacheConf{}) + if err != nil { + t.Fatalf("new cache: %v", err) + } + t.Cleanup(cleanup) + + data := &basedata.Data{Cache: cache} + common := notficationcommon.NewNotificationCommon(data, nil, nil, nil, nil, nil, noticequeue.NewService(), nil, nil) + service := &NotificationService{ + data: data, + notificationRepo: &badgeAlertTestRepo{}, + notificationCommon: common, + } + + ctx := context.TODO() + const userID, notificationID = "10000000000000001", "10000000000000002" + if err := common.AddBadgeAwardAlertCache(ctx, userID, notificationID, "10000000000000003"); err != nil { + t.Fatalf("add badge award alert: %v", err) + } + + if err := service.ClearIDUnRead(ctx, userID, notificationID); err != nil { + t.Fatalf("clear notification: %v", err) + } + + key := fmt.Sprintf(constant.RedDotCacheKey, constant.NotificationTypeBadgeAchievement, userID) + if _, exist, err := cache.GetString(ctx, key); err != nil { + t.Fatalf("read cache: %v", err) + } else if exist { + t.Fatal("the badge alert is still there, the dialog would come back") + } +} diff --git a/internal/service/notification/notification_service.go b/internal/service/notification/notification_service.go index 6a69cbaef..09f6ea9a8 100644 --- a/internal/service/notification/notification_service.go +++ b/internal/service/notification/notification_service.go @@ -182,6 +182,15 @@ func (ns *NotificationService) ClearUnRead(ctx context.Context, userID string, n } func (ns *NotificationService) ClearIDUnRead(ctx context.Context, userID string, id string) error { + // The badge alert lives in the cache, the notification row does not have to: it may have been + // deleted, or the notification may already be read. Both cases used to leave the alert behind, + // and the congratulations dialog then came back on every page load, so drop the alert first. + // The cache key is built from the caller's own user id, so this can only ever clear the + // caller's own alert. + if err := ns.notificationCommon.RemoveBadgeAwardAlertCache(ctx, userID, id); err != nil { + log.Errorf("remove badge award alert cache failed: %v", err) + } + notificationInfo, exist, err := ns.notificationRepo.GetById(ctx, id) if err != nil { log.Errorf("get notification failed: %v", err) @@ -197,11 +206,6 @@ func (ns *NotificationService) ClearIDUnRead(ctx context.Context, userID string, } } - err = ns.notificationCommon.RemoveBadgeAwardAlertCache(ctx, userID, id) - if err != nil { - log.Errorf("remove badge award alert cache failed: %v", err) - } - _ = ns.notificationCommon.DecreaseRedDot(ctx, userID, notificationInfo.Type) return nil }