Having trouble with multiple if clauses

[ppetrov]

Hello guys, noobish question here.

I am trying to build a system where users will invite other users to their organizations. So for that purpose i have the following schemas:

Users - self explaning

Organizations - the actual objects.

Organisation Admins - the original admin is added when the Organization object is created. Additional will come with invitation

System Messages - the schema for the invitations.

The workflow i am trying to achieve is the following

The page has a form with single text field and “Invite“ button. I am collecting the email of the user. My “save” event need to perform several checks before the record in the invitation is actually inserted. So i need to check

1. Is there an active invitation at the moment? i.e. invitation which is not complete.
2. Is there an admin with the same email or user-id already?
3. is the user present in the DB? (if not i do display the same message after the email submission but it doesn’t do anything)
4. If all of the above are not true - perform the invitation insert in the database.

So i have tried multiple consecutive conditions with “if..else“ and also nested but i cannot make the invitation check to trigger for some reason.

Here is sample of the code:

the liveview save event:

    def handle_event("save", %{"organization_admin" => organization_admin_params}, socket) do

        %{"email" => email} = organization_admin_params
        user_data = UserChecks.get_user_by_email(email)

        if (UserChecks.check_invitation_status(socket.assigns.org_id, user_data.id) != nil) do
           socket =
            socket
            |>put_flash(:info, "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA")
            |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")
          {:noreply, socket}
        end

        user_to_invite = UserCreates.invite_admin_to_org(socket.assigns.org_id, user_data.id)
        UserCreates.send_org_admin_invitation(socket.assigns.org_id, user_to_invite.id, socket.assigns.current_scope.user.id)
            socket =
              socket
              |>put_flash(:info, "If the user exists in our database they will receive your invitation.")
              |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")
            {:noreply, socket}

end

and the database query:

def check_invitation_status(orgid, userid) do
  Repo.all_by(SysMessage, recipient: userid)
end

The DB query is just an example. It should list the messages for that particular user and then redirect the user. I have more detailed query which reacts the same.

The question here is what i am missing? I suspect that if..else does not work the way i am understanding it. I have checked the documentation but can’t figure out how to make it work. Also read one of the posts here where NoobZ explains it with consecutive def lines and maybe i can do as well but i need help to understand where i am doing it wrong.

I hope that makes sense.

[NobbZ]

Its 2 B and 1 O

Though I have no clue what post(s) of mine you are refering to. “def” and “if” are barely related.

I also do not really understand your current problem or how it is related to “mutliple if clauses”. An “if” doesn’t have clauses at all, it has a single condition, and 2 branches, whicl will be taken depending on the conditions truthines.

What I see though is, that your if expression you have there does nothing.

Even if the condition is true, and some CPU cycles are burned, there are no sideeffects triggered and the value of the expression is discarded.

[sodapopcan]

To add some context, if blocks, and all Elixir blocks, are their own closed scope, returning the last expression. The push_navigation doesn’t cause an immediate navigation, it’s annotating the socket for redirection after returning from the handle_event clause. Easiest change here is to add and else:

if UserChecks... do
  socket =
    socket
    |> socket(...)
    |> push_navigate(...)

  {:noreply, socket}
else
  user_to_invite = ...
  socket = ...

  {:noreply, socket}
end

[ppetrov]

Sorry for misspelling your name.

It was a solution you provided for similar question.

[ppetrov]

This one:

Nested conditionals

In principle, there is nothing wrong with if statements. However, you could probably leverage pattern matching to simplify your code. If you only have a handful of protocols, you could do something like this: def full_route("http://" <> url), do: "http://" <> url def full_route("https://" <> url), do: "https://" <> url def full_route("/" <> path), do: do_full_route(path) def full_route(path), do: do_full_route(path) defp do_full_route(path), do: Application.get_env(:my_app, :url) <> "/" <> st…

[ppetrov]

Here is the original code i had:

 def handle_event("save", %{"organization_admin" => organization_admin_params}, socket) do


        %{"email" => email} = organization_admin_params
        user_data = UserChecks.get_user_by_email(email)


        if(UserChecks.get_org_admin(user_data.id, socket.assigns.org_id)) do
           socket =
            socket
            |>put_flash(:info, "This user is already an admin in your organization.")
            |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")
          {:noreply, socket}
        end



        if (SysMessageQ.check_invitation_status(socket.assigns.org_id, user_data.id) != nil) do
          socket =
            socket
            |> put_flash(:warn, "The previous invitation has not been completed yet.")
            |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")
          {:noreply, socket}
        end


        if(user_data != nil and user_data.email != socket.assigns.current_scope.user.email and user_data.site_role != :admin) do

            if(UserChecks.get_org_admin(user_data.id, socket.assigns.org_id)) do
              socket =
                socket
                |> put_flash(:info, "This user is already an admin in your organization.")
                |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

              {:noreply, socket}

            else

              user_to_invite = UserCreates.invite_admin_to_org(socket.assigns.org_id, user_data.id)
              UserCreates.send_org_admin_invitation(socket.assigns.org_id, user_to_invite.id, socket.assigns.current_scope.user.id)
                  socket =
                    socket
                    |>put_flash(:info, "If the user exists in our database they will receive your invitation.")
                    |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")
                  {:noreply, socket}

            end
          else
            socket =
              socket
              |>put_flash(:info, "If the user exist in our database they will be invited. The user will appear only after accepts the invitation.")
              |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")
              {:noreply, socket}

        end

    end

It hits the function when there is an admin with the same email, it also hits the function when there is no user with such email registered. But i am struggling to hit the function where the check for the invitation will find there is one.

[paulsullivanjr]

I think may be after something like this? I’m pretty much guessing since I have no context.

def handle_event("save", %{"organization_admin" => %{"email" => email}}, socket) do
  %{org_id: org_id, current_scope: %{user: %{id: inviter_id}}} = socket.assigns
  user = UserChecks.get_user_by_email(email)

  case UserChecks.check_invitation_status(org_id, user.id) do
    nil ->
      user_to_invite = UserCreates.invite_admin_to_org(org_id, user.id)
      UserCreates.send_org_admin_invitation(org_id, user_to_invite.id, inviter_id)

      socket =
        socket
        |> put_flash(:info, "If the user exists in our database they will receive your invitation.")
        |> push_navigate(to: ~p"/org/#{org_id}/org-admins")

      {:noreply, socket}

    _existing ->
      socket =
        socket
        |> put_flash(:info, "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA")
        |> push_navigate(to: ~p"/org/#{org_id}/org-admins")

      {:noreply, socket}
  end
end

[sodapopcan]

See my answer? And @NobbZ again.

If you write:

def handle_event("...", _, socket) do
  if ... do
    socket = push_navigate(socket, to: ...)

    {:noreply, socket}
  end

  {:noreply, socket}
end

The first {:noreply, socket} won’t cause an earlier return or anything like that if that is what you are thinking. Also, the socket from the params will not pick up any changes you set in the if block. The last {:noreply, socket} contains the socket in its form when it was originally passed into the function. You’d have to do:

def handle_event("...", _, socket) do
  {:noreply, socket} =
    if ... do
      socket = push_navigate(socket, to: ...)

      {:noreply, socket}
    end

  {:noreply, socket}
end

to “mutate” the socket.

[ppetrov]

Yes, i saw your answer and it makes sense. If i understand correctly my first noreply actually doesn’t prevent the following if/else statements but it rather continues through the list until finishes and then executes the last one that returns true hence it does not breaks the operation…

Then is there a better way to write that logic?

[sodapopcan]

Yes exactly. You cannot early return from functions. As well, do blocks are their own world and cannot manipulate bindings outside of their scope.

iex> foo = %{}
%{}
iex> if true, do: foo = Map.put(foo, :bar, "baz")
%{bar: "baz"}
iex> foo
%{}

I would write it as it is in @paulsullivanjr answer. If you have more branches then the Elixir equivalent of eliseif is cond

If you are trying to accumulate different things into the socket conditionally, then you’d have to keep re-assigning socket within the function’s main scope.

socket = assign(socket, :foo, "bar")

socket =
  if some_condition? do
    assign(socket, :baz, "qux")
  else
    socket
  end

{:noreply, socket}

[al2o3cr]

cond is a useful option for chains-of-ifs like this, it lets each of its expressions “early return”. Here’s your code rewritten to use it:

def handle_event("save", %{"organization_admin" => organization_admin_params}, socket) do
  %{"email" => email} = organization_admin_params
  user_data = UserChecks.get_user_by_email(email)

  cond do
    UserChecks.get_org_admin(user_data.id, socket.assigns.org_id) ->
      socket =
        socket
        |> put_flash(:info, "This user is already an admin in your organization.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}

    SysMessageQ.check_invitation_status(socket.assigns.org_id, user_data.id) != nil ->
      socket =
        socket
        |> put_flash(:warn, "The previous invitation has not been completed yet.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}

    # NOTE: user_data can't be nil, or the previous clauses would have failed!
    user_data.email != socket.assigns.current_scope.user.email and user_data.site_role != :admin ->

      # NOTE: get_org_admin was already called and returned nil or false, skipping that branch
      user_to_invite = UserCreates.invite_admin_to_org(socket.assigns.org_id, user_data.id)
      UserCreates.send_org_admin_invitation(socket.assigns.org_id, user_to_invite.id, socket.assigns.current_scope.user.id)

      socket =
        socket
        |> put_flash(:info, "If the user exists in our database they will receive your invitation.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}

    true ->
      socket =
        socket
        |> put_flash(:info, "If the user exist in our database they will be invited. The user will appear only after accepts the invitation.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}
  end
end

Some of the conditions / branches in the original version were unreachable:

• checking user_data != nil after two other places already said user_data.id will always return true, since the code will crash otherwise
• the second call to UserChecks.get_org_admin will always return false or nil, since the first clause would have matched otherwise

Two other general notes:

• consider extracting common subexpressions like socket.assigns.org_id to variables at the start, to reduce visual clutter
• APIs that take bare IDs as positional arguments don’t give the compiler much chance of catching mistakes. Consider passing structs that can be pattern-matched or a kwlist that has clear labels. APIs that take multiple bare ID arguments should especially avoid doing it in different orders - eg get_org_admin vs check_invitation_status are inviting future “oops, arguments swapped” bugs.

[ppetrov]

Thanks! I have already worked on something like that trying to follow the guidance of @paulsullivanjr and @sodapopcan . And as you explained i am hitting these issues. Altho everyone pushed me towards the solution i will accept @al2o3cr answer. Here is the final (and tested) solution.

def handle_event("save", %{"organization_admin" => organization_admin_params}, socket) do
  %{"email" => email} = organization_admin_params
  user_data = UserChecks.get_user_by_email(email)


  cond do
    user_data == nil ->
        IO.puts("User seems to be nil")

      socket =
        socket
        |> put_flash(:info, "If the user exists in our database they will receive your invitation.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}


    UserChecks.get_org_admin(user_data.id, socket.assigns.org_id) ->
              IO.puts("User is already an admin")

      socket =
        socket
        |> put_flash(:info, "This user is already an admin in your organization.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}

    UserChecks.check_invitation_status(socket.assigns.org_id, user_data.id)  ->
        IO.puts("Hitting this branch.")

      socket =
        socket
        |> put_flash(:info, "The previous invitation has not been completed yet.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}

      user_data.email != socket.assigns.current_scope.user.email and user_data.site_role != :admin  ->

      user_to_invite = UserCreates.invite_admin_to_org(socket.assigns.org_id, user_data.id)
      UserCreates.send_org_admin_invitation(socket.assigns.org_id, user_to_invite.id, socket.assigns.current_scope.user.id)

      socket =
        socket
        |> put_flash(:info, "If the user exists in our database they will receive your invitation.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}

    true ->
      socket =
        socket
        |> put_flash(:info, "If the user exist in our database they will be invited. The user will appear only after accepts the invitation.")
        |> push_navigate(to: ~p"/org/#{socket.assigns.org_id}/org-admins")

      {:noreply, socket}
  end
end

This way it works properly with all conditions and exactly the way i wanted it to work. Same approach i will have for similar functionality adding members to the organizations which are not admins.

The only thing it did not work was the :warn message which i would say probably i am missing in the components but that is not an issue for me as i can add it later.

Thanks a lot everyone!