feat: Redirect some auth paths

[jonallured]

This PR alters force to redirect like so:

• /log_in => /login
• /sign_up => /signup

This change is motivated by some SEO guidance but it's also been an opportunity to do some refactoring which is why this PR is 12 commits, haha.

Forgetting What We Already Knew

While reading through the auth code to make this change, I found a familiar anti-pattern that I refer to as "forgetting what we already knew". Check out this routing code:

app.get("/forgot", authHandler)
app.get("/login", authHandler)
app.get("/signup", authHandler)

const authHandler = (req, res, next) = > {
  switch (req.path) {
    case "/forgot":
      type = ModalType.forgot
      break
    case "/login":
      type = ModalType.login
      break
    case "/signup":
      type = ModalType.signup
      break
  }

  // lots more code
}

This code starts by telling Express that we want to handle three paths and sends them each to a single handler. That handler then examines the path on the request to know which type we're working with. This is bananas! We already knew what type of auth we were dealing with when Express picked up the route and called the handler but then we forgot it by routing them all through a single callback!

The refactor here is to extract a handler for each path and DECLARE the values that are specific to that path and then call shared code. This allows us to remove conditionals that are specific to a particular path. Removing ifs and case statements is a great way to reduce the complexity of a piece of code.

Finding Seams When SRP is Violated

There's another, bigger problem with the code I found and it's a bit harder to see at first. When I started reading the code that handled the three requests I was overwhelmed. What was signal and what was noise? This function was violating the Single Responsibility Principle, that much was clear, but how could I improve it?

I turned to the tests hoping that they could help unravel what was going on. What I saw there were a bunch of tests asserting about the arguments being sent to a particular function call. We used jest to mock calls to stitch and then assert about the various intricacies of the object being sent to it.

Ah ha! The true responsibility of the handler function is to call stitch and we're very concerned with the shape of the arguments. This allowed me to see that there was a missing seam - we could extract a function that computes the options being sent to stitch and then reduce the noise in our handlers by encapsulating all the complexity of request parsing, etc in this compute function.

When to Stop Refactoring

I could have done so much more refactoring but I felt this was a good place to stop because it was a coherent piece of work. I wanted to make a change so I refactored to make that change easier and then I made that change. Beck would be so proud of me!

My next step will be to examine more closely how the meta part of this works because I need to set canonical tags differently. That will cause me to bump into a few things that I'm interested in improving so I can pick back up on this line of work then.

https://artsyproduct.atlassian.net/browse/GRO-18

/cc @artsy/grow-devs

[damassi]

Nice 👌

[damassi]

I ❤️ this PR so much

[dzucconi]

Oh wow, I somehow missed this PR — but this always bugged me; very nice!