Fix #6587 Backend: Announcement start and end time should not clash with another announcement (#6618)

2025-08-27 10:26:09 +00:00 · 2022-08-06 09:15:56 -07:00 · 2022-08-06 09:15:56 -07:00 · e9d566e497
commit e9d566e497
parent 92ea12dd29
5 changed files with 264 additions and 9 deletions
--- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java
+++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java
@ -146,4 +146,12 @@ public final class CatalogExceptionMessage {
  public static String invalidTeamOwner(TeamType teamType) {
    return String.format("Team of type %s can't own entities. Only Team of type Group can own entities.", teamType);
  }
+
+  public static String announcementOverlap() {
+    return "There is already an announcement scheduled that overlaps with the given start time and end time";
+  }
+
+  public static String announcementInvalidStartTime() {
+    return "Announcement start time must be earlier than the end time";
+  }
 }
--- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java
+++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java
@ -618,6 +618,23 @@ public interface CollectionDAO {
      return listBefore(limit, before, status, resolved, type);
    }

+    default List<String> listAnnouncementBetween(String entityId, long startTs, long endTs) {
+      return listAnnouncementBetween(null, entityId, startTs, endTs);
+    }
+
+    @SqlQuery(
+        "SELECT json FROM thread_entity "
+            + "WHERE type='Announcement' AND (:threadId IS NULL OR id != :threadId) "
+            + "AND entityId = :entityId "
+            + "AND (( :startTs >= announcementStart AND :startTs < announcementEnd) "
+            + "OR (:endTs > announcementStart AND :endTs < announcementEnd) "
+            + "OR (:startTs <= announcementStart AND :endTs >= announcementEnd))")
+    List<String> listAnnouncementBetween(
+        @Bind("threadId") String threadId,
+        @Bind("entityId") String entityId,
+        @Bind("startTs") long startTs,
+        @Bind("endTs") long endTs);
+
    @ConnectionAwareSqlQuery(
        value =
            "SELECT json FROM thread_entity "
--- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java
+++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java
@ -37,6 +37,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicInteger;
@ -68,6 +69,7 @@ import org.openmetadata.catalog.resources.feeds.FeedResource;
 import org.openmetadata.catalog.resources.feeds.FeedUtil;
 import org.openmetadata.catalog.resources.feeds.MessageParser;
 import org.openmetadata.catalog.resources.feeds.MessageParser.EntityLink;
+import org.openmetadata.catalog.type.AnnouncementDetails;
 import org.openmetadata.catalog.type.Column;
 import org.openmetadata.catalog.type.EntityReference;
 import org.openmetadata.catalog.type.Include;
@ -127,6 +129,18 @@ public class FeedRepository {
      thread.withTask(thread.getTask().withId(getNextTaskId()));
    }

+    // if thread is of type "announcement", validate start and end time
+    if (thread.getType().equals(ThreadType.Announcement)) {
+      validateAnnouncement(thread.getAnnouncement());
+      long startTime = thread.getAnnouncement().getStartTime();
+      long endTime = thread.getAnnouncement().getEndTime();
+      List<String> announcements = dao.feedDAO().listAnnouncementBetween(entityId.toString(), startTime, endTime);
+      if (announcements.size() > 0) {
+        // There is already an announcement that overlaps the new one
+        throw new IllegalArgumentException(CatalogExceptionMessage.announcementOverlap());
+      }
+    }
+
    // Insert a new thread
    dao.feedDAO().insert(JsonUtils.pojoToJson(thread));

@ -749,6 +763,21 @@ public class FeedRepository {
      updated.getTask().getAssignees().sort(compareEntityReference);
    }

+    if (updated.getAnnouncement() != null) {
+      validateAnnouncement(updated.getAnnouncement());
+      // check if the announcement start and end time clashes with other existing announcements
+      List<String> announcements =
+          dao.feedDAO()
+              .listAnnouncementBetween(
+                  id.toString(),
+                  updated.getEntityId().toString(),
+                  updated.getAnnouncement().getStartTime(),
+                  updated.getAnnouncement().getEndTime());
+      if (announcements.size() > 0) {
+        throw new IllegalArgumentException(CatalogExceptionMessage.announcementOverlap());
+      }
+    }
+
    // Update the attributes
    String change = patchUpdate(original, updated) ? RestUtil.ENTITY_UPDATED : RestUtil.ENTITY_NO_CHANGE;
    sortPosts(updated);
@ -756,9 +785,15 @@ public class FeedRepository {
    return new PatchResponse<>(Status.OK, updatedHref, change);
  }

+  private void validateAnnouncement(AnnouncementDetails announcementDetails) {
+    if (announcementDetails.getStartTime() >= announcementDetails.getEndTime()) {
+      throw new IllegalArgumentException(CatalogExceptionMessage.announcementInvalidStartTime());
+    }
+  }
+
  private void restorePatchAttributes(Thread original, Thread updated) {
    // Patch can't make changes to following fields. Ignore the changes
-    updated.withId(original.getId()).withAbout(original.getAbout());
+    updated.withId(original.getId()).withAbout(original.getAbout()).withType(original.getType());
  }

  private void restorePatchAttributes(Post original, Post updated) {
@ -810,13 +845,17 @@ public class FeedRepository {
  }

  private boolean fieldsChanged(Thread original, Thread updated) {
-    // Patch supports isResolved, message, task assignees, and reactions for now
+    // Patch supports isResolved, message, task assignees, reactions, and announcements for now
    return !original.getResolved().equals(updated.getResolved())
        || !original.getMessage().equals(updated.getMessage())
        || (Collections.isEmpty(original.getReactions()) && !Collections.isEmpty(updated.getReactions()))
        || (!Collections.isEmpty(original.getReactions()) && Collections.isEmpty(updated.getReactions()))
        || original.getReactions().size() != updated.getReactions().size()
        || !original.getReactions().containsAll(updated.getReactions())
+        || (original.getAnnouncement() != null
+            && (!original.getAnnouncement().getDescription().equals(updated.getAnnouncement().getDescription())
+                || !Objects.equals(original.getAnnouncement().getStartTime(), updated.getAnnouncement().getStartTime())
+                || !Objects.equals(original.getAnnouncement().getEndTime(), updated.getAnnouncement().getEndTime())))
        || (original.getTask() != null
            && (original.getTask().getAssignees().size() != updated.getTask().getAssignees().size()
                || !original.getTask().getAssignees().containsAll(updated.getTask().getAssignees())));
--- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java
+++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java
@ -25,6 +25,8 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.openmetadata.catalog.exception.CatalogExceptionMessage.announcementInvalidStartTime;
+import static org.openmetadata.catalog.exception.CatalogExceptionMessage.announcementOverlap;
 import static org.openmetadata.catalog.exception.CatalogExceptionMessage.entityNotFound;
 import static org.openmetadata.catalog.exception.CatalogExceptionMessage.noPermission;
 import static org.openmetadata.catalog.resources.EntityResourceTest.USER_ADDRESS_TAG_LABEL;
@ -426,8 +428,8 @@ public class FeedResourceTest extends CatalogApplicationTest {
    AnnouncementDetails announcementDetails =
        new AnnouncementDetails()
            .withDescription("First announcement")
-            .withStartTime(now.plusDays(1L).toEpochSecond(ZoneOffset.UTC))
-            .withEndTime(now.plusDays(2L).toEpochSecond(ZoneOffset.UTC));
+            .withStartTime(now.plusDays(10L).toEpochSecond(ZoneOffset.UTC))
+            .withEndTime(now.plusDays(11L).toEpochSecond(ZoneOffset.UTC));
    CreateThread create =
        create()
            .withMessage("Announcement One")
@ -436,6 +438,9 @@ public class FeedResourceTest extends CatalogApplicationTest {
    Map<String, String> userAuthHeaders = authHeaders(USER.getEmail());
    createAndCheck(create, userAuthHeaders);

+    announcementDetails
+        .withStartTime(now.plusDays(12L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(13L).toEpochSecond(ZoneOffset.UTC));
    create =
        create()
            .withMessage("Announcement Two")
@ -445,8 +450,8 @@ public class FeedResourceTest extends CatalogApplicationTest {

    // create one expired announcement
    announcementDetails
-        .withStartTime(now.minusDays(3L).toEpochSecond(ZoneOffset.UTC))
-        .withEndTime(now.minusMinutes(1L).toEpochSecond(ZoneOffset.UTC));
+        .withStartTime(now.minusDays(30L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.minusDays(20L).toEpochSecond(ZoneOffset.UTC));
    create =
        create()
            .withMessage("Announcement Three")
@ -457,8 +462,8 @@ public class FeedResourceTest extends CatalogApplicationTest {
    // create one active announcement
    announcementDetails
        .withDescription("Active Announcement")
-        .withStartTime(now.minusDays(2L).toEpochSecond(ZoneOffset.UTC))
-        .withEndTime(now.plusDays(5L).toEpochSecond(ZoneOffset.UTC));
+        .withStartTime(now.minusDays(1L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(1L).toEpochSecond(ZoneOffset.UTC));
    create =
        create()
            .withMessage("Announcement Four")
@ -497,6 +502,65 @@ public class FeedResourceTest extends CatalogApplicationTest {
    assertEquals(totalAnnouncementCount + 3, announcements.getData().size());
  }

+  @Test
+  void post_invalidAnnouncement_400() throws IOException {
+    // create two announcements with same start time in the future
+    LocalDateTime now = LocalDateTime.now();
+    AnnouncementDetails announcementDetails =
+        new AnnouncementDetails()
+            .withDescription("First announcement")
+            .withStartTime(now.plusDays(3L).toEpochSecond(ZoneOffset.UTC))
+            .withEndTime(now.plusDays(5L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create =
+        create()
+            .withMessage("Announcement One")
+            .withType(ThreadType.Announcement)
+            .withAnnouncementDetails(announcementDetails);
+    Map<String, String> userAuthHeaders = authHeaders(USER.getEmail());
+    createAndCheck(create, userAuthHeaders);
+
+    CreateThread create2 =
+        create()
+            .withMessage("Announcement Two")
+            .withType(ThreadType.Announcement)
+            .withAnnouncementDetails(announcementDetails);
+
+    // create announcement with same start and end time
+    assertResponse(() -> createThread(create2, userAuthHeaders), BAD_REQUEST, announcementOverlap());
+
+    // create announcement with start time > end time
+    announcementDetails
+        .withStartTime(now.plusDays(3L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(2L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create3 = create2.withAnnouncementDetails(announcementDetails);
+    assertResponse(() -> createThread(create3, userAuthHeaders), BAD_REQUEST, announcementInvalidStartTime());
+
+    // create announcement with overlaps
+    announcementDetails
+        .withStartTime(now.plusDays(2L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(6L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create4 = create2.withAnnouncementDetails(announcementDetails);
+    assertResponse(() -> createThread(create4, userAuthHeaders), BAD_REQUEST, announcementOverlap());
+
+    announcementDetails
+        .withStartTime(now.plusDays(3L).plusHours(2L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(4L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create5 = create2.withAnnouncementDetails(announcementDetails);
+    assertResponse(() -> createThread(create5, userAuthHeaders), BAD_REQUEST, announcementOverlap());
+
+    announcementDetails
+        .withStartTime(now.plusDays(2L).plusHours(12L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(4L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create6 = create2.withAnnouncementDetails(announcementDetails);
+    assertResponse(() -> createThread(create6, userAuthHeaders), BAD_REQUEST, announcementOverlap());
+
+    announcementDetails
+        .withStartTime(now.plusDays(4L).plusHours(12L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(6L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create7 = create2.withAnnouncementDetails(announcementDetails);
+    assertResponse(() -> createThread(create7, userAuthHeaders), BAD_REQUEST, announcementOverlap());
+  }
+
  @Test
  void put_resolveTask_description_200() throws IOException {
    CreateTaskDetails taskDetails =
@ -845,6 +909,133 @@ public class FeedResourceTest extends CatalogApplicationTest {
    assertEquals(TEST_USER_NAME, patched.getUpdatedBy());
  }

+  @Test
+  void patch_announcement_200() throws IOException {
+    LocalDateTime now = LocalDateTime.now();
+    AnnouncementDetails announcementDetails =
+        new AnnouncementDetails()
+            .withDescription("First announcement")
+            .withStartTime(now.plusDays(5L).toEpochSecond(ZoneOffset.UTC))
+            .withEndTime(now.plusDays(6L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create =
+        create()
+            .withMessage("Announcement One")
+            .withType(ThreadType.Announcement)
+            .withAnnouncementDetails(announcementDetails);
+    Map<String, String> userAuthHeaders = authHeaders(USER.getEmail());
+    Thread thread = createAndCheck(create, userAuthHeaders);
+
+    String originalJson = JsonUtils.pojoToJson(thread);
+
+    long startTs = now.plusDays(6L).toEpochSecond(ZoneOffset.UTC);
+    long endTs = now.plusDays(7L).toEpochSecond(ZoneOffset.UTC);
+    announcementDetails.withStartTime(startTs).withEndTime(endTs);
+    Thread updated = thread.withAnnouncement(announcementDetails);
+
+    Thread patched = patchThreadAndCheck(updated, originalJson, TEST_AUTH_HEADERS);
+
+    assertNotEquals(patched.getUpdatedAt(), thread.getUpdatedAt());
+    assertEquals(TEST_USER_NAME, patched.getUpdatedBy());
+    assertEquals(startTs, patched.getAnnouncement().getStartTime());
+    assertEquals(endTs, patched.getAnnouncement().getEndTime());
+
+    Thread thread1 = getThread(thread.getId(), ADMIN_AUTH_HEADERS);
+    assertEquals(startTs, thread1.getAnnouncement().getStartTime());
+    assertEquals(endTs, thread1.getAnnouncement().getEndTime());
+
+    // patch description
+    originalJson = JsonUtils.pojoToJson(thread1);
+    AnnouncementDetails announcementDetails1 = thread1.getAnnouncement().withDescription("New Description");
+    updated = thread1.withAnnouncement(announcementDetails1);
+    patched = patchThreadAndCheck(updated, originalJson, TEST_AUTH_HEADERS);
+    assertEquals("New Description", patched.getAnnouncement().getDescription());
+
+    Thread thread2 = getThread(thread.getId(), ADMIN_AUTH_HEADERS);
+    assertEquals("New Description", thread2.getAnnouncement().getDescription());
+  }
+
+  @Test
+  void patch_invalidAnnouncement_400() throws IOException {
+    LocalDateTime now = LocalDateTime.now();
+    AnnouncementDetails announcementDetails =
+        new AnnouncementDetails()
+            .withDescription("First announcement")
+            .withStartTime(now.plusDays(53L).toEpochSecond(ZoneOffset.UTC))
+            .withEndTime(now.plusDays(55L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create =
+        create()
+            .withMessage("Announcement One")
+            .withType(ThreadType.Announcement)
+            .withAnnouncementDetails(announcementDetails);
+    Map<String, String> userAuthHeaders = authHeaders(USER.getEmail());
+    Thread thread1 = createAndCheck(create, userAuthHeaders);
+
+    announcementDetails
+        .withStartTime(now.plusDays(57L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(59L).toEpochSecond(ZoneOffset.UTC));
+    CreateThread create2 =
+        create()
+            .withMessage("Announcement Two")
+            .withType(ThreadType.Announcement)
+            .withAnnouncementDetails(announcementDetails);
+    Thread thread2 = createAndCheck(create, userAuthHeaders);
+
+    String originalJson = JsonUtils.pojoToJson(thread2);
+
+    // patch announcement2 with same start and end time as announcement1
+    Thread updated = thread2.withAnnouncement(thread1.getAnnouncement());
+
+    assertResponse(
+        () -> patchThread(thread2.getId(), originalJson, updated, userAuthHeaders), BAD_REQUEST, announcementOverlap());
+
+    // create announcement with start time > end time
+    announcementDetails
+        .withStartTime(now.plusDays(58L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(57L).toEpochSecond(ZoneOffset.UTC));
+    Thread updated2 = thread2.withAnnouncement(announcementDetails);
+    assertResponse(
+        () -> patchThread(thread2.getId(), originalJson, updated2, userAuthHeaders),
+        BAD_REQUEST,
+        announcementInvalidStartTime());
+
+    // create announcement with overlaps
+    announcementDetails
+        .withStartTime(now.plusDays(52L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(56L).toEpochSecond(ZoneOffset.UTC));
+    Thread updated3 = thread2.withAnnouncement(announcementDetails);
+    assertResponse(
+        () -> patchThread(thread2.getId(), originalJson, updated3, userAuthHeaders),
+        BAD_REQUEST,
+        announcementOverlap());
+
+    announcementDetails
+        .withStartTime(now.plusDays(53L).plusHours(2L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(54L).toEpochSecond(ZoneOffset.UTC));
+    Thread updated4 = thread2.withAnnouncement(announcementDetails);
+    assertResponse(
+        () -> patchThread(thread2.getId(), originalJson, updated4, userAuthHeaders),
+        BAD_REQUEST,
+        announcementOverlap());
+
+    announcementDetails
+        .withStartTime(now.plusDays(52L).plusHours(12L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(54L).toEpochSecond(ZoneOffset.UTC));
+    Thread updated5 = thread2.withAnnouncement(announcementDetails);
+    assertResponse(
+        () -> patchThread(thread2.getId(), originalJson, updated5, userAuthHeaders),
+        BAD_REQUEST,
+        announcementOverlap());
+
+    announcementDetails
+        .withStartTime(now.plusDays(54L).plusHours(12L).toEpochSecond(ZoneOffset.UTC))
+        .withEndTime(now.plusDays(56L).toEpochSecond(ZoneOffset.UTC));
+    Thread updated6 = thread2.withAnnouncement(announcementDetails);
+    assertResponse(
+        () -> patchThread(thread2.getId(), originalJson, updated6, userAuthHeaders),
+        BAD_REQUEST,
+        announcementOverlap());
+  }
+
  @Test
  void patch_thread_not_allowed_fields() throws IOException {
    // create a thread
--- a/openmetadata-ui/src/main/resources/ui/src/components/Modals/AnnouncementModal/AddAnnouncementModal.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/components/Modals/AnnouncementModal/AddAnnouncementModal.tsx
@ -97,7 +97,7 @@ const AddAnnouncementModal: FC<Props> = ({
        type: 'primary',
        htmlType: 'submit',
      }}
-      okText="Sumbit"
+      okText="Submit"
      title="Make an announcement"
      visible={open}
      width={620}