cleanup(TripPlan): no choosing between OTP1 & OTP2 (#1873)

* cleanup(TripPlan): no choosing between OTP1 & OTP2

* cleanup: remove OTP version feature flags

* cleanup: remove laboratory

* deps: remove laboratory

.envrc.template

@@ -11,9 +11,7 @@
 
 export DRUPAL_ROOT=https://live-mbta.pantheonsite.io
 # export GOOGLE_API_KEY=
-export OPEN_TRIP_PLANNER_URL=http://otp-local.mbtace.com
-export OPEN_TRIP_PLANNER_2_URL=http://otp2-local.mbtace.com
-export OPEN_TRIP_PLANNER_2_PERCENTAGE=0
+export OPEN_TRIP_PLANNER_URL=http://otp2-local.mbtace.com
 export RECAPTCHA_PUBLIC_KEY=6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI
 export RECAPTCHA_PRIVATE_KEY=6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe
 # export STATIC_HOST=

config/runtime.exs

@@ -114,9 +114,7 @@ end
 
 config :dotcom, OpenTripPlanner,
   timezone: System.get_env("OPEN_TRIP_PLANNER_TIMEZONE", "America/New_York"),
-  otp1_url: System.get_env("OPEN_TRIP_PLANNER_URL"),
-  otp2_url: System.get_env("OPEN_TRIP_PLANNER_2_URL"),
-  otp2_percentage: System.get_env("OPEN_TRIP_PLANNER_2_PERCENTAGE"),
+  otp_url: System.get_env("OPEN_TRIP_PLANNER_URL"),
   wiremock_proxy: System.get_env("WIREMOCK_PROXY", "false"),
   wiremock_proxy_url: System.get_env("WIREMOCK_TRIP_PLAN_PROXY_URL")
 

cypress/support/commands.js

@@ -73,7 +73,7 @@ Cypress.Commands.add("selectRandomServiceAndSubject", () => {
  * 
  * This includes a hack so that full page snapshots can be properly captured.
  */
-Cypress.Commands.add("takeFullScreenshot", (subject, name, options={}) => {
+Cypress.Commands.add("takeFullScreenshot", (subject, name, options = {}) => {
   /**
    * Hack for dealing with screenshots.
    * See Cypress bugs: https://github.com/cypress-io/cypress/issues/2681
@@ -88,11 +88,3 @@ Cypress.Commands.add("takeFullScreenshot", (subject, name, options={}) => {
   cy.screenshot(subject, name, options);
 });
 
-/**
- * Navigates to /_flags to enable a feature.
- * e.g. cy.enableFlaggedFeature("nav_redesign")
- */
-Cypress.Commands.add("enableFlaggedFeature", (featureSelector) => {
-  cy.visit("/_flags");
-  cy.get(`form[action="/_flags/enable/${featureSelector}"]`).submit();
-});

lib/dotcom_web/controllers/trip_plan_controller.ex

@@ -9,6 +9,7 @@ defmodule DotcomWeb.TripPlanController do
   alias Routes.Route
   alias Dotcom.TripPlan.{Query, RelatedLink, ItineraryRow, ItineraryRowList}
   alias Dotcom.TripPlan.Map, as: TripPlanMap
+  alias DotcomWeb.Plugs.Cookies
   alias TripPlan.{Itinerary, Leg, NamedPosition, PersonalDetail, TransitDetail, Transfer}
 
   plug(:assign_initial_map)
@@ -239,27 +240,12 @@ defmodule DotcomWeb.TripPlanController do
     Enum.map(related_links, fn x -> Enum.uniq_by(x, fn y -> get_route(y) end) end)
   end
 
-  def get_conn_opts(conn) do
-    cookie = DotcomWeb.Plugs.Cookies.id_cookie_name()
-    user_cookie = Map.get(conn.cookies, cookie, "0")
-
+  defp get_conn_opts(conn) do
     user_id =
-      try do
-        String.to_integer(user_cookie)
-      rescue
-        e in ArgumentError ->
-          Logger.warning(fn ->
-            "#{__MODULE__}.get_conn_opts Couldn't parse #{cookie} cookie as an int, using 0. #{cookie}=#{user_cookie} parse_error=#{e.message}"
-          end)
-
-          0
-      end
+      conn.cookies
+      |> Map.get(Cookies.id_cookie_name())
 
-    [
-      user_id: user_id,
-      force_otp1: Laboratory.enabled?(conn, :force_otp1),
-      force_otp2: Laboratory.enabled?(conn, :force_otp2)
-    ]
+    [user_id: user_id]
   end
 
   @spec render_plan(Plug.Conn.t(), map) :: Plug.Conn.t()

lib/dotcom_web/router.ex

@@ -29,7 +29,6 @@ defmodule DotcomWeb.Router do
     plug(DotcomWeb.Plugs.ClearCookies)
     plug(DotcomWeb.Plugs.Cookies)
     plug(:optional_disable_indexing)
-    plug(:activate_flag)
     plug(DotcomWeb.Plugs.LineSuspensions)
   end
 
@@ -275,12 +274,6 @@ defmodule DotcomWeb.Router do
     get("/about_the_mbta/news_events", Redirector, to: "/news")
   end
 
-  scope "/_flags" do
-    pipe_through([:secure, :browser])
-
-    forward("/", Laboratory.Router)
-  end
-
   scope "/", DotcomWeb do
     pipe_through([:secure, :browser])
 
@@ -300,22 +293,4 @@ defmodule DotcomWeb.Router do
       Plug.Conn.put_resp_header(conn, "x-robots-tag", "noindex")
     end
   end
-
-  # Activates a feature flag using a url parameter, e.g.
-  # visiting /?active_flag=nav_redesign, using the same cookie settings as
-  # activating via /_flags
-  defp activate_flag(%{query_params: %{"active_flag" => flag}} = conn, _) do
-    # check if it's a known flagged feature
-    known_features =
-      Application.get_env(:laboratory, :features)
-      |> Enum.map(fn {key, _, _} -> Atom.to_string(key) end)
-
-    if Enum.member?(known_features, flag) do
-      Plug.Conn.put_resp_cookie(conn, flag, "true", Application.get_env(:laboratory, :cookie))
-    else
-      conn
-    end
-  end
-
-  defp activate_flag(conn, _), do: conn
 end

lib/trip_plan/api.ex

@@ -16,7 +16,7 @@ defmodule TripPlan.Api do
           | {:optimize_for, :less_walking | :fewest_transfers}
   @type plan_opts :: [plan_opt]
 
-  @type connection_opts :: [user_id: integer, force_otp1: boolean, force_otp2: boolean]
+  @type connection_opts :: [user_id: integer]
 
   @type error ::
           :outside_bounds

lib/trip_plan/api/open_trip_planner.ex

@@ -9,8 +9,8 @@ defmodule TripPlan.Api.OpenTripPlanner do
     plan(from, to, connection_opts, opts)
   end
 
-  @impl true
-  def plan(from, to, connection_opts, opts) do
+  @impl TripPlan.Api
+  def plan(from, to, _connection_opts, opts) do
     accessible? = Keyword.get(opts, :wheelchair_accessible?, false)
 
     with {:ok, params} <- build_params(from, to, opts) do
@@ -25,63 +25,13 @@ defmodule TripPlan.Api.OpenTripPlanner do
       }
       """
 
-      root_url = Keyword.get(opts, :root_url, nil) || pick_url(connection_opts)
+      root_url = Keyword.get(opts, :root_url) || config(:otp_url)
       graphql_url = "#{root_url}/otp/routers/default/index/"
 
       send_request(graphql_url, graphql_query, accessible?, &parse_ql/2)
     end
   end
 
-  def pick_url(connection_opts) do
-    user_id = connection_opts[:user_id]
-
-    cond do
-      connection_opts[:force_otp2] ->
-        Logger.info(fn ->
-          "#{__MODULE__}.pick_url Force OTP2 flag enabled, skipping random assignment mbta_id=#{user_id}"
-        end)
-
-        config(:otp2_url)
-
-      connection_opts[:force_otp1] ->
-        Logger.info(fn ->
-          "#{__MODULE__}.pick_url Force OTP1 flag enabled, skipping random assignment mbta_id=#{user_id}"
-        end)
-
-        config(:otp1_url)
-
-      true ->
-        percent_threshold = get_otp2_percentage()
-
-        :rand.seed(:exsss, user_id)
-        placement = :rand.uniform()
-        use_otp2 = placement < percent_threshold / 100
-
-        Logger.info(fn ->
-          "#{__MODULE__}.pick_url placement=#{placement} otp2_percentage=#{percent_threshold}% mbta_id=#{user_id} use_otp2=#{use_otp2}"
-        end)
-
-        if use_otp2 do
-          config(:otp2_url)
-        else
-          config(:otp1_url)
-        end
-    end
-  end
-
-  def get_otp2_percentage() do
-    try do
-      String.to_integer(config(:otp2_percentage))
-    rescue
-      e in ArgumentError ->
-        Logger.warning(fn ->
-          "#{__MODULE__}.get_otp2_percentage Couldn't parse OPEN_TRIP_PLANNER_2_PERCENTAGE env var as an int, using 0. OPEN_TRIP_PLANNER_2_PERCENTAGE=#{config(:otp2_percentage)} parse_error=#{e.message}"
-        end)
-
-        0
-    end
-  end
-
   def config(key) do
     Util.config(:dotcom, OpenTripPlanner, key)
   end
@@ -113,7 +63,7 @@ defmodule TripPlan.Api.OpenTripPlanner do
 
     _ =
       Logger.info(fn ->
-        "#{__MODULE__}.plan_response url=#{url} is_otp2=#{String.contains?(url, config(:otp2_url))} query=#{inspect(query)} #{status_text(response)} duration=#{duration / :timer.seconds(1)}"
+        "#{__MODULE__}.plan_response url=#{url} query=#{inspect(query)} #{status_text(response)} duration=#{duration / :timer.seconds(1)}"
       end)
 
     response
@@ -185,7 +135,6 @@ defmodule TripPlan.Api.OpenTripPlanner do
               riderCategory {
                 id
                 name
-
               }
             }
           }

mix.exs

@@ -19,7 +19,7 @@ defmodule DotCom.Mixfile do
         "coveralls.html": :test
       ],
       dialyzer: [
-        plt_add_apps: [:mix, :phoenix_live_reload, :laboratory, :ex_aws, :ex_aws_ses],
+        plt_add_apps: [:mix, :phoenix_live_reload, :ex_aws, :ex_aws_ses],
         flags: [:race_conditions, :unmatched_returns],
         ignore_warnings: ".dialyzer.ignore-warnings"
       ],
@@ -57,8 +57,6 @@ defmodule DotCom.Mixfile do
     [
       # the module to invoke when the application is started
       mod: {Dotcom.Application, []},
-      # a list of applications that will be included in the application
-      included_applications: [:laboratory],
       # a list of OTP applications your application depends on which are not included in :deps
       extra_applications: extra_apps
     ]
@@ -96,7 +94,6 @@ defmodule DotCom.Mixfile do
       {:httpoison, "~> 1.5"},
       {:inflex, "~> 1.8.0"},
       {:jason, "~> 1.1"},
-      {:laboratory, [github: "paulswartz/laboratory", ref: "cookie_opts"]},
       {:logster, "~> 0.4.0"},
       {:mail, "~> 0.2"},
       {:mock, "~> 0.3.3", [only: :test]},

mix.lock

@@ -41,7 +41,6 @@
   "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},
   "inflex": {:hex, :inflex, "1.8.1", "9fa9684ff1a872eab7415c0be500cc1b7782f28da6ed75423081e75f92831b1c", [:mix], [], "hexpm", "f9d542ec03df73be5106a72357572728d83c4f1f6824d0a080bdf472784c49e6"},
   "jason": {:hex, :jason, "1.1.2", "b03dedea67a99223a2eaf9f1264ce37154564de899fd3d8b9a21b1a6fd64afe7", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fdf843bca858203ae1de16da2ee206f53416bbda5dc8c9e78f43243de4bc3afe"},
-  "laboratory": {:git, "https://github.com/paulswartz/laboratory.git", "24adbe2cb18d8368e140ab5e13c8f5b75adea742", [ref: "cookie_opts"]},
   "logster": {:hex, :logster, "0.4.3", "a20e9c60e94847e60064d99042d10b1d03abac354c2b360842bf9b3386e56b4a", [:mix], [{:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 1.5 or ~> 2.0 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm", "8a01498f0bf54a760086dc6c577f941f1b70a36f697bcda6249726725a8e6f72"},
   "mail": {:hex, :mail, "0.2.2", "b1d31beaa2a7b23d7b84b2794f037ef4dfdaba9e66d877142bedbaf0625b9c16", [:mix], [], "hexpm", "1c9d31548a60c44ded1806369e07a7dd4d05737eb47fa3238bbf2436b3da8a32"},
   "makeup": {:hex, :makeup, "1.1.1", "fa0bc768698053b2b3869fa8a62616501ff9d11a562f3ce39580d60860c3a55e", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "5dc62fbdd0de44de194898b6710692490be74baa02d9d108bc29f007783b0b48"},

test/dotcom/trip_plan/alerts_test.exs

@@ -8,7 +8,7 @@ defmodule Dotcom.TripPlan.AlertsTest do
 
   @from TripPlan.Api.MockPlanner.random_stop(stop_id: nil)
   @to TripPlan.Api.MockPlanner.random_stop(stop_id: nil)
-  @connection_opts [user_id: 1, force_otp1: false, force_otp2: false]
+  @connection_opts [user_id: 1]
   @date_time ~N[2017-06-27T11:43:00]
 
   describe "filter_for_itinerary/2" do

test/dotcom/trip_plan/itinerary_row_list_test.exs

@@ -5,7 +5,7 @@ defmodule Dotcom.TripPlan.ItineraryRowListTest do
 
   @from TripPlan.Api.MockPlanner.random_stop(stop_id: "place-sstat")
   @to TripPlan.Api.MockPlanner.random_stop(stop_id: nil)
-  @connection_opts [user_id: 1, force_otp1: false, force_otp2: false]
+  @connection_opts [user_id: 1]
   @date_time ~N[2017-06-27T11:43:00]
 
   describe "from_itinerary" do

test/dotcom/trip_plan/query_test.exs

@@ -24,9 +24,7 @@ defmodule Dotcom.TripPlan.QueryTest do
   ]
 
   @connection_opts [
-    user_id: 1,
-    force_otp1: false,
-    force_otp2: false
+    user_id: 1
   ]
 
   describe "from_query/1" do

test/dotcom/trip_plan/related_link_test.exs

@@ -8,7 +8,7 @@ defmodule Dotcom.TripPlan.RelatedLinkTest do
   setup do
     from = MockPlanner.random_stop()
     to = MockPlanner.random_stop()
-    connection_opts = [user_id: 1, force_otp1: false, force_otp2: false]
+    connection_opts = [user_id: 1]
     {:ok, [itinerary]} = TripPlan.plan(from, to, connection_opts, [])
     {:ok, %{itinerary: itinerary}}
   end
@@ -35,7 +35,7 @@ defmodule Dotcom.TripPlan.RelatedLinkTest do
     end
 
     test "returns a non-empty list for multiple kinds of itineraries" do
-      connection_opts = [user_id: 1, force_otp1: false, force_otp2: false]
+      connection_opts = [user_id: 1]
 
       for _i <- 0..100 do
         {:ok, [itinerary]} =

test/dotcom_web/controllers/trip_plan_controller_test.exs

@@ -832,7 +832,7 @@ defmodule DotcomWeb.TripPlanControllerTest do
     setup do
       from = MockPlanner.random_stop()
       to = MockPlanner.random_stop()
-      connection_opts = [user_id: 1, force_otp1: false, force_otp2: false]
+      connection_opts = [user_id: 1]
       {:ok, itineraries} = TripPlan.plan(from, to, connection_opts, [])
       {:ok, %{itineraries: itineraries}}
     end

test/dotcom_web/router_test.exs

@@ -149,18 +149,4 @@ defmodule Phoenix.Router.RoutingTest do
     conn = get(conn, "/")
     refute Enum.find(conn.resp_headers, &(&1 == {"x-robots-tag", "noindex"}))
   end
-
-  test "Assigns cookie for known flagged features via URL params", %{conn: conn} do
-    conn = Plug.Conn.fetch_cookies(conn)
-    refute conn.cookies["some_feature"]
-
-    # add a feature flag
-    Application.put_env(:laboratory, :features, [
-      {:some_feature, "", ""}
-      | Application.get_env(:laboratory, :features)
-    ])
-
-    assert get(conn, "/?active_flag=some_feature") |> Laboratory.enabled?(:some_feature)
-    refute get(conn, "/?active_flag=unknown_feature") |> Laboratory.enabled?(:unknown_feature)
-  end
 end