Fixes : 14333 - Changed announcements startTime and endTime from seconds to milliseconds. (#16438)

* Improvement: Changed announcements startTime and endTime from seconds to milliseconds.

* Improvement: Changed announcements startTime and endTime from seconds to milliseconds.

* supported announcement milleseconds change from UI

* fix sonar issue

* remove unwanted code

---------

Co-authored-by: Ashish Gupta <ashish@getcollate.io>
Co-authored-by: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com>
This commit is contained in:
Siddhant 2024-06-03 15:22:37 +05:30 committed by GitHub
parent 612de36ffb
commit 7558ccc432
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 264 additions and 43 deletions

View File

@ -31,7 +31,7 @@ public class FeedFilter {
condition1 = String.format("type = '%s'", threadType.value());
if (ThreadType.Announcement.equals(threadType) && activeAnnouncement != null) {
// Add activeAnnouncement filter
long now = System.currentTimeMillis() / 1000; // epoch time in seconds
long now = System.currentTimeMillis(); // epoch time in milliseconds
String condition2 =
activeAnnouncement
? String.format("%s BETWEEN announcementStart AND announcementEnd", now)

View File

@ -36,6 +36,8 @@ import static org.openmetadata.service.jdbi3.UserRepository.TEAMS_FIELD;
import static org.openmetadata.service.util.EntityUtil.compareEntityReference;
import io.jsonwebtoken.lang.Collections;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
@ -112,6 +114,8 @@ public class FeedRepository {
public static final String DELETED_USER_DISPLAY = "User was deleted";
public static final String DELETED_TEAM_NAME = "DeletedTeam";
public static final String DELETED_TEAM_DISPLAY = "Team was deleted";
private static final long MAX_SECONDS_TIMESTAMP = 2147483647L;
private final CollectionDAO dao;
private static final MessageDecorator<FeedMessage> FEED_MESSAGE_FORMATTER =
new FeedMessageDecorator();
@ -864,6 +868,12 @@ public class FeedRepository {
if (startTime >= endTime) {
throw new IllegalArgumentException(ANNOUNCEMENT_INVALID_START_TIME);
}
// Converts start and end times to milliseconds if they are in seconds.
if (startTime <= MAX_SECONDS_TIMESTAMP && endTime <= MAX_SECONDS_TIMESTAMP) {
convertStartAndEndTimeToMilliseconds(thread);
}
// TODO fix this - overlapping announcements should be allowed
List<String> announcements =
dao.feedDAO()
@ -874,6 +884,26 @@ public class FeedRepository {
}
}
private static void convertStartAndEndTimeToMilliseconds(Thread thread) {
Optional.ofNullable(thread.getAnnouncement())
.ifPresent(
announcement -> {
Optional.ofNullable(announcement.getStartTime())
.ifPresent(
startTime ->
announcement.setStartTime(convertSecondsToMilliseconds(startTime)));
Optional.ofNullable(announcement.getEndTime())
.ifPresent(
endTime -> announcement.setEndTime(convertSecondsToMilliseconds(endTime)));
});
}
private static long convertSecondsToMilliseconds(long seconds) {
return LocalDateTime.ofEpochSecond(seconds, 0, ZoneOffset.UTC)
.toInstant(ZoneOffset.UTC)
.toEpochMilli();
}
private void validateAssignee(Thread thread) {
if (thread != null && ThreadType.Task.equals(thread.getType())) {
String createdByUserName = thread.getCreatedBy();

View File

@ -0,0 +1,20 @@
package org.openmetadata.service.migration.mysql.v141;
import static org.openmetadata.service.migration.utils.v141.MigrationUtil.migrateAnnouncementsTimeFormat;
import lombok.SneakyThrows;
import org.openmetadata.service.migration.api.MigrationProcessImpl;
import org.openmetadata.service.migration.utils.MigrationFile;
public class Migration extends MigrationProcessImpl {
public Migration(MigrationFile migrationFile) {
super(migrationFile);
}
@Override
@SneakyThrows
public void runDataMigration() {
migrateAnnouncementsTimeFormat(handle, false);
}
}

View File

@ -0,0 +1,19 @@
package org.openmetadata.service.migration.postgres.v141;
import static org.openmetadata.service.migration.utils.v141.MigrationUtil.migrateAnnouncementsTimeFormat;
import lombok.SneakyThrows;
import org.openmetadata.service.migration.api.MigrationProcessImpl;
import org.openmetadata.service.migration.utils.MigrationFile;
public class Migration extends MigrationProcessImpl {
public Migration(MigrationFile migrationFile) {
super(migrationFile);
}
@Override
@SneakyThrows
public void runDataMigration() {
migrateAnnouncementsTimeFormat(handle, true);
}
}

View File

@ -0,0 +1,102 @@
package org.openmetadata.service.migration.utils.v141;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.util.List;
import java.util.Optional;
import lombok.extern.slf4j.Slf4j;
import org.jdbi.v3.core.Handle;
import org.openmetadata.schema.entity.feed.Thread;
import org.openmetadata.schema.type.AnnouncementDetails;
import org.openmetadata.service.util.JsonUtils;
@Slf4j
public class MigrationUtil {
private static final long MAX_SECONDS_TIMESTAMP = 2147483647L;
public static void migrateAnnouncementsTimeFormat(Handle handle, boolean postgresDbFlag) {
// Fetch all threads of type Announcement from the database
List<Thread> threads = fetchAnnouncementThreads(handle);
threads.forEach(
thread -> {
Optional<AnnouncementDetails> announcementOpt =
Optional.ofNullable(thread.getAnnouncement());
boolean updated =
announcementOpt
.map(
announcement -> {
boolean startUpdated =
Optional.ofNullable(announcement.getStartTime())
.filter(MigrationUtil::isInSeconds)
.map(MigrationUtil::convertSecondsToMilliseconds)
.map(
startTime -> {
announcement.setStartTime(startTime);
return true;
})
.orElse(false);
boolean endUpdated =
Optional.ofNullable(announcement.getEndTime())
.filter(MigrationUtil::isInSeconds)
.map(MigrationUtil::convertSecondsToMilliseconds)
.map(
endTime -> {
announcement.setEndTime(endTime);
return true;
})
.orElse(false);
return startUpdated || endUpdated;
})
.orElse(false);
// If we made any updates, persist the changes back to the database
if (updated) {
if (postgresDbFlag) {
updateThreadPostgres(handle, thread);
} else {
updateThreadMySql(handle, thread);
}
}
});
}
private static List<Thread> fetchAnnouncementThreads(Handle handle) {
String query = "SELECT json FROM thread_entity WHERE type = 'Announcement'";
return handle
.createQuery(query)
.map((rs, ctx) -> JsonUtils.readValue(rs.getString("json"), Thread.class))
.list();
}
private static boolean isInSeconds(Long timestamp) {
return timestamp != null && timestamp <= MAX_SECONDS_TIMESTAMP;
}
private static void updateThreadMySql(Handle handle, Thread thread) {
String updateQuery = "UPDATE thread_entity SET json = :json WHERE id = :id";
handle
.createUpdate(updateQuery)
.bind("json", JsonUtils.pojoToJson(thread))
.bind("id", thread.getId().toString())
.execute();
}
private static void updateThreadPostgres(Handle handle, Thread thread) {
String updateQuery = "UPDATE thread_entity SET json = CAST(:json AS jsonb) WHERE id = :id";
handle
.createUpdate(updateQuery)
.bind("json", JsonUtils.pojoToJson(thread))
.bind("id", thread.getId().toString())
.execute();
}
private static long convertSecondsToMilliseconds(long seconds) {
return LocalDateTime.ofEpochSecond(seconds, 0, ZoneOffset.UTC)
.toInstant(ZoneOffset.UTC)
.toEpochMilli();
}
}

View File

@ -991,8 +991,9 @@ public class FeedResourceTest extends OpenMetadataApplicationTest {
USER.getName(), about, "Announcement One", announcementDetails, USER_AUTH_HEADERS);
String originalJson = JsonUtils.pojoToJson(thread);
long startTs = now.plusDays(6L).toEpochSecond(ZoneOffset.UTC);
long endTs = now.plusDays(7L).toEpochSecond(ZoneOffset.UTC);
long startTs = now.plusDays(6L).toInstant(ZoneOffset.UTC).toEpochMilli();
long endTs = now.plusDays(7L).toInstant(ZoneOffset.UTC).toEpochMilli();
announcementDetails.withStartTime(startTs).withEndTime(endTs);
Thread updated = thread.withAnnouncement(announcementDetails);
@ -1045,8 +1046,8 @@ public class FeedResourceTest extends OpenMetadataApplicationTest {
// create announcement with start time > end time
announcementDetails
.withStartTime(now.plusDays(58L).toEpochSecond(ZoneOffset.UTC))
.withEndTime(now.plusDays(57L).toEpochSecond(ZoneOffset.UTC));
.withStartTime(now.plusDays(58L).toInstant(ZoneOffset.UTC).toEpochMilli())
.withEndTime(now.plusDays(57L).toInstant(ZoneOffset.UTC).toEpochMilli());
Thread updated2 = thread2.withAnnouncement(announcementDetails);
assertResponse(
() -> patchThread(thread2.getId(), originalJson, updated2, USER_AUTH_HEADERS),
@ -1055,8 +1056,8 @@ public class FeedResourceTest extends OpenMetadataApplicationTest {
// create announcement with overlaps
announcementDetails
.withStartTime(now.plusDays(52L).toEpochSecond(ZoneOffset.UTC))
.withEndTime(now.plusDays(56L).toEpochSecond(ZoneOffset.UTC));
.withStartTime(now.plusDays(52L).toInstant(ZoneOffset.UTC).toEpochMilli())
.withEndTime(now.plusDays(56L).toInstant(ZoneOffset.UTC).toEpochMilli());
Thread updated3 = thread2.withAnnouncement(announcementDetails);
assertResponse(
() -> patchThread(thread2.getId(), originalJson, updated3, USER_AUTH_HEADERS),
@ -1064,8 +1065,8 @@ public class FeedResourceTest extends OpenMetadataApplicationTest {
ANNOUNCEMENT_OVERLAP);
announcementDetails
.withStartTime(now.plusDays(53L).plusHours(2L).toEpochSecond(ZoneOffset.UTC))
.withEndTime(now.plusDays(54L).toEpochSecond(ZoneOffset.UTC));
.withStartTime(now.plusDays(53L).plusHours(2L).toInstant(ZoneOffset.UTC).toEpochMilli())
.withEndTime(now.plusDays(54L).toInstant(ZoneOffset.UTC).toEpochMilli());
Thread updated4 = thread2.withAnnouncement(announcementDetails);
assertResponse(
() -> patchThread(thread2.getId(), originalJson, updated4, USER_AUTH_HEADERS),
@ -1073,8 +1074,8 @@ public class FeedResourceTest extends OpenMetadataApplicationTest {
ANNOUNCEMENT_OVERLAP);
announcementDetails
.withStartTime(now.plusDays(52L).plusHours(12L).toEpochSecond(ZoneOffset.UTC))
.withEndTime(now.plusDays(54L).toEpochSecond(ZoneOffset.UTC));
.withStartTime(now.plusDays(52L).plusHours(12L).toInstant(ZoneOffset.UTC).toEpochMilli())
.withEndTime(now.plusDays(54L).toInstant(ZoneOffset.UTC).toEpochMilli());
Thread updated5 = thread2.withAnnouncement(announcementDetails);
assertResponse(
() -> patchThread(thread2.getId(), originalJson, updated5, USER_AUTH_HEADERS),
@ -1082,8 +1083,8 @@ public class FeedResourceTest extends OpenMetadataApplicationTest {
ANNOUNCEMENT_OVERLAP);
announcementDetails
.withStartTime(now.plusDays(54L).plusHours(12L).toEpochSecond(ZoneOffset.UTC))
.withEndTime(now.plusDays(56L).toEpochSecond(ZoneOffset.UTC));
.withStartTime(now.plusDays(54L).plusHours(12L).toInstant(ZoneOffset.UTC).toEpochMilli())
.withEndTime(now.plusDays(56L).toInstant(ZoneOffset.UTC).toEpochMilli());
Thread updated6 = thread2.withAnnouncement(announcementDetails);
assertResponse(
() -> patchThread(thread2.getId(), originalJson, updated6, USER_AUTH_HEADERS),
@ -1980,7 +1981,7 @@ public class FeedResourceTest extends OpenMetadataApplicationTest {
LocalDateTime now = LocalDateTime.now();
return new AnnouncementDetails()
.withDescription(description)
.withStartTime(now.plusDays(start).toEpochSecond(ZoneOffset.UTC))
.withEndTime(now.plusDays(end).toEpochSecond(ZoneOffset.UTC));
.withStartTime(now.plusDays(start).toInstant(ZoneOffset.UTC).toEpochMilli())
.withEndTime(now.plusDays(end).toInstant(ZoneOffset.UTC).toEpochMilli());
}
}

View File

@ -15,6 +15,7 @@ import {
findByTestId,
findByText,
fireEvent,
getByTestId,
render,
} from '@testing-library/react';
import React from 'react';
@ -25,6 +26,10 @@ const mockCancel = jest.fn();
const mockUpdate = jest.fn();
jest.mock('../../../../utils/FeedUtils', () => ({
formatDateTime: jest.fn().mockReturnValue('Jan 1, 1970, 12:00 AM'),
}));
jest.mock('../../../../utils/FeedUtils', () => ({
getFrontEndFormat: jest.fn(),
MarkdownToHTMLConverter: {
@ -67,6 +72,26 @@ describe('Test FeedCardBody component', () => {
expect(messagePreview).toBeInTheDocument();
});
it('Check if FeedCardBody render announcement data', async () => {
const { container } = render(
<FeedCardBody
{...mockFeedCardBodyProps}
announcementDetails={{
description: 'description',
startTime: 1717070243489,
endTime: 1717070248489,
}}
/>,
{
wrapper: MemoryRouter,
}
);
const announcementData = getByTestId(container, 'announcement-data');
expect(announcementData).toBeInTheDocument();
});
it('Should render editor if editpost is true', async () => {
const { container } = render(
<FeedCardBody {...mockFeedCardBodyProps} isEditPost />,

View File

@ -16,7 +16,7 @@ import classNames from 'classnames';
import { isUndefined } from 'lodash';
import React, { FC, useEffect, useMemo, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { formatDateTimeFromSeconds } from '../../../../utils/date-time/DateTimeUtils';
import { formatDateTime } from '../../../../utils/date-time/DateTimeUtils';
import {
getFrontEndFormat,
MarkdownToHTMLConverter,
@ -104,12 +104,12 @@ const FeedCardBody: FC<FeedBodyProp> = ({
<>
<div className={classNames('feed-message', isEditPost ? '' : className)}>
{!isUndefined(announcementDetails) ? (
<Space direction="vertical" size={4}>
<Space data-testid="announcement-data" direction="vertical" size={4}>
<Typography.Text className="feed-body-schedule text-xs text-grey-muted">
{t('label.schedule')}{' '}
{formatDateTimeFromSeconds(announcementDetails.startTime)}{' '}
{formatDateTime(announcementDetails.startTime)}{' '}
{t('label.to-lowercase')}{' '}
{formatDateTimeFromSeconds(announcementDetails.endTime)}
{formatDateTime(announcementDetails.endTime)}{' '}
</Typography.Text>
<Typography.Text className="font-medium">
{postMessage}

View File

@ -192,9 +192,9 @@ const FeedCardBodyV1 = ({
{showSchedule && (
<Typography.Text className="feed-body-schedule text-xs text-grey-muted">
{t('label.schedule')}{' '}
{formatDateTime(announcement.startTime * 1000)}{' '}
{formatDateTime(announcement.startTime)}{' '}
{t('label.to-lowercase')}{' '}
{formatDateTime(announcement.endTime * 1000)}
{formatDateTime(announcement.endTime)}
</Typography.Text>
)}
</Col>

View File

@ -64,8 +64,8 @@ const AddAnnouncementModal: FC<Props> = ({
endTime,
description,
}: CreateAnnouncement) => {
const startTimeMs = startTime.unix();
const endTimeMs = endTime.unix();
const startTimeMs = startTime.valueOf();
const endTimeMs = endTime.valueOf();
if (startTimeMs >= endTimeMs) {
showErrorToast(t('message.announcement-invalid-start-time'));

View File

@ -86,8 +86,8 @@ const EditAnnouncementModal: FC<Props> = ({
initialValues={{
title: announcementTitle,
description: announcement.description,
startTime: moment.unix(announcement.startTime),
endTime: moment.unix(announcement.endTime),
startTime: moment(announcement.startTime),
endTime: moment(announcement.endTime),
}}
layout="vertical"
validateMessages={VALIDATION_MESSAGES}

View File

@ -0,0 +1,35 @@
/*
* Copyright 2024 Collate.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { isActiveAnnouncement } from './AnnouncementsUtils';
describe('Test isActiveAnnouncement utility', () => {
jest.useFakeTimers('modern').setSystemTime(new Date('2024-02-05'));
it('should return true for active announcement', () => {
const result = isActiveAnnouncement(
new Date('2024-02-03').getTime(),
new Date('2024-02-10').getTime()
);
expect(result).toBe(true);
});
it('should return false for inActive announcements', () => {
const result = isActiveAnnouncement(
new Date('2024-02-01').getTime(),
new Date('2024-02-04').getTime()
);
expect(result).toBe(false);
});
});

View File

@ -35,8 +35,14 @@ export const ANNOUNCEMENT_ENTITIES = [
EntityType.SEARCH_SERVICE,
];
/*
@param startTime: number -> Milliseconds
@param endTime: number -> Milliseconds
@returns boolean
*/
export const isActiveAnnouncement = (startTime: number, endTime: number) => {
const currentTime = Date.now() / 1000;
const currentTime = Date.now();
return currentTime > startTime && currentTime < endTime;
};

View File

@ -15,7 +15,6 @@ import {
customFormatDateTime,
formatDate,
formatDateTime,
formatDateTimeFromSeconds,
formatDateTimeLong,
formatTimeDurationFromSeconds,
isValidDateFormat,
@ -47,10 +46,6 @@ describe('DateTimeUtils tests', () => {
expect(formatDate(0)).toBe(`Jan 1, 1970`);
});
it(`formatDateTimeFromSeconds should formate date and time both`, () => {
expect(formatDateTimeFromSeconds(0)).toBe(`Jan 1, 1970, 12:00 AM`);
});
it(`formatDateShort should formate date and time both`, () => {
expect(formatDateTimeLong(0)).toBe(`Thu 1th January, 1970, 12:00 AM`);
});

View File

@ -27,18 +27,6 @@ export const formatDateTime = (date?: number) => {
return dateTime.toLocaleString(DateTime.DATETIME_MED);
};
/**
* @param date EPOCH seconds
* @returns Formatted date for valid input. Format: MMM DD, YYYY, HH:MM AM/PM
*/
export const formatDateTimeFromSeconds = (date?: number) => {
if (isNil(date)) {
return '';
}
return formatDateTime(date * 1000);
};
/**
* @param date EPOCH millis
* @returns Formatted date for valid input. Format: MMM DD, YYYY