NotificationTemplate Schema and Repository Improvements (#23769)

* NotificationTemplate Schema and Repository Improvements * Update generated TypeScript types --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Aniket Katkar <aniketkatkar97@gmail.com>
2025-11-01 19:18:05 +00:00 · 2025-10-10 14:49:33 +02:00 · 2025-10-10 14:49:33 +02:00 · 0b107569d7
commit 0b107569d7
parent 13ab5af508
5 changed files with 93 additions and 89 deletions
--- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/NotificationTemplateRepository.java
+++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/NotificationTemplateRepository.java
@ -14,6 +14,7 @@
 package org.openmetadata.service.jdbi3;

 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 import lombok.extern.slf4j.Slf4j;
@ -24,6 +25,7 @@ import org.openmetadata.schema.entity.events.NotificationTemplate;
 import org.openmetadata.schema.type.Include;
 import org.openmetadata.schema.type.ProviderType;
 import org.openmetadata.schema.type.change.ChangeSource;
+import org.openmetadata.schema.utils.JsonUtils;
 import org.openmetadata.service.Entity;
 import org.openmetadata.service.resources.events.NotificationTemplateResource;
 import org.openmetadata.service.template.NotificationTemplateProcessor;
@ -62,29 +64,23 @@ public class NotificationTemplateRepository extends EntityRepository<Notificatio

  @Override
  public void prepare(NotificationTemplate entity, boolean update) {
-    // Validate template if body is present
-    if (entity.getTemplateBody() != null) {
-      NotificationTemplateValidationRequest request = new NotificationTemplateValidationRequest();
-      request.setTemplateBody(entity.getTemplateBody());
-      request.setTemplateSubject(entity.getTemplateSubject());
+    NotificationTemplateValidationRequest request = new NotificationTemplateValidationRequest();
+    request.setTemplateBody(entity.getTemplateBody());
+    request.setTemplateSubject(entity.getTemplateSubject());

-      NotificationTemplateValidationResponse response = templateProcessor.validate(request);
+    NotificationTemplateValidationResponse response = templateProcessor.validate(request);

-      // Check for validation errors
-      StringBuilder errors = new StringBuilder();
-      if (response.getTemplateBody() != null && !response.getTemplateBody().getPassed()) {
-        errors.append("Template body: ").append(response.getTemplateBody().getError());
-      }
-      if (response.getTemplateSubject() != null && !response.getTemplateSubject().getPassed()) {
-        if (errors.length() > 0) {
-          errors.append("; ");
-        }
-        errors.append("Template subject: ").append(response.getTemplateSubject().getError());
-      }
+    // Check for validation errors
+    List<String> errors = new ArrayList<>();
+    if (response.getTemplateBody() != null && !response.getTemplateBody().getPassed()) {
+      errors.add("Template body: " + response.getTemplateBody().getError());
+    }
+    if (response.getTemplateSubject() != null && !response.getTemplateSubject().getPassed()) {
+      errors.add("Template subject: " + response.getTemplateSubject().getError());
+    }

-      if (errors.length() > 0) {
-        throw new IllegalArgumentException("Invalid template: " + errors);
-      }
+    if (!errors.isEmpty()) {
+      throw new IllegalArgumentException("Invalid template: " + String.join("; ", errors));
    }
  }

@ -119,70 +115,80 @@ public class NotificationTemplateRepository extends EntityRepository<Notificatio
    for (NotificationTemplate seedTemplate : seedTemplates) {
      String fqn = seedTemplate.getFullyQualifiedName();
      NotificationTemplate existing = findByNameOrNull(fqn, Include.ALL);
-      String seedChecksum = calculateTemplateChecksum(seedTemplate);

      if (existing == null) {
-        seedTemplate.withIsModifiedFromDefault(false).withDefaultTemplateChecksum(seedChecksum);
-        initializeEntity(seedTemplate);
-        LOG.info("Created new system template {}", fqn);
-        continue;
+        createSystemTemplateFromSeed(seedTemplate, fqn);
+      } else if (shouldUpdateSystemTemplate(existing, seedTemplate)) {
+        updateSystemTemplateFromSeed(existing, seedTemplate, fqn);
+      } else {
+        LOG.debug("Skipping template {} - user template or modified system template", fqn);
      }
+    }
+  }

-      if (ProviderType.SYSTEM.equals(existing.getProvider())
-          && Boolean.FALSE.equals(existing.getIsModifiedFromDefault())
-          && !Objects.equals(seedChecksum, existing.getDefaultTemplateChecksum())) {
-        // Only update functional fields for unmodified templates (hybrid approach)
-        // This preserves any user-customized displayName or description
-        existing
-            .withTemplateBody(seedTemplate.getTemplateBody())
-            .withTemplateSubject(seedTemplate.getTemplateSubject())
-            .withDefaultTemplateChecksum(seedChecksum);
-        store(existing, true);
-        LOG.info("Updated system template {} to new version", existing.getFullyQualifiedName());
-        continue;
-      }
+  private void createSystemTemplateFromSeed(NotificationTemplate seedTemplate, String fqn)
+      throws IOException {
+    seedTemplate.withIsModifiedFromDefault(false);
+    try {
+      initializeEntity(seedTemplate);
+    } catch (IllegalArgumentException e) {
+      throw new IOException(
+          String.format("Failed to validate seed template '%s': %s", fqn, e.getMessage()), e);
+    }
+  }

-      LOG.debug(
-          "Skipping template {} - either user template or modified system template",
-          existing.getFullyQualifiedName());
+  private boolean shouldUpdateSystemTemplate(
+      NotificationTemplate existing, NotificationTemplate seedTemplate) {
+    // Update only if system template has not been update and seed content differs from current
+    // content
+    return ProviderType.SYSTEM.equals(existing.getProvider())
+        && Boolean.FALSE.equals(existing.getIsModifiedFromDefault())
+        && !Objects.equals(
+            calculateTemplateChecksum(seedTemplate), calculateTemplateChecksum(existing));
+  }
+
+  private void updateSystemTemplateFromSeed(
+      NotificationTemplate existing, NotificationTemplate seedTemplate, String fqn)
+      throws IOException {
+    existing
+        .withTemplateBody(seedTemplate.getTemplateBody())
+        .withTemplateSubject(seedTemplate.getTemplateSubject());
+    try {
+      prepare(existing, true);
+      store(existing, true);
+    } catch (IllegalArgumentException e) {
+      throw new IOException(
+          String.format("Failed to validate seed template '%s': %s", fqn, e.getMessage()), e);
    }
  }

  public void resetToDefault(String fqn) {
    try {
-      NotificationTemplate template = getByName(null, fqn, getFields("*"));
-      if (template == null) {
+      NotificationTemplate original = getByName(null, fqn, getFields("*"));
+      if (original == null) {
        throw new IllegalArgumentException("NotificationTemplate not found: " + fqn);
      }

-      if (!ProviderType.SYSTEM.equals(template.getProvider())) {
+      if (!ProviderType.SYSTEM.equals(original.getProvider())) {
        throw new IllegalArgumentException(
            "Cannot reset template: only SYSTEM templates can be reset to default");
      }

-      String seedPath =
-          ResourcePathResolver.getResourcePath(NotificationTemplateResourcePathProvider.class);
-      List<NotificationTemplate> defaultTemplates = getEntitiesFromSeedData(seedPath);
-      NotificationTemplate defaultTemplate =
-          defaultTemplates.stream()
-              .filter(t -> fqn.equals(t.getFullyQualifiedName()))
-              .findFirst()
-              .orElseThrow(
-                  () ->
-                      new IllegalArgumentException(
-                          "Default template not found in seed data: " + fqn));
-
-      template
+      NotificationTemplate defaultTemplate = getDefaultTemplateFromSeed(fqn);
+      NotificationTemplate updated = JsonUtils.deepCopy(original, NotificationTemplate.class);
+      updated
          .withTemplateBody(defaultTemplate.getTemplateBody())
          .withTemplateSubject(defaultTemplate.getTemplateSubject())
          .withDescription(defaultTemplate.getDescription())
-          .withDisplayName(defaultTemplate.getDisplayName())
-          .withIsModifiedFromDefault(false)
-          .withDefaultTemplateChecksum(calculateTemplateChecksum(defaultTemplate));
+          .withDisplayName(defaultTemplate.getDisplayName());

-      store(template, true);
+      EntityUpdater entityUpdater = getUpdater(original, updated, Operation.PUT, null);
+      entityUpdater.update();

      LOG.info("Reset NotificationTemplate {} to default", fqn);
+    } catch (IllegalArgumentException e) {
+      LOG.error("Failed to reset template: {}", e.getMessage(), e);
+      throw e;
    } catch (IOException e) {
      LOG.error("Failed to load seed data for reset operation", e);
      throw new RuntimeException("Failed to reset template due to seed data loading error", e);
@ -201,6 +207,17 @@ public class NotificationTemplateRepository extends EntityRepository<Notificatio
    return templateProcessor.validate(request);
  }

+  private NotificationTemplate getDefaultTemplateFromSeed(String fqn) throws IOException {
+    String seedPath =
+        ResourcePathResolver.getResourcePath(NotificationTemplateResourcePathProvider.class);
+    List<NotificationTemplate> defaultTemplates = getEntitiesFromSeedData(seedPath);
+    return defaultTemplates.stream()
+        .filter(t -> fqn.equals(t.getFullyQualifiedName()))
+        .findFirst()
+        .orElseThrow(
+            () -> new IllegalArgumentException("Default template not found in seed data: " + fqn));
+  }
+
  private String calculateTemplateChecksum(NotificationTemplate template) {
    // Only include functional fields in checksum
    // This allows users to customize displayName and description without triggering "modified"
@ -221,18 +238,21 @@ public class NotificationTemplateRepository extends EntityRepository<Notificatio

    @Override
    protected void entitySpecificUpdate(boolean consolidatingChanges) {
-      // Record template-specific content changes for versioning and rollback
      recordChange("templateBody", original.getTemplateBody(), updated.getTemplateBody());
      recordChange("templateSubject", original.getTemplateSubject(), updated.getTemplateSubject());

-      // Update modification tracking WITHOUT recording (metadata management)
      if (ProviderType.SYSTEM.equals(original.getProvider())) {
-        // Calculate current template checksum
-        String currentChecksum = calculateTemplateChecksum(updated);
-        String defaultChecksum = original.getDefaultTemplateChecksum();
+        try {
+          NotificationTemplate defaultTemplate =
+              getDefaultTemplateFromSeed(original.getFullyQualifiedName());

-        // Set modification status based on checksum comparison
-        updated.setIsModifiedFromDefault(!Objects.equals(currentChecksum, defaultChecksum));
+          String currentChecksum = calculateTemplateChecksum(updated);
+          String defaultChecksum = calculateTemplateChecksum(defaultTemplate);
+
+          updated.setIsModifiedFromDefault(!Objects.equals(currentChecksum, defaultChecksum));
+        } catch (IOException | IllegalArgumentException e) {
+          LOG.warn("Failed to load seed data for modification tracking", e);
+        }
      }
    }
  }
--- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/events/NotificationTemplateMapper.java
+++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/events/NotificationTemplateMapper.java
@ -2,6 +2,7 @@ package org.openmetadata.service.resources.events;

 import org.openmetadata.schema.api.events.CreateNotificationTemplate;
 import org.openmetadata.schema.entity.events.NotificationTemplate;
+import org.openmetadata.schema.type.ProviderType;
 import org.openmetadata.service.mapper.EntityMapper;

 public class NotificationTemplateMapper
@ -11,6 +12,7 @@ public class NotificationTemplateMapper
  public NotificationTemplate createToEntity(CreateNotificationTemplate create, String user) {
    return copy(new NotificationTemplate(), create, user)
        .withTemplateSubject(create.getTemplateSubject())
-        .withTemplateBody(create.getTemplateBody());
+        .withTemplateBody(create.getTemplateBody())
+        .withProvider(ProviderType.USER);
  }
 }
--- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/events/NotificationTemplateResourceTest.java
+++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/events/NotificationTemplateResourceTest.java
@ -586,7 +586,6 @@ public class NotificationTemplateResourceTest
      "system-notification-entity-deleted",
      "system-notification-entity-soft-deleted",
      "system-notification-entity-default",
-      "system-notification-test-result",
      "system-notification-logical-test-case-added",
      "system-notification-task-closed",
      "system-notification-task-resolved"
@ -646,8 +645,6 @@ public class NotificationTemplateResourceTest
    assertFalse(
        systemTemplate.getIsModifiedFromDefault(),
        "Fresh template should not be marked as modified");
-    assertNotNull(
-        systemTemplate.getDefaultTemplateChecksum(), "Template should have a default checksum");
  }

  @Test
@ -661,7 +658,6 @@ public class NotificationTemplateResourceTest
    assertFalse(
        systemTemplate.getIsModifiedFromDefault(),
        "Fresh template should not be marked as modified");
-    String originalChecksum = systemTemplate.getDefaultTemplateChecksum();

    // User modifies the template using PATCH (following existing test patterns)
    String newBody = "<div>User modified template: {{entity.name}}</div>";
@ -678,10 +674,6 @@ public class NotificationTemplateResourceTest
        modifiedTemplate.getIsModifiedFromDefault(),
        "Template should be marked as modified after change");
    assertEquals(newBody, modifiedTemplate.getTemplateBody());
-    assertEquals(
-        originalChecksum,
-        modifiedTemplate.getDefaultTemplateChecksum(),
-        "Default checksum should be preserved");
  }

  @Test
@ -731,8 +723,6 @@ public class NotificationTemplateResourceTest
    assertEquals(originalSubject, resetResult.getTemplateSubject());
    assertEquals(originalDescription, resetResult.getDescription());
    assertEquals(originalDisplayName, resetResult.getDisplayName());
-    assertNotNull(
-        resetResult.getDefaultTemplateChecksum(), "Template should have checksum after reset");
  }

  @Test
--- a/openmetadata-spec/src/main/resources/json/schema/entity/events/notificationTemplate.json
+++ b/openmetadata-spec/src/main/resources/json/schema/entity/events/notificationTemplate.json
@ -79,15 +79,12 @@
      "description": "Indicates if this system template has been modified from its default version. Only applicable to system templates.",
      "type": "boolean",
      "default": false
-    },
-    "defaultTemplateChecksum": {
-      "description": "Checksum of the default template to detect if updates are available. Only applicable to system templates.",
-      "type": "string"
    }
  },
  "required": [
    "id",
    "name",
+    "templateSubject",
    "templateBody"
  ],
  "additionalProperties": false
--- a/openmetadata-ui/src/main/resources/ui/src/generated/entity/events/notificationTemplate.ts
+++ b/openmetadata-ui/src/main/resources/ui/src/generated/entity/events/notificationTemplate.ts
@ -19,11 +19,6 @@ export interface NotificationTemplate {
     * Change that lead to this version of the template.
     */
    changeDescription?: ChangeDescription;
-    /**
-     * Checksum of the default template to detect if updates are available. Only applicable to
-     * system templates.
-     */
-    defaultTemplateChecksum?: string;
    /**
     * When `true` indicates the template has been soft deleted.
     */
@ -74,7 +69,7 @@ export interface NotificationTemplate {
    /**
     * Handlebars template for the email subject line with placeholders.
     */
-    templateSubject?: string;
+    templateSubject: string;
    /**
     * Last update time corresponding to the new version of the template.
     */