Fixes [ads] Toggle between Likes and Dislikes in 30-Day history is br…

…oken

components/brave_ads/core/internal/user_engagement/reactions/reactions.cc

@@ -36,8 +36,10 @@ void Reactions::ToggleLikeAd(mojom::ReactionInfoPtr mojom_reaction,
   const mojom::ReactionType toggled_mojom_reaction_type =
       ToggleLikedReactionType(mojom_reaction_type);
   if (toggled_mojom_reaction_type == mojom::ReactionType::kNeutral) {
+    // Neutral reaction, so remove it.
     ad_reactions_.erase(advertiser_id);
   } else {
+    // Non-neutral reaction, so set it.
     ad_reactions_[advertiser_id] = toggled_mojom_reaction_type;
   }
   SetProfileDictPref(prefs::kAdReactions, ReactionMapToDict(ad_reactions_));
@@ -63,8 +65,10 @@ void Reactions::ToggleDislikeAd(mojom::ReactionInfoPtr mojom_reaction,
   const mojom::ReactionType toggled_mojom_reaction_type =
       ToggleDislikedReactionType(mojom_reaction_type);
   if (toggled_mojom_reaction_type == mojom::ReactionType::kNeutral) {
+    // Neutral reaction, so remove it.
     ad_reactions_.erase(advertiser_id);
   } else {
+    // Non-neutral reaction, so set it.
     ad_reactions_[advertiser_id] = toggled_mojom_reaction_type;
   }
   SetProfileDictPref(prefs::kAdReactions, ReactionMapToDict(ad_reactions_));
@@ -82,8 +86,10 @@ mojom::ReactionType Reactions::AdReactionTypeForId(
     const std::string& advertiser_id) const {
   const auto iter = ad_reactions_.find(advertiser_id);
   if (iter == ad_reactions_.cend()) {
+    // No reaction, so neutral.
     return mojom::ReactionType::kNeutral;
   }
+  // Reaction found.
   const auto& [_, reaction_type] = *iter;
   return reaction_type;
 }
@@ -100,8 +106,10 @@ void Reactions::ToggleLikeSegment(mojom::ReactionInfoPtr mojom_reaction,
   const mojom::ReactionType toggled_mojom_reaction_type =
       ToggleLikedReactionType(mojom_reaction_type);
   if (toggled_mojom_reaction_type == mojom::ReactionType::kNeutral) {
+    // Neutral reaction, so remove it.
     segment_reactions_.erase(segment);
   } else {
+    // Non-neutral reaction, so set it.
     segment_reactions_[segment] = toggled_mojom_reaction_type;
   }
   SetProfileDictPref(prefs::kSegmentReactions,
@@ -126,8 +134,10 @@ void Reactions::ToggleDislikeSegment(mojom::ReactionInfoPtr mojom_reaction,
   const mojom::ReactionType toggled_mojom_reaction_type =
       ToggleDislikedReactionType(mojom_reaction_type);
   if (toggled_mojom_reaction_type == mojom::ReactionType::kNeutral) {
+    // Neutral reaction, so remove it.
     segment_reactions_.erase(segment);
   } else {
+    // Non-neutral reaction, so set it.
     segment_reactions_[segment] = toggled_mojom_reaction_type;
   }
   SetProfileDictPref(prefs::kSegmentReactions,
@@ -144,6 +154,7 @@ mojom::ReactionType Reactions::SegmentReactionTypeForId(
     const std::string& segment) const {
   const auto iter = segment_reactions_.find(segment);
   if (iter == segment_reactions_.cend()) {
+    // No reaction, so neutral.
     return mojom::ReactionType::kNeutral;
   }
   const auto [_, reaction_type] = *iter;
@@ -156,10 +167,11 @@ void Reactions::ToggleSaveAd(mojom::ReactionInfoPtr mojom_reaction,
     return std::move(callback).Run(/*success=*/false);
   }
 
-  const auto [iterator, inserted] =
+  const auto [iter, inserted] =
       saved_ads_.insert(mojom_reaction->creative_instance_id);
   if (!inserted) {
-    saved_ads_.erase(iterator);
+    // Already saved, so unsave it.
+    saved_ads_.erase(iter);
   }
   SetProfileListPref(prefs::kSaveAds, ReactionSetToList(saved_ads_));
 
@@ -183,10 +195,11 @@ void Reactions::ToggleMarkAdAsInappropriate(
     return std::move(callback).Run(/*success=*/false);
   }
 
-  const auto [iterator, inserted] =
+  const auto [iter, inserted] =
       marked_as_inappropriate_.insert(mojom_reaction->creative_set_id);
   if (!inserted) {
-    marked_as_inappropriate_.erase(iterator);
+    // Already marked as inappropriate, so unmark it.
+    marked_as_inappropriate_.erase(iter);
   }
   SetProfileListPref(prefs::kMarkedAsInappropriate,
                      ReactionSetToList(marked_as_inappropriate_));

components/brave_ads/core/internal/user_engagement/reactions/reactions_type_util.cc

@@ -5,22 +5,47 @@
 
 #include "brave/components/brave_ads/core/internal/user_engagement/reactions/reactions_type_util.h"
 
+#include "base/notreached.h"
 #include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h"
 
 namespace brave_ads {
 
 mojom::ReactionType ToggleLikedReactionType(
     const mojom::ReactionType mojom_reaction_type) {
-  return mojom_reaction_type == mojom::ReactionType::kNeutral
-             ? mojom::ReactionType::kLiked
-             : mojom::ReactionType::kNeutral;
+  switch (mojom_reaction_type) {
+    case mojom::ReactionType::kNeutral:
+    case mojom::ReactionType::kDisliked: {
+      // If the reaction is neutral or disliked, toggle to liked.
+      return mojom::ReactionType::kLiked;
+    }
+
+    case mojom::ReactionType::kLiked: {
+      // If the reaction is liked, toggle to neutral.
+      return mojom::ReactionType::kNeutral;
+    }
+  }
+
+  NOTREACHED_NORETURN() << "Unexpected value for mojom::ReactionType: "
+                        << mojom_reaction_type;
 }
 
 mojom::ReactionType ToggleDislikedReactionType(
     const mojom::ReactionType mojom_reaction_type) {
-  return mojom_reaction_type == mojom::ReactionType::kNeutral
-             ? mojom::ReactionType::kDisliked
-             : mojom::ReactionType::kNeutral;
+  switch (mojom_reaction_type) {
+    case mojom::ReactionType::kNeutral:
+    case mojom::ReactionType::kLiked: {
+      // If the reaction is neutral or liked, toggle to disliked.
+      return mojom::ReactionType::kDisliked;
+    }
+
+    case mojom::ReactionType::kDisliked: {
+      // If the reaction is disliked, toggle to neutral.
+      return mojom::ReactionType::kNeutral;
+    }
+  }
+
+  NOTREACHED_NORETURN() << "Unexpected value for mojom::ReactionType: "
+                        << mojom_reaction_type;
 }
 
 }  // namespace brave_ads

components/brave_ads/core/internal/user_engagement/reactions/reactions_type_util_unittest.cc

@@ -29,9 +29,9 @@ TEST_F(BraveAdsReactionsTypeUtilTest,
 }
 
 TEST_F(BraveAdsReactionsTypeUtilTest,
-       ToggleLikedReactionTypeFromDislikedToNeutral) {
+       ToggleLikedReactionTypeFromDislikedToLiked) {
   // Act & Assert
-  EXPECT_EQ(mojom::ReactionType::kNeutral,
+  EXPECT_EQ(mojom::ReactionType::kLiked,
             ToggleLikedReactionType(mojom::ReactionType::kDisliked));
 }
 
@@ -50,9 +50,9 @@ TEST_F(BraveAdsReactionsTypeUtilTest,
 }
 
 TEST_F(BraveAdsReactionsTypeUtilTest,
-       ToggleDislikedReactionTypeFromLikedToNeutral) {
+       ToggleDislikedReactionTypeFromLikedToDisliked) {
   // Act & Assert
-  EXPECT_EQ(mojom::ReactionType::kNeutral,
+  EXPECT_EQ(mojom::ReactionType::kDisliked,
             ToggleDislikedReactionType(mojom::ReactionType::kLiked));
 }
 

components/brave_ads/core/internal/user_engagement/reactions/reactions_unittest.cc

@@ -137,7 +137,7 @@ TEST_F(BraveAdsReactionsTest, Ads) {
   const ReactionMap expected_ad_reactions = {
       {test::kAdvertiserId, mojom::ReactionType::kLiked},
       {"2c0577b2-097b-41e8-81db-685de60d26e5", mojom::ReactionType::kDisliked},
-  };
+      {"57b26e2f-2e77-4b31-a3ce-d3faf82dd574", mojom::ReactionType::kDisliked}};
   EXPECT_EQ(expected_ad_reactions, GetReactions().Ads());
 }
 
@@ -218,8 +218,8 @@ TEST_F(BraveAdsReactionsTest, Segments) {
   // Act & Assert
   const ReactionMap expected_segment_reactions = {
       {test::kSegment, mojom::ReactionType::kLiked},
-      {"food & drink", mojom::ReactionType::kDisliked},
-  };
+      {"technology & computing", mojom::ReactionType::kDisliked},
+      {"food & drink", mojom::ReactionType::kDisliked}};
   EXPECT_EQ(expected_segment_reactions, GetReactions().Segments());
 }