refactor(Addressed PR feedback and updated accept link to include loc…

…ale):
ibi-group · Oct 31, 2024 · 99341cb · 99341cb
1 parent b8070d3
commit 99341cb
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 24 deletions.
diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java
@@ -25,6 +25,7 @@
 
 import static io.github.manusant.ss.descriptor.MethodDescriptor.path;
 import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.ACCEPT_KEY;
+import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.USER_LOCALE;
 import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.ensureRelatedUserIntegrity;
 import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.manageAcceptDependentEmail;
 import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt;
@@ -95,6 +96,7 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
                     .withDescription("Accept a dependent request.")
                     .withResponses(SwaggerUtils.createStandardResponses(OtpUser.class))
                     .withPathParam().withName(ACCEPT_KEY).withRequired(true).withDescription("The accept dependent unique key.").and()
+                    .withPathParam().withName(USER_LOCALE).withRequired(true).withDescription("The accepting user's locale.").and()
                     .withResponseType(OtpUser.class),
                 TrustedCompanion::acceptDependent
             )

diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java
@@ -18,7 +18,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static com.mongodb.client.model.Filters.eq;
+import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.removeDependent;
 
 /**
  * This represents a user of an OpenTripPlanner instance (typically of the standard OTP UI/otp-react-redux).
@@ -144,13 +144,8 @@ public boolean delete(boolean deleteAuth0User) {
 
         // If a dependent, remove relationship with all related users.
         for (RelatedUser relatedUser : relatedUsers) {
-            OtpUser user = Persistence.otpUsers.getOneFiltered(eq("email", relatedUser.email));
-            if (user != null) {
-                user.dependents.remove(this.id);
-                Persistence.otpUsers.replace(user.id, user);
-            }
+            removeDependent(this, relatedUser);
         }
-
         return Persistence.otpUsers.removeById(this.id);
     }
 

diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java
@@ -1,6 +1,7 @@
 package org.opentripplanner.middleware.tripmonitor;
 
 import com.mongodb.client.model.Filters;
+import org.apache.logging.log4j.util.Strings;
 import org.opentripplanner.middleware.OtpMiddlewareMain;
 import org.opentripplanner.middleware.i18n.Message;
 import org.opentripplanner.middleware.models.OtpUser;
@@ -13,6 +14,7 @@
 import spark.Request;
 import spark.Response;
 
+import java.net.URLEncoder;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -22,7 +24,9 @@
 import java.util.stream.Collectors;
 
 import static com.mongodb.client.model.Filters.eq;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH;
+import static org.opentripplanner.middleware.utils.I18nUtils.getLocaleFromString;
 import static org.opentripplanner.middleware.utils.I18nUtils.label;
 
 public class TrustedCompanion {
@@ -34,8 +38,10 @@ private TrustedCompanion() {
     private static final String AWS_API_SERVER = ConfigUtils.getConfigPropertyAsText("AWS_API_SERVER");
     private static final String AWS_API_STAGE = ConfigUtils.getConfigPropertyAsText("AWS_API_STAGE");
     private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL");
-    private static final String TRUSTED_COMPANION_CONFIRMATION_PAGE_URL = ConfigUtils.getConfigPropertyAsText("TRUSTED_COMPANION_CONFIRMATION_PAGE_URL");
+    private static final String TRUSTED_COMPANION_CONFIRMATION_PAGE_URL =
+        ConfigUtils.getConfigPropertyAsText("TRUSTED_COMPANION_CONFIRMATION_PAGE_URL");
     public static final String ACCEPT_KEY = "acceptKey";
+    public static final String USER_LOCALE = "userLocale";
     public static final String EMAIL_FIELD_NAME = "email";
 
     /** Note: This path is excluded from security checks, see {@link OtpMiddlewareMain#initializeHttpEndpoints()}. */
@@ -46,6 +52,7 @@ private TrustedCompanion() {
      * successful redirect the user to the confirmation page, else redirect to the error page with related error.
      */
     public static OtpUser acceptDependent(Request request, Response response) {
+        Locale locale = getUserLocaleFromRequest(request);
         try {
             String acceptKey = getAcceptKeyFromRequest(request);
             OtpUser dependentUser = getUserFromAcceptKey(acceptKey);
@@ -69,9 +76,11 @@ public static OtpUser acceptDependent(Request request, Response response) {
                 return dependentUser;
             }
         } catch (IllegalArgumentException e) {
-            response.redirect(
-                String.format("%s?error=%s", TRUSTED_COMPANION_CONFIRMATION_PAGE_URL, Message.ACCEPT_DEPENDENT_ERROR.get(null))
-            );
+            response.redirect(String.format(
+                "%s?%s",
+                TRUSTED_COMPANION_CONFIRMATION_PAGE_URL,
+                URLEncoder.encode(String.format("error=%s", Message.ACCEPT_DEPENDENT_ERROR.get(locale)), UTF_8)
+            ));
         }
         return null;
     }
@@ -102,6 +111,15 @@ private static String getAcceptKeyFromRequest(Request request) throws IllegalArg
         return acceptKey;
     }
 
+    /**
+     * Extract the user's language tag from the request and return the {@link Locale} from it.
+     */
+    private static Locale getUserLocaleFromRequest(Request request) throws IllegalArgumentException {
+        // Note: optional is true so a missing locale will be handled here.
+        String languageTag = HttpUtils.getQueryParamFromRequest(request, USER_LOCALE, true);
+        return getLocaleFromString(languageTag);
+    }
+
     /**
      * Retrieve the dependent user matching the accept key.
      */
@@ -111,7 +129,7 @@ private static OtpUser getUserFromAcceptKey(String acceptKey) throws IllegalArgu
         }
         OtpUser user = getUserForAcceptKey(acceptKey);
         if (user == null) {
-            throw new IllegalArgumentException("Otp user unknown.");
+            throw new IllegalArgumentException("OTP user unknown.");
         }
         return user;
     }
@@ -153,8 +171,8 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r
         Locale locale = I18nUtils.getOtpUserLocale(relatedUser);
 
         String acceptDependentLinkLabel = Message.ACCEPT_DEPENDENT_EMAIL_LINK_TEXT.get(locale);
-        String acceptDependentUrl = getAcceptDependentUrl(acceptKey);
-        String addressee = (dependentUser.name != null) ? dependentUser.name : dependentUser.email;
+        String acceptDependentUrl = getAcceptDependentUrl(acceptKey, locale);
+        String addressee = (Strings.isBlank(dependentUser.name)) ? dependentUser.email : dependentUser.name;
 
         // A HashMap is needed instead of a Map for template data to be serialized to the template renderer.
         Map<String, Object> templateData = new HashMap<>(
@@ -178,12 +196,12 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r
         );
     }
 
-    private static String getAcceptDependentUrl(String acceptKey) {
-        return String.format("%s/%s%s", AWS_API_SERVER, AWS_API_STAGE, getAcceptDependentEndPoint(acceptKey));
+    private static String getAcceptDependentUrl(String acceptKey, Locale locale) {
+        return String.format("%s/%s%s", AWS_API_SERVER, AWS_API_STAGE, getAcceptDependentEndPoint(acceptKey, locale));
     }
 
-    public static String getAcceptDependentEndPoint(String acceptKey) {
-        return String.format("/%s?%s=%s", ACCEPT_DEPENDENT_PATH, ACCEPT_KEY, acceptKey);
+    public static String getAcceptDependentEndPoint(String acceptKey, Locale locale) {
+        return String.format("/%s?%s=%s&%s=%s", ACCEPT_DEPENDENT_PATH, ACCEPT_KEY, acceptKey, USER_LOCALE, locale.toLanguageTag());
     }
 
     /**
@@ -203,11 +221,18 @@ public static void ensureRelatedUserIntegrity(OtpUser updatedUser, OtpUser preEx
             .filter(relatedUser -> !updatedUser.relatedUsers.contains(relatedUser))
             .collect(Collectors.toList());
         for (RelatedUser relatedUser : difference) {
-            OtpUser user = Persistence.otpUsers.getOneFiltered(eq(EMAIL_FIELD_NAME, relatedUser.email));
-            if (user != null) {
-                user.dependents.remove(updatedUser.id);
-                Persistence.otpUsers.replace(user.id, user);
-            }
+            removeDependent(updatedUser, relatedUser);
+        }
+    }
+
+    /**
+     * Remove the dependent reference from the related user.
+     */
+    public static void removeDependent(OtpUser dependent, RelatedUser relatedUser) {
+        OtpUser user = Persistence.otpUsers.getOneFiltered(eq(EMAIL_FIELD_NAME, relatedUser.email));
+        if (user != null) {
+            user.dependents.remove(dependent.id);
+            Persistence.otpUsers.replace(user.id, user);
         }
     }
 }
diff --git a/src/main/java/org/opentripplanner/middleware/utils/I18nUtils.java b/src/main/java/org/opentripplanner/middleware/utils/I18nUtils.java
@@ -4,6 +4,7 @@
 import org.opentripplanner.middleware.models.OtpUser;
 
 import java.util.Collection;
+import java.util.IllformedLocaleException;
 import java.util.Locale;
 
 public class I18nUtils {
@@ -31,4 +32,18 @@ public static Locale getOtpUserLocale(OtpUser user) {
         return Locale.forLanguageTag(user == null || user.preferredLocale == null ? "en-US" : user.preferredLocale);
     }
 
+    /**
+     * Attempt to create a {@link Locale} from the language tag.
+     */
+    public static Locale getLocaleFromString(String languageTag) {
+        Locale locale = Locale.forLanguageTag("en-US");
+        if (languageTag != null) {
+            try {
+                locale = Locale.forLanguageTag(languageTag);
+            } catch (NullPointerException | IllformedLocaleException e) {
+                // Give up and use the default!
+            }
+        }
+        return locale;
+    }
 }
diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java
@@ -21,6 +21,7 @@
 
 import java.util.Date;
 import java.util.List;
+import java.util.Locale;
 import java.util.UUID;
 import java.util.stream.Stream;
 
@@ -187,7 +188,8 @@ void canAcceptDependentRequest() {
         ));
         Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne);
 
-        String path = TrustedCompanion.getAcceptDependentEndPoint(acceptKey);
+        Locale locale = new Locale("en", "GB");
+        String path = TrustedCompanion.getAcceptDependentEndPoint(acceptKey, locale);
         makeGetRequest(path, null);
 
         relatedUserOne = Persistence.otpUsers.getById(relatedUserOne.id);