Fix #505: Tag remove patch is not working for chart entity; Fix #504: Changing Tier on dashboard does not replace the previously selected Tier (#509)

* Fix #505: Tag remove patch is not working for chart entity; Fix #504: Changing Tier on dashboard does not replace the previously selected Tier

* Fix #505: Tag remove patch is not working for chart entity; Fix #504: Changing Tier on dashboard does not replace the previously selected Tier

catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/ChartRepository.java

@@ -224,7 +224,8 @@ public abstract class ChartRepository {
     EntityReference newOwner = EntityUtil.populateOwner(userDAO(), teamDAO(), updated.getOwner());
 
     EntityReference newService = updated.getService();
-
+    // Remove previous tags. Merge tags from the update and the existing tags
+    EntityUtil.removeTags(tagDAO(), original.getFullyQualifiedName());
     updated.setHref(null);
     updated.setOwner(null);
     updated.setService(null);

catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DashboardRepository.java

@@ -291,6 +291,8 @@ public abstract class DashboardRepository {
     EntityReference newOwner = EntityUtil.populateOwner(userDAO(), teamDAO(), updated.getOwner());
 
     EntityReference newService = updated.getService();
+    // Remove previous tags. Merge tags from the update and the existing tags
+    EntityUtil.removeTags(tagDAO(), original.getFullyQualifiedName());
 
     updated.setHref(null);
     updated.setOwner(null);

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/charts/ChartResourceTest.java

@@ -89,6 +89,8 @@ public class ChartResourceTest extends CatalogApplicationTest {
   public static EntityReference SUPERSET_REFERENCE;
   public static EntityReference LOOKER_REFERENCE;
   public static final TagLabel USER_ADDRESS_TAG_LABEL = new TagLabel().withTagFQN("User.Address");
+  public static final TagLabel TIER_1 = new TagLabel().withTagFQN("Tier.Tier1");
+
 
 
   @BeforeAll
@@ -401,7 +403,7 @@ public class ChartResourceTest extends CatalogApplicationTest {
     assertNull(chart.getDescription());
     assertNull(chart.getOwner());
     assertNotNull(chart.getService());
-    List<TagLabel> chartTags = singletonList(USER_ADDRESS_TAG_LABEL);
+    List<TagLabel> chartTags = List.of(USER_ADDRESS_TAG_LABEL);
 
     chart = getChart(chart.getId(), "service,owner,tags", adminAuthHeaders());
     chart.getService().setHref(null); // href is readonly and not patchable
@@ -410,14 +412,13 @@ public class ChartResourceTest extends CatalogApplicationTest {
     chart = patchChartAttributesAndCheck(chart, "description", TEAM_OWNER1, chartTags, adminAuthHeaders());
     chart.setOwner(TEAM_OWNER1); // Get rid of href and name returned in the response for owner
     chart.setService(SUPERSET_REFERENCE); // Get rid of href and name returned in the response for service
-    chart.setTags(singletonList(USER_ADDRESS_TAG_LABEL));
+    chartTags = List.of(USER_ADDRESS_TAG_LABEL, TIER_1);
-
     // Replace description, tier, owner
     chart = patchChartAttributesAndCheck(chart, "description1", USER_OWNER1, chartTags,
             adminAuthHeaders());
     chart.setOwner(USER_OWNER1); // Get rid of href and name returned in the response for owner
     chart.setService(SUPERSET_REFERENCE); // Get rid of href and name returned in the response for service
-
+    chartTags = List.of(TIER_1);
     // Remove description, tier, owner
     patchChartAttributesAndCheck(chart, null, null, chartTags, adminAuthHeaders());
   }

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java

@@ -36,9 +36,11 @@ import org.openmetadata.catalog.exception.CatalogExceptionMessage;
 import org.openmetadata.catalog.resources.charts.ChartResourceTest;
 import org.openmetadata.catalog.resources.dashboards.DashboardResource.DashboardList;
 import org.openmetadata.catalog.resources.services.DashboardServiceResourceTest;
+import org.openmetadata.catalog.resources.tags.TagResourceTest;
 import org.openmetadata.catalog.resources.teams.TeamResourceTest;
 import org.openmetadata.catalog.resources.teams.UserResourceTest;
 import org.openmetadata.catalog.type.EntityReference;
+import org.openmetadata.catalog.type.TagLabel;
 import org.openmetadata.catalog.util.EntityUtil;
 import org.openmetadata.catalog.util.JsonUtils;
 import org.openmetadata.catalog.util.TestUtils;
@@ -53,7 +55,9 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
+import java.util.stream.Collectors;
 
+import static java.util.Collections.singletonList;
 import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
 import static javax.ws.rs.core.Response.Status.CONFLICT;
 import static javax.ws.rs.core.Response.Status.CREATED;
@@ -81,6 +85,9 @@ public class DashboardResourceTest extends CatalogApplicationTest {
   public static EntityReference SUPERSET_REFERENCE;
   public static EntityReference LOOKER_REFERENCE;
   public static List<EntityReference> CHART_REFERENCES;
+  public static final TagLabel TIER_1 = new TagLabel().withTagFQN("Tier.Tier1");
+  public static final TagLabel USER_ADDRESS_TAG_LABEL = new TagLabel().withTagFQN("User.Address");
+
 
   @BeforeAll
   public static void setup(TestInfo test) throws HttpResponseException {
@@ -437,21 +444,22 @@ public class DashboardResourceTest extends CatalogApplicationTest {
 
     dashboard = getDashboard(dashboard.getId(), "service,owner,usageSummary", adminAuthHeaders());
     dashboard.getService().setHref(null); // href is readonly and not patchable
+    List<TagLabel> dashboardTags = singletonList(TIER_1);
 
     // Add description, owner when previously they were null
     dashboard = patchDashboardAttributesAndCheck(dashboard, "description",
-            TEAM_OWNER1, adminAuthHeaders());
+            TEAM_OWNER1, dashboardTags, adminAuthHeaders());
     dashboard.setOwner(TEAM_OWNER1); // Get rid of href and name returned in the response for owner
     dashboard.setService(SUPERSET_REFERENCE); // Get rid of href and name returned in the response for service
-
+    dashboardTags = singletonList(USER_ADDRESS_TAG_LABEL);
     // Replace description, tier, owner
     dashboard = patchDashboardAttributesAndCheck(dashboard, "description1",
-            USER_OWNER1, adminAuthHeaders());
+            USER_OWNER1, dashboardTags, adminAuthHeaders());
     dashboard.setOwner(USER_OWNER1); // Get rid of href and name returned in the response for owner
     dashboard.setService(SUPERSET_REFERENCE); // Get rid of href and name returned in the response for service
 
     // Remove description, tier, owner
-    patchDashboardAttributesAndCheck(dashboard, null, null, adminAuthHeaders());
+    patchDashboardAttributesAndCheck(dashboard, null, null, dashboardTags, adminAuthHeaders());
   }
 
   @Test
@@ -541,7 +549,8 @@ public class DashboardResourceTest extends CatalogApplicationTest {
                                                   Map<String, String> authHeaders) throws HttpResponseException {
     create.withCharts(charts);
     Dashboard dashboard = createDashboard(create, authHeaders);
-    validateDashboard(dashboard, create.getDescription(), create.getOwner(), create.getService(), charts);
+    validateDashboard(dashboard, create.getDescription(), create.getOwner(), create.getService(), create.getTags(),
+            charts);
     return getAndValidate(dashboard.getId(), create, authHeaders);
   }
 
@@ -643,7 +652,8 @@ public class DashboardResourceTest extends CatalogApplicationTest {
 
   private static Dashboard validateDashboard(Dashboard dashboard, String expectedDescription,
                                              EntityReference expectedOwner, EntityReference expectedService,
-                                              List<EntityReference> charts) {
+                                              List<TagLabel> expectedTags,
+                                              List<EntityReference> charts) throws HttpResponseException {
     assertNotNull(dashboard.getId());
     assertNotNull(dashboard.getHref());
     assertEquals(expectedDescription, dashboard.getDescription());
@@ -663,6 +673,7 @@ public class DashboardResourceTest extends CatalogApplicationTest {
       assertEquals(expectedService.getType(), dashboard.getService().getType());
     }
     validateDashboardCharts(dashboard, charts);
+    validateTags(expectedTags, dashboard.getTags());
     return dashboard;
   }
 
@@ -681,22 +692,45 @@ public class DashboardResourceTest extends CatalogApplicationTest {
     }
   }
 
+  private static void validateTags(List<TagLabel> expectedList, List<TagLabel> actualList)
+          throws HttpResponseException {
+    if (expectedList == null) {
+      return;
+    }
+    // When tags from the expected list is added to an entity, the derived tags for those tags are automatically added
+    // So add to the expectedList, the derived tags before validating the tags
+    List<TagLabel> updatedExpectedList = new ArrayList<>(expectedList);
+    for (TagLabel expected : expectedList) {
+      List<TagLabel> derived = EntityUtil.getDerivedTags(expected, TagResourceTest.getTag(expected.getTagFQN(),
+              adminAuthHeaders()));
+      updatedExpectedList.addAll(derived);
+    }
+    updatedExpectedList = updatedExpectedList.stream().distinct().collect(Collectors.toList());
+
+    assertTrue(actualList.containsAll(updatedExpectedList));
+    assertTrue(updatedExpectedList.containsAll(actualList));
+  }
+
   private Dashboard patchDashboardAttributesAndCheck(Dashboard dashboard, String newDescription,
-                                                EntityReference newOwner, Map<String, String> authHeaders)
+                                                     EntityReference newOwner, List<TagLabel> tags,
+                                                     Map<String, String> authHeaders)
           throws JsonProcessingException, HttpResponseException {
     String dashboardJson = JsonUtils.pojoToJson(dashboard);
 
     // Update the table attributes
     dashboard.setDescription(newDescription);
     dashboard.setOwner(newOwner);
+    dashboard.setTags(tags);
 
     // Validate information returned in patch response has the updates
     Dashboard updatedDashboard = patchDashboard(dashboardJson, dashboard, authHeaders);
-    validateDashboard(updatedDashboard, dashboard.getDescription(), newOwner, null);
+    validateDashboard(updatedDashboard, dashboard.getDescription(), newOwner, null, tags,
+            dashboard.getCharts());
 
     // GET the table and Validate information returned
     Dashboard getDashboard = getDashboard(dashboard.getId(), "service,owner", authHeaders);
-    validateDashboard(getDashboard, dashboard.getDescription(), newOwner, null);
+    validateDashboard(updatedDashboard, dashboard.getDescription(), newOwner, null, tags,
+            dashboard.getCharts());
     return updatedDashboard;
   }