From 0958045e27c60596be91ac67a80b30eb778df453 Mon Sep 17 00:00:00 2001
From: Oliver Hamlet <oliver.hamlet@gmail.com>
Date: Mon, 30 Mar 2020 19:39:19 +0100
Subject: [PATCH] Change group handling in MergeMetadata

Only replace the group if one is not already explicitly set. For #64.
---
 src/api/metadata/plugin_metadata.cpp          |  2 +-
 .../internals/metadata/plugin_metadata_test.h | 31 +++++++++++++++++--
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/api/metadata/plugin_metadata.cpp b/src/api/metadata/plugin_metadata.cpp
index 0419c89e..8a1be51f 100644
--- a/src/api/metadata/plugin_metadata.cpp
+++ b/src/api/metadata/plugin_metadata.cpp
@@ -59,7 +59,7 @@ void PluginMetadata::MergeMetadata(const PluginMetadata& plugin) {
   // but if the 'group' value is not explicit, ignore it.
   enabled_ = plugin.IsEnabled();
 
-  if (plugin.GetGroup()) {
+  if (!group_.has_value() && plugin.GetGroup()) {
     group_ = plugin.GetGroup();
   }
 
diff --git a/src/tests/api/internals/metadata/plugin_metadata_test.h b/src/tests/api/internals/metadata/plugin_metadata_test.h
index 7dd73391..83e5daaa 100644
--- a/src/tests/api/internals/metadata/plugin_metadata_test.h
+++ b/src/tests/api/internals/metadata/plugin_metadata_test.h
@@ -140,7 +140,8 @@ TEST_P(PluginMetadataTest,
   EXPECT_FALSE(plugin1.IsEnabled());
 }
 
-TEST_P(PluginMetadataTest, mergeMetadataShouldUseMergedGroupIfItIsExplicit) {
+TEST_P(PluginMetadataTest,
+  mergeMetadataShouldNotUseMergedGroupIfItAndCurrentGroupAreBothExplicit) {
   PluginMetadata plugin1;
   PluginMetadata plugin2;
 
@@ -148,10 +149,22 @@ TEST_P(PluginMetadataTest, mergeMetadataShouldUseMergedGroupIfItIsExplicit) {
   plugin2.SetGroup("group2");
   plugin1.MergeMetadata(plugin2);
 
-  EXPECT_EQ("group2", plugin1.GetGroup());
+  EXPECT_EQ("group1", plugin1.GetGroup());
 }
 
-TEST_P(PluginMetadataTest, mergeMetadataShouldNotUseMergedGroupIfItIsImplicit) {
+TEST_P(PluginMetadataTest,
+       mergeMetadataShouldNotUseMergedGroupIfItAndCurrentGroupAreBothImplicit) {
+  PluginMetadata plugin1;
+  PluginMetadata plugin2;
+
+  plugin1.MergeMetadata(plugin2);
+
+  EXPECT_FALSE(plugin1.GetGroup().has_value());
+}
+
+TEST_P(
+    PluginMetadataTest,
+    mergeMetadataShouldNotUseMergedGroupIfItIsImplicitAndCurrentGroupIsExplicit) {
   PluginMetadata plugin1;
   PluginMetadata plugin2;
 
@@ -161,6 +174,18 @@ TEST_P(PluginMetadataTest, mergeMetadataShouldNotUseMergedGroupIfItIsImplicit) {
   EXPECT_EQ("group1", plugin1.GetGroup());
 }
 
+TEST_P(
+    PluginMetadataTest,
+    mergeMetadataShouldUseMergedGroupIfItIsExplicitAndCurrentGroupIsImplicit) {
+  PluginMetadata plugin1;
+  PluginMetadata plugin2;
+
+  plugin2.SetGroup("group2");
+  plugin1.MergeMetadata(plugin2);
+
+  EXPECT_EQ("group2", plugin1.GetGroup());
+}
+
 TEST_P(PluginMetadataTest, mergeMetadataShouldMergeLoadAfterData) {
   PluginMetadata plugin1;
   PluginMetadata plugin2;