Skip to content

Commit

Permalink
fix(TripSurveyController): Set date opened on survey notification onl…
Browse files Browse the repository at this point in the history 
…y if empty.
  • Loading branch information
@binh-dam-ibigroup
binh-dam-ibigroup committed Dec 13, 2024
1 parent 26bf78a commit 9891b7b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 34 deletions.
12 changes: 6 additions & 6 deletions src/main/java/org/opentripplanner/middleware/controllers/api/TripSurveyController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ private static void returnInvalidUrlParametersError(Request request) {
}

/**
* Mark notification as opened.
* Mark notification as opened, but if an opened date is already populated, do nothing.
*/
private static void updateNotificationState(OtpUser user, String notificationId) {
Optional<TripSurveyNotification> notificationOpt = user.findNotification(notificationId);
if (notificationOpt.isPresent()) {
notificationOpt.get().timeOpened = Date.from(Instant.now());
private static void updateNotificationStateIfNeeded(OtpUser user, String notificationId) {
TripSurveyNotification notification = user.findNotification(notificationId).orElse(null);
if (notification != null && notification.timeOpened == null) {
notification.timeOpened = Date.from(Instant.now());
Persistence.otpUsers.replace(user.id, user);
}
}
Expand Down Expand Up @@ -112,7 +112,7 @@ private static boolean processCall(Request req, Response res) {
);

// Update notification state
updateNotificationState(user, notificationId);
updateNotificationStateIfNeeded(user, notificationId);

// Redirect
res.redirect(surveyUrl);
Expand Down
61 changes: 33 additions & 28 deletions src/test/java/org/opentripplanner/middleware/controllers/api/TripSurveyControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.opentripplanner.middleware.testutils.ApiTestUtils;
import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment;
import org.opentripplanner.middleware.testutils.PersistenceTestUtils;
import org.opentripplanner.middleware.utils.HttpResponseValues;

import java.time.Instant;
import java.util.Date;
Expand All @@ -33,6 +34,7 @@
import static org.opentripplanner.middleware.testutils.ApiTestUtils.makeRequest;

class TripSurveyControllerTest extends OtpMiddlewareTestEnvironment {
public static final String SURVEY_PATH_TEMPLATE = "api/trip-survey/open?user_id=%s&trip_id=%s&notification_id=%s";
private static OtpUser otpUser;
private static MonitoredTrip monitoredTrip;
private static TrackedJourney trackedJourney;
Expand Down Expand Up @@ -91,22 +93,10 @@ void canOpenSurveyAndUpdateNotificationStatus() {

OtpUser existingUser = Persistence.otpUsers.getById(otpUser.id);
assertNotNull(existingUser);
existingUser.tripSurveyNotifications.forEach(n -> {
assertNull(n.timeOpened);
});
existingUser.tripSurveyNotifications.forEach(n -> assertNull(n.timeOpened));

Instant requestInstant = Instant.now();
var response = makeRequest(
String.format(
"api/trip-survey/open?user_id=%s&trip_id=%s&notification_id=%s",
otpUser.id,
monitoredTrip.id,
NOTIFICATION_ID
),
"",
Map.of(),
HttpMethod.GET
);
var response = invokeSurveyEndpoint(otpUser.id, monitoredTrip.id, NOTIFICATION_ID);
Instant requestCompleteInstant = Instant.now();

assertEquals(HttpStatus.OK_200, response.status);
Expand All @@ -116,35 +106,40 @@ void canOpenSurveyAndUpdateNotificationStatus() {
assertEquals(otpUser.tripSurveyNotifications.size(), updatedUser.tripSurveyNotifications.size());

int updatedNotificationCount = 0;
Date notificationTimeOpened = null;
for (TripSurveyNotification notification : updatedUser.tripSurveyNotifications) {
if (NOTIFICATION_ID.equals(notification.id)) {
assertNotNull(notification.timeOpened);
assertTrue(notification.timeOpened.toInstant().isAfter(requestInstant));
assertTrue(notification.timeOpened.toInstant().isBefore(requestCompleteInstant));
notificationTimeOpened = notification.timeOpened;
assertNotNull(notificationTimeOpened);
assertTrue(notificationTimeOpened.toInstant().isAfter(requestInstant));
assertTrue(notificationTimeOpened.toInstant().isBefore(requestCompleteInstant));
updatedNotificationCount++;
} else {
assertNull(notification.timeOpened);
}
}
assertEquals(1, updatedNotificationCount);
assertNotNull(notificationTimeOpened);

// A second request should not change the notification opened date.
var response2 = invokeSurveyEndpoint(otpUser.id, monitoredTrip.id, NOTIFICATION_ID);
assertEquals(HttpStatus.OK_200, response2.status);

updatedUser = Persistence.otpUsers.getById(otpUser.id);
assertNotNull(updatedUser);
for (TripSurveyNotification notification : updatedUser.tripSurveyNotifications) {
if (NOTIFICATION_ID.equals(notification.id)) {
assertEquals(notificationTimeOpened, notification.timeOpened);
}
}
}

@ParameterizedTest
@MethodSource("createShouldRejectInvalidParamsCases")
void shouldRejectInvalidParams(String userId, String tripId, String notificationId) {
assumeTrue(IS_END_TO_END);

var response = makeRequest(
String.format(
"api/trip-survey/open?user_id=%s&trip_id=%s&notification_id=%s",
userId,
tripId,
notificationId
),
"",
Map.of(),
HttpMethod.GET
);
var response = invokeSurveyEndpoint(userId, tripId, notificationId);

assertEquals(
HttpStatus.BAD_REQUEST_400,
Expand All @@ -163,4 +158,14 @@ private static Stream<Arguments> createShouldRejectInvalidParamsCases() {
Arguments.of(otpUser.id, monitoredTrip.id, null)
);
}

/** Helper to invoke the survey endpoint */
private static HttpResponseValues invokeSurveyEndpoint(String userId, String tripId, String notificationId) {
return makeRequest(
String.format(SURVEY_PATH_TEMPLATE, userId, tripId, notificationId),
"",
Map.of(),
HttpMethod.GET
);
}
}

0 comments on commit 9891b7b

Please sign in to comment.