Skip to content

Commit de4cf10

Browse files
jjoonleojjoonleo
authored
Merge pull request #328 from DevKor-github/feature/fix-duplicate-schedule-notifications-dev
[codex] Fix duplicate schedule notifications
2 parents 580c8af + 2254565 commit de4cf10

6 files changed

Lines changed: 267 additions & 6 deletions

File tree

‎ontime-back/src/main/java/devkor/ontime_back/repository/NotificationScheduleRepository.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import java.time.LocalDateTime;
99
import java.util.List;
10-
import java.util.Optional;
1110
import java.util.UUID;
1211

1312
@Repository
@@ -18,5 +17,5 @@ public interface NotificationScheduleRepository extends JpaRepository<Notificati
1817
"WHERE n.notificationTime > :now AND n.isSent = false")
1918
List<NotificationSchedule> findAllWithScheduleAndUser(LocalDateTime now);
2019

21-
Optional<NotificationSchedule> findByScheduleScheduleId(UUID scheduleId);
20+
List<NotificationSchedule> findAllByScheduleScheduleIdOrderByIdAsc(UUID scheduleId);
2221
}

‎ontime-back/src/main/java/devkor/ontime_back/service/ScheduleService.java‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,11 @@ public ScheduleDto showScheduleByScheduleId(Long userId, UUID scheduleId) {
123123
public void deleteSchedule(UUID scheduleId, Long userId) {
124124
Schedule schedule = getLockedScheduleWithAuthorization(scheduleId, userId);
125125
assertScheduleNotFinished(schedule);
126-
NotificationSchedule notification = notificationScheduleRepository.findByScheduleScheduleId(scheduleId)
127-
.orElseThrow(() -> new GeneralException(NOTIFICATION_NOT_FOUND));
126+
List<NotificationSchedule> notifications = notificationScheduleRepository.findAllByScheduleScheduleIdOrderByIdAsc(scheduleId);
127+
if (notifications.isEmpty()) {
128+
throw new GeneralException(NOTIFICATION_NOT_FOUND);
129+
}
130+
notifications.forEach(notification -> notificationService.cancelScheduledNotification(notification.getId()));
128131
scheduleRepository.deleteByScheduleId(scheduleId);
129132
}
130133

@@ -160,6 +163,9 @@ public void updateAndRescheduleNotification(LocalDateTime newNotificationTime, N
160163
// schedule 추가
161164
@Transactional
162165
public void addSchedule(ScheduleAddDto scheduleAddDto, Long userId) {
166+
if (scheduleRepository.existsById(scheduleAddDto.getScheduleId())) {
167+
throw new GeneralException(RESOURCE_ALREADY_EXISTS);
168+
}
163169
User user = userRepository.findById(userId)
164170
.orElseThrow(() -> new GeneralException(USER_NOT_FOUND));
165171
Place place = placeRepository.findByPlaceName(scheduleAddDto.getPlaceName())
@@ -559,9 +565,8 @@ public void refreshNotStartedDefaultModeSchedules(Long userId) {
559565
}
560566

561567
private void refreshScheduleNotification(Schedule schedule) {
562-
NotificationSchedule notification = notificationScheduleRepository.findByScheduleScheduleId(schedule.getScheduleId())
563-
.orElseThrow(() -> new GeneralException(NOTIFICATION_NOT_FOUND));
564568
LocalDateTime newNotificationTime = getNotificationTime(schedule, schedule.getUser());
569+
NotificationSchedule notification = resolveNotificationForRefresh(schedule, newNotificationTime);
565570
if (newNotificationTime.equals(notification.getNotificationTime())) {
566571
notificationService.cancelScheduledNotification(notification.getId());
567572
notification.markAsUnsent();
@@ -572,6 +577,27 @@ private void refreshScheduleNotification(Schedule schedule) {
572577
updateAndRescheduleNotification(newNotificationTime, notification);
573578
}
574579

580+
private NotificationSchedule resolveNotificationForRefresh(Schedule schedule, LocalDateTime notificationTime) {
581+
List<NotificationSchedule> notifications = notificationScheduleRepository
582+
.findAllByScheduleScheduleIdOrderByIdAsc(schedule.getScheduleId());
583+
if (notifications.isEmpty()) {
584+
NotificationSchedule notification = NotificationSchedule.builder()
585+
.notificationTime(notificationTime)
586+
.isSent(false)
587+
.schedule(schedule)
588+
.build();
589+
return notificationScheduleRepository.save(notification);
590+
}
591+
592+
NotificationSchedule notification = notifications.get(0);
593+
for (int i = 1; i < notifications.size(); i++) {
594+
NotificationSchedule duplicate = notifications.get(i);
595+
notificationService.cancelScheduledNotification(duplicate.getId());
596+
notificationScheduleRepository.delete(duplicate);
597+
}
598+
return notification;
599+
}
600+
575601
private PreparationDto mapPreparationUserToDto(PreparationUser preparationUser) {
576602
return new PreparationDto(
577603
preparationUser.getPreparationUserId(),

‎ontime-back/src/main/resources/db/migration/V19__deduplicate_notification_schedule_add_unique_constraint.sql‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
DELETE ns_duplicate
2+
FROM notification_schedule ns_duplicate
3+
JOIN notification_schedule ns_keep
4+
ON ns_duplicate.schedule_id = ns_keep.schedule_id
5+
AND ns_duplicate.id > ns_keep.id
6+
WHERE ns_duplicate.schedule_id IS NOT NULL;
7+
8+
CREATE UNIQUE INDEX uk_notification_schedule_schedule
9+
ON notification_schedule (schedule_id);

‎ontime-back/src/test/java/devkor/ontime_back/service/PreparationUserServiceTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,4 +299,43 @@ void handlePreparationUsers_withDeletingExisting() {
299299
.containsExactlyInAnyOrder("세면", "옷입기");
300300
}
301301

302+
@Test
303+
@DisplayName("기존 준비과정 ID를 유지하며 기본 준비과정을 수정한다.")
304+
void updatePreparationUsers_reusesExistingStepIds() {
305+
// given
306+
User newUser = User.builder()
307+
.email("reuse@example.com")
308+
.password(passwordEncoder.encode("password1234"))
309+
.name("reuse")
310+
.punctualityScore(-1f)
311+
.scheduleCountAfterReset(0)
312+
.latenessCountAfterReset(0)
313+
.build();
314+
userRepository.save(newUser);
315+
316+
UUID preparationUser1Id = UUID.randomUUID();
317+
UUID preparationUser2Id = UUID.randomUUID();
318+
319+
PreparationUser preparationUser2 = preparationUserRepository.save(new PreparationUser(
320+
preparationUser2Id, newUser, "화장실 가기", 3, 1, null));
321+
preparationUserRepository.save(new PreparationUser(
322+
preparationUser1Id, newUser, "메이크업", 38, 0, preparationUser2));
323+
324+
List<PreparationDto> preparationDtoList = List.of(
325+
new PreparationDto(preparationUser1Id, "메이크업", 38, preparationUser2Id),
326+
new PreparationDto(preparationUser2Id, "화장실 가기", 3, null)
327+
);
328+
329+
// when
330+
preparationUserService.updatePreparationUsers(newUser.getId(), preparationDtoList);
331+
332+
// then
333+
List<PreparationDto> result = preparationUserService.showAllPreparationUsers(newUser.getId());
334+
assertThat(result).hasSize(2);
335+
assertThat(result.get(0).getPreparationId()).isEqualTo(preparationUser1Id);
336+
assertThat(result.get(0).getNextPreparationId()).isEqualTo(preparationUser2Id);
337+
assertThat(result.get(1).getPreparationId()).isEqualTo(preparationUser2Id);
338+
assertThat(result.get(1).getNextPreparationId()).isNull();
339+
}
340+
302341
}

‎ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceNotificationRefreshTest.java‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package devkor.ontime_back.service;
2+
3+
import devkor.ontime_back.dto.PreparationDto;
4+
import devkor.ontime_back.entity.DoneStatus;
5+
import devkor.ontime_back.entity.NotificationSchedule;
6+
import devkor.ontime_back.entity.PreparationMode;
7+
import devkor.ontime_back.entity.Schedule;
8+
import devkor.ontime_back.entity.User;
9+
import devkor.ontime_back.repository.NotificationScheduleRepository;
10+
import devkor.ontime_back.repository.PlaceRepository;
11+
import devkor.ontime_back.repository.PreparationScheduleRepository;
12+
import devkor.ontime_back.repository.PreparationTemplateRepository;
13+
import devkor.ontime_back.repository.PreparationTemplateStepRepository;
14+
import devkor.ontime_back.repository.PreparationUserRepository;
15+
import devkor.ontime_back.repository.ScheduleRepository;
16+
import devkor.ontime_back.repository.UserRepository;
17+
import org.junit.jupiter.api.BeforeEach;
18+
import org.junit.jupiter.api.Test;
19+
import org.junit.jupiter.api.extension.ExtendWith;
20+
import org.mockito.Mock;
21+
import org.mockito.junit.jupiter.MockitoExtension;
22+
import org.springframework.test.util.ReflectionTestUtils;
23+
24+
import java.time.LocalDateTime;
25+
import java.util.List;
26+
import java.util.Optional;
27+
import java.util.UUID;
28+
29+
import static org.assertj.core.api.Assertions.assertThat;
30+
import static org.mockito.Mockito.verify;
31+
import static org.mockito.Mockito.when;
32+
33+
@ExtendWith(MockitoExtension.class)
34+
class ScheduleServiceNotificationRefreshTest {
35+
36+
@Mock
37+
private UserService userService;
38+
@Mock
39+
private NotificationService notificationService;
40+
@Mock
41+
private AlarmService alarmService;
42+
@Mock
43+
private ScheduleRepository scheduleRepository;
44+
@Mock
45+
private UserRepository userRepository;
46+
@Mock
47+
private PlaceRepository placeRepository;
48+
@Mock
49+
private PreparationScheduleRepository preparationScheduleRepository;
50+
@Mock
51+
private PreparationUserRepository preparationUserRepository;
52+
@Mock
53+
private PreparationTemplateRepository preparationTemplateRepository;
54+
@Mock
55+
private PreparationTemplateStepRepository preparationTemplateStepRepository;
56+
@Mock
57+
private NotificationScheduleRepository notificationScheduleRepository;
58+
@Mock
59+
private PreparationStepService preparationStepService;
60+
61+
private ScheduleService scheduleService;
62+
63+
@BeforeEach
64+
void setUp() {
65+
scheduleService = new ScheduleService(
66+
userService,
67+
notificationService,
68+
alarmService,
69+
scheduleRepository,
70+
userRepository,
71+
placeRepository,
72+
preparationScheduleRepository,
73+
preparationUserRepository,
74+
preparationTemplateRepository,
75+
preparationTemplateStepRepository,
76+
notificationScheduleRepository,
77+
preparationStepService
78+
);
79+
}
80+
81+
@Test
82+
void refreshDefaultModeScheduleDeduplicatesNotificationRows() {
83+
UUID scheduleId = UUID.randomUUID();
84+
User user = User.builder()
85+
.id(1L)
86+
.spareTime(10)
87+
.build();
88+
Schedule schedule = Schedule.builder()
89+
.scheduleId(scheduleId)
90+
.scheduleName("학교")
91+
.scheduleTime(LocalDateTime.of(2026, 6, 29, 14, 30))
92+
.moveTime(10)
93+
.doneStatus(DoneStatus.NOT_ENDED)
94+
.preparationMode(PreparationMode.DEFAULT)
95+
.user(user)
96+
.build();
97+
NotificationSchedule canonical = notification(1L, schedule, LocalDateTime.of(2026, 6, 29, 13, 0));
98+
NotificationSchedule duplicate1 = notification(2L, schedule, LocalDateTime.of(2026, 6, 29, 13, 5));
99+
NotificationSchedule duplicate2 = notification(3L, schedule, LocalDateTime.of(2026, 6, 29, 13, 10));
100+
NotificationSchedule duplicate3 = notification(4L, schedule, LocalDateTime.of(2026, 6, 29, 13, 15));
101+
102+
when(scheduleRepository.findNotStartedDefaultModeSchedules(1L)).thenReturn(List.of(schedule));
103+
when(scheduleRepository.findByIdWithUser(scheduleId)).thenReturn(Optional.of(schedule));
104+
when(preparationUserRepository.findByUserIdWithNextPreparation(1L)).thenReturn(List.of());
105+
when(preparationStepService.toLinkedDtoFromUser(List.of())).thenReturn(List.of(
106+
new PreparationDto(UUID.randomUUID(), "메이크업", 38, null),
107+
new PreparationDto(UUID.randomUUID(), "화장실 가기", 3, null)
108+
));
109+
when(alarmService.getDefaultAlarmOffsetMinutes(1L)).thenReturn(0);
110+
when(notificationScheduleRepository.findAllByScheduleScheduleIdOrderByIdAsc(scheduleId))
111+
.thenReturn(List.of(canonical, duplicate1, duplicate2, duplicate3));
112+
113+
scheduleService.refreshNotStartedDefaultModeSchedules(1L);
114+
115+
assertThat(canonical.getNotificationTime()).isEqualTo(LocalDateTime.of(2026, 6, 29, 13, 29));
116+
verify(notificationService).cancelScheduledNotification(2L);
117+
verify(notificationService).cancelScheduledNotification(3L);
118+
verify(notificationService).cancelScheduledNotification(4L);
119+
verify(notificationScheduleRepository).delete(duplicate1);
120+
verify(notificationScheduleRepository).delete(duplicate2);
121+
verify(notificationScheduleRepository).delete(duplicate3);
122+
verify(notificationScheduleRepository).save(canonical);
123+
verify(notificationService).scheduleReminder(canonical);
124+
}
125+
126+
private NotificationSchedule notification(Long id, Schedule schedule, LocalDateTime notificationTime) {
127+
NotificationSchedule notification = NotificationSchedule.builder()
128+
.schedule(schedule)
129+
.notificationTime(notificationTime)
130+
.isSent(false)
131+
.build();
132+
ReflectionTestUtils.setField(notification, "id", id);
133+
return notification;
134+
}
135+
}

‎ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,59 @@ void addSchedule_withNewPlace() {
11091109
assertThat(savedSchedule.get().getPlace().getPlaceName()).isEqualTo("고려대학교");
11101110
}
11111111

1112+
@Test
1113+
@DisplayName("이미 존재하는 scheduleId로 약속 추가 시 실패한다.")
1114+
void addSchedule_failByDuplicateScheduleId() {
1115+
// given
1116+
User newUser = User.builder()
1117+
.email("user@example.com")
1118+
.password(passwordEncoder.encode("password1234"))
1119+
.name("jinsuh")
1120+
.punctualityScore(-1f)
1121+
.scheduleCountAfterReset(0)
1122+
.latenessCountAfterReset(0)
1123+
.build();
1124+
userRepository.save(newUser);
1125+
1126+
Place place = Place.builder()
1127+
.placeId(UUID.fromString("70d460da-6a82-4c57-a285-567cdeda5601"))
1128+
.placeName("과학도서관")
1129+
.build();
1130+
placeRepository.save(place);
1131+
1132+
UUID duplicateScheduleId = UUID.fromString("023e4567-e89b-12d3-a456-426614170000");
1133+
scheduleRepository.save(Schedule.builder()
1134+
.scheduleId(duplicateScheduleId)
1135+
.scheduleName("기존 약속")
1136+
.scheduleTime(LocalDateTime.of(2025, 2, 10, 14, 0))
1137+
.moveTime(30)
1138+
.latenessTime(-1)
1139+
.doneStatus(DoneStatus.NOT_ENDED)
1140+
.place(place)
1141+
.user(newUser)
1142+
.build());
1143+
1144+
ScheduleAddDto scheduleAddDto = ScheduleAddDto.builder()
1145+
.scheduleId(duplicateScheduleId)
1146+
.scheduleName("중복 약속")
1147+
.scheduleTime(LocalDateTime.of(2025, 2, 11, 14, 0))
1148+
.moveTime(30)
1149+
.scheduleNote("늦으면 안됨")
1150+
.placeId(place.getPlaceId())
1151+
.placeName(place.getPlaceName())
1152+
.scheduleSpareTime(5)
1153+
.isChange(false)
1154+
.isStarted(false)
1155+
.build();
1156+
1157+
// when & then
1158+
assertThatThrownBy(() -> scheduleService.addSchedule(scheduleAddDto, newUser.getId()))
1159+
.isInstanceOf(GeneralException.class)
1160+
.hasMessage(ErrorCode.RESOURCE_ALREADY_EXISTS.getMessage())
1161+
.extracting("errorCode")
1162+
.isEqualTo(ErrorCode.RESOURCE_ALREADY_EXISTS);
1163+
}
1164+
11121165
@Test
11131166
@DisplayName("다른 사용자가 약속 추가 시 실패한다.")
11141167
void addSchedule_failByNonExistentUser() {

0 commit comments

Comments
 (0)