camstuart

Help with idiomatic Elixir style when dealing with logic flow

Hello,

I am somewhat new to Elixir, and finding that I am having difficulty grasping how I should handle logic for a series of sequences in an “operation”. For example, I have a phoenix post controller that I am using to onboard an organisation and user. So there are a few steps.

Verify where the request came from (I know this is not perfect)
Create an organisation if one does not already exist
Create a user for this organisation if one does not already exist

In this example I am relying on “halt” to essentially return early. But I think I might be approaching the problem in Elixir like an imperative language. But in a regular function where such a mechanism does (no return statement in the language) I get a bit lost on how I should manage control flow. This seems it should be broken up somehow.

I also have ended up with a rather “nested” outcome, which perhaps could (or should?) be avoided with that cool pipeline operator, but I don’t know how I would handle the unhappy path nicely.

I would really appreciate some feedback, and any learning resources that help people like me who have been working in imperative languages so long that we have trouble breaking the habit! Thanks for reading!

def onboard(conn, params) do
    case verify_zendesk_origin(conn) do
      {:ok, _origin} ->
        user_params = params["user"]

        organisation_attrs = %{
          subdomain: params["subdomain"],
          name: params["name"],
          public_key: params["public_key"]
        }

        case Organisations.upsert(organisation_attrs) do
          {:ok, organisation} ->
            user_attrs = %{
              external_id: to_string(user_params["id"]),
              name: user_params["name"],
              role: user_params["role"],
              avatar_url: user_params["avatarUrl"],
              organisation_id: organisation.id
            }

            case ExternalAccounts.upsert_user(user_attrs) do
              {:ok, user} ->
                conn
                |> put_status(:ok)
                |> json(%{user_id: user.id})
                |> halt()

              {:error, changeset} ->
                IO.inspect(changeset.errors, label: "user upsert (onboard) changeset errors")

                conn
                |> put_status(:unprocessable_entity)
                |> json(%{
                  error: "invalid user data",
                  details: changeset_error_to_string(changeset)
                })
                |> halt()
            end

          {:error, changeset} ->
            IO.inspect(changeset.errors, label: "organisation upsert (onboard) changeset errors")

            conn
            |> put_status(:unprocessable_entity)
            |> json(%{
              error: "invalid organisation data",
              details: changeset_error_to_string(changeset)
            })
            |> halt()
        end

      {:error, _reason} ->
        conn
        |> put_status(:forbidden)
        |> json(%{error: "Invalid origin"})
        |> halt()
    end
  end

31 comments

/phoenix #code-style

37 859 31

2024-09-27 11:15:11 UTC

Marked As Solved

Lucassifoni

The logic could be extracted to an use-case module.
The origin verification could also be a plug.

At a very high and very verbose level :


plug YourAppWeb.Plugs, :verify_zendesk_origin when action in [:onboard, ...]
alias YourAppWeb.UserFlows.OnboardUser

def onboard(conn, params) do
    with {_, {:ok, organisation}} <- {:upsert_organisation, OnboardUser.upsert_organisation(conn.assigns.organisation_attrs)},
            {_, {:ok, user_attrs}} <- {:user_attrs, OnboardUser.user_attrs_from_params_and_org(params, organisation)}
            {_, {:ok, user}} <- {:upsert_external_user, OnboardUser.upsert_external_user(user_attrs)} do
         conn |> put_status(:ok) |> json(%{user_id: user.id})
   else
     {:upsert_organisation, {:error, e}} -> # handle this case
     {:user_attrs, {:error, e}} -> #handle this one
     {:upsert_external_user, {:error, e}} # handle this one
     _ -> # etc
    end
end

I’ve put this fictional OnboardUser module in YourAppWeb because the helper function user_attrs_from_params_and_org depends on the params arg, so it is linked to the transport.
You can of course refine that and have a pure non-web logic OnboardUser use-case while having other extracted utilities to properly construct the arguments it consumes from the request.

There also are a few different ways to tag error tuples or triples with with.
I like to do it at the call site to keep the logic free of this tagging.

The more non-web your logic is, the more testable it becomes

Edit : use-case based modules are an opinionated choice and not the idiomatic choice.

Post #3

Also Liked

dimitarvp

I agree that it’s noisy, it just became my default because very often I had to consume APIs where I couldn’t influence the return values of the functions (hence your {:organisation_error, error_information} was often not possible to have). To me with plus tagging seemed like the better cope.

We had lengthy discussions on the forum in the past and I could see the perceived benefits of making smaller functions that call the 3rd party API and slightly reshape their returns (very often you would want to convert :error to {:error, :cannot_parse_url} for example) but I could not see the objectively demonstrable value of (1) adding extra functions, (2) reshaping the return values of the wrapped API just so we emulate an extra atom in a tuple in a with chain. It was just a preference that many people stated that they have but I was still left unconvinced because, again, it was just a preference.

I have to be honest here, the older I get the more I want LESS freedom for anyone in programming to just do stuff as they prefer – myself included, I made quite some impressive messes many times and really should have been slapped back to other techniques.

Post #12

Hermanverschooten

I would definitely go for a with in this case. Take a look at this anti-pattern, and use it as an example. Create functions that return either an :ok-tuple or a specific :error-tuple (or triplet) for that case.

Post #2

stefanluptak

I would probably do something like this.

Usually, there are few types of errors that your web layer will handle.

Something like:

{:error, :not_found}
{:error, changeset}
{:error, "Some error message as string"}

If your context functions always return these, you can have your error handling in the fallback controller and then just do those nice with {:ok, something} <- YourContext.some_fun(params) calls. And if there’s some exception to that, you can handle it in the else clause of the with statement.

Post #8