Implement and test update endpoint

Author: VeljkoMaksimovic

ee/ephemeral_environments/lib/ephemeral_environments/grpc/ephemeral_environments_server.ex

@@ -53,8 +53,17 @@ defmodule EphemeralEnvironments.Grpc.EphemeralEnvironmentsServer do
     end
   end
 
-  def update(_request, _stream) do
-    %UpdateResponse{}
+  def update(request, _stream) do
+    case EphemeralEnvironments.Service.EphemeralEnvironmentType.update(request.environment_type) do
+      {:ok, environment_type} ->
+        %{environment_type: environment_type}
+
+      {:error, :not_found} ->
+        raise GRPC.RPCError, status: :not_found, message: "Environment type not found"
+
+      {:error, error_message} ->
+        raise GRPC.RPCError, status: :unknown, message: error_message
+    end
   end
 
   def delete(_request, _stream) do

ee/ephemeral_environments/lib/ephemeral_environments/service/ephemeral_environment_type.ex

@@ -43,6 +43,70 @@ defmodule EphemeralEnvironments.Service.EphemeralEnvironmentType do
     end
   end
 
+  @doc """
+  Updates an existing ephemeral environment type.
+
+  ## Parameters
+    - attrs: Map with keys:
+      - id (required)
+      - org_id (required)
+      - last_updated_by (required)
+      - name (optional)
+      - description (optional)
+      - max_number_of_instances (optional)
+      - state (optional)
+
+  ## Returns
+    - {:ok, map} on success
+    - {:error, :not_found} if the environment type doesn't exist
+    - {:error, String.t()} on validation failure
+  """
+  def update(attrs) do
+    # Filter out proto default values that shouldn't be updated
+    attrs = filter_proto_defaults(attrs)
+
+    with {:ok, record} <- get_record(attrs[:id], attrs[:org_id]),
+         {:ok, updated_record} <- update_record(record, attrs) do
+      {:ok, struct_to_map(updated_record)}
+    end
+  end
+
+  # Remove proto default values that indicate "not set" rather than explicit values
+  defp filter_proto_defaults(attrs) do
+    attrs
+    |> Enum.reject(fn
+      # Empty strings from proto mean "not set"
+      {_key, ""} -> true
+      # :unspecified enum means "not set"
+      {:state, :unspecified} -> true
+      # 0 for max_number_of_instances means "not set" (since validation requires > 0)
+      {:max_number_of_instances, 0} -> true
+      # Keep everything else
+      _ -> false
+    end)
+    |> Map.new()
+  end
+
+  defp get_record(id, org_id) when is_binary(id) and is_binary(org_id) do
+    Schema
+    |> where([e], e.id == ^id and e.org_id == ^org_id)
+    |> Repo.one()
+    |> case do
+      nil -> {:error, :not_found}
+      record -> {:ok, record}
+    end
+  end
+
+  defp update_record(record, attrs) do
+    record
+    |> Schema.changeset(attrs)
+    |> Repo.update()
+    |> case do
+      {:ok, updated_record} -> {:ok, updated_record}
+      {:error, changeset} -> {:error, format_errors(changeset)}
+    end
+  end
+
   @doc """
   Creates a new ephemeral environment type.
 
@@ -92,12 +156,19 @@ defmodule EphemeralEnvironments.Service.EphemeralEnvironmentType do
   defp format_errors(changeset) do
     Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} ->
       Enum.reduce(opts, msg, fn {key, value}, acc ->
-        String.replace(acc, "%{#{key}}", to_string(value))
+        String.replace(acc, "%{#{key}}", safe_to_string(value))
       end)
     end)
     |> Enum.map(fn {field, errors} ->
       "#{field}: #{Enum.join(errors, ", ")}"
     end)
     |> Enum.join("; ")
   end
+
+  # Safely convert values to strings, handling complex types
+  defp safe_to_string(value) when is_binary(value), do: value
+  defp safe_to_string(value) when is_atom(value), do: to_string(value)
+  defp safe_to_string(value) when is_number(value), do: to_string(value)
+  defp safe_to_string(value) when is_list(value), do: inspect(value)
+  defp safe_to_string(value), do: inspect(value)
 end

ee/ephemeral_environments/test/grpc/ephemeral_environments_server_test.exs

@@ -10,7 +10,8 @@ defmodule EphemeralEnvironments.Grpc.EphemeralEnvironmentsServerTest do
     EphemeralEnvironmentType,
     EphemeralEnvironments,
     ListRequest,
-    DescribeRequest
+    DescribeRequest,
+    UpdateRequest
   }
 
   @org_id Ecto.UUID.generate()
@@ -199,6 +200,206 @@ defmodule EphemeralEnvironments.Grpc.EphemeralEnvironmentsServerTest do
   end
 
   describe "update/2" do
+    test "updates environment type successfully", %{channel: channel} do
+      {:ok, env_type} =
+        Factories.EphemeralEnvironmentsType.insert(
+          org_id: @org_id,
+          name: "Original Name",
+          description: "Original description",
+          created_by: @user_id,
+          state: :draft,
+          max_number_of_instances: 5
+        )
+
+      updater_id = Ecto.UUID.generate()
+
+      request = %UpdateRequest{
+        environment_type: %EphemeralEnvironmentType{
+          id: env_type.id,
+          org_id: @org_id,
+          name: "Updated Name",
+          description: "Updated description",
+          last_updated_by: updater_id,
+          state: :TYPE_STATE_READY,
+          max_number_of_instances: 10
+        }
+      }
+
+      {:ok, response} = EphemeralEnvironments.Stub.update(channel, request)
+
+      assert response.environment_type.id == env_type.id
+      assert response.environment_type.org_id == @org_id
+      assert response.environment_type.name == "Updated Name"
+      assert response.environment_type.description == "Updated description"
+      assert response.environment_type.last_updated_by == updater_id
+      assert response.environment_type.state == :TYPE_STATE_READY
+      assert response.environment_type.max_number_of_instances == 10
+      # created_by should remain unchanged
+      assert response.environment_type.created_by == @user_id
+
+      # Verify database record was updated
+      db_record = Repo.get(Schema, env_type.id)
+      assert db_record.name == "Updated Name"
+      assert db_record.description == "Updated description"
+      assert db_record.last_updated_by == updater_id
+      assert db_record.state == :ready
+    end
+
+    test "updates only provided fields", %{channel: channel} do
+      {:ok, env_type} =
+        Factories.EphemeralEnvironmentsType.insert(
+          org_id: @org_id,
+          name: "Original Name",
+          description: "Original description",
+          created_by: @user_id,
+          state: :draft,
+          max_number_of_instances: 5
+        )
+
+      updater_id = Ecto.UUID.generate()
+
+      # Only update name and last_updated_by
+      request = %UpdateRequest{
+        environment_type: %EphemeralEnvironmentType{
+          id: env_type.id,
+          org_id: @org_id,
+          name: "New Name",
+          last_updated_by: updater_id
+        }
+      }
+
+      {:ok, response} = EphemeralEnvironments.Stub.update(channel, request)
+
+      assert response.environment_type.name == "New Name"
+      assert response.environment_type.last_updated_by == updater_id
+      # Other fields should remain unchanged
+      assert response.environment_type.description == "Original description"
+      assert response.environment_type.state == :TYPE_STATE_DRAFT
+      assert response.environment_type.max_number_of_instances == 5
+    end
+
+    test "returns not_found when environment type doesn't exist", %{channel: channel} do
+      non_existent_id = Ecto.UUID.generate()
+
+      request = %UpdateRequest{
+        environment_type: %EphemeralEnvironmentType{
+          id: non_existent_id,
+          org_id: @org_id,
+          name: "Updated Name",
+          last_updated_by: @user_id
+        }
+      }
+
+      assert {:error, %GRPC.RPCError{status: 5, message: "Environment type not found"}} =
+               EphemeralEnvironments.Stub.update(channel, request)
+    end
+
+    test "returns not_found when updating with wrong org_id", %{channel: channel} do
+      {:ok, env_type} =
+        Factories.EphemeralEnvironmentsType.insert(
+          org_id: @org_id,
+          name: "Test Environment"
+        )
+
+      different_org_id = Ecto.UUID.generate()
+
+      request = %UpdateRequest{
+        environment_type: %EphemeralEnvironmentType{
+          id: env_type.id,
+          org_id: different_org_id,
+          name: "Updated Name",
+          last_updated_by: @user_id
+        }
+      }
+
+      assert {:error, %GRPC.RPCError{status: 5, message: "Environment type not found"}} =
+               EphemeralEnvironments.Stub.update(channel, request)
+    end
+
+    test "fails validation when updating with duplicate name in same org", %{channel: channel} do
+      {:ok, env1} =
+        Factories.EphemeralEnvironmentsType.insert(
+          org_id: @org_id,
+          name: "Environment 1"
+        )
+
+      {:ok, env2} =
+        Factories.EphemeralEnvironmentsType.insert(
+          org_id: @org_id,
+          name: "Environment 2"
+        )
+
+      # Try to rename env2 to env1's name
+      request = %UpdateRequest{
+        environment_type: %EphemeralEnvironmentType{
+          id: env2.id,
+          org_id: @org_id,
+          name: "Environment 1",
+          last_updated_by: @user_id
+        }
+      }
+
+      {:error, error} = EphemeralEnvironments.Stub.update(channel, request)
+      assert %GRPC.RPCError{} = error
+      # UNKNOWN
+      assert error.status == 2
+      assert error.message == "duplicate_name: ephemeral environment name has already been taken"
+    end
+
+    test "allows updating to same name in different org", %{channel: channel} do
+      {:ok, env1} =
+        Factories.EphemeralEnvironmentsType.insert(
+          org_id: @org_id,
+          name: "Shared Name"
+        )
+
+      other_org_id = Ecto.UUID.generate()
+
+      {:ok, env2} =
+        Factories.EphemeralEnvironmentsType.insert(
+          org_id: other_org_id,
+          name: "Original Name"
+        )
+
+      # Update env2 to use the same name as env1 (but different org)
+      request = %UpdateRequest{
+        environment_type: %EphemeralEnvironmentType{
+          id: env2.id,
+          org_id: other_org_id,
+          name: "Shared Name",
+          last_updated_by: @user_id
+        }
+      }
+
+      assert {:ok, response} = EphemeralEnvironments.Stub.update(channel, request)
+      assert response.environment_type.name == "Shared Name"
+    end
+
+    test "updates timestamp when updating", %{channel: channel} do
+      {:ok, env_type} = Factories.EphemeralEnvironmentsType.insert(org_id: @org_id)
+
+      # Wait a bit to ensure timestamp changes
+      :timer.sleep(100)
+
+      request = %UpdateRequest{
+        environment_type: %EphemeralEnvironmentType{
+          id: env_type.id,
+          org_id: @org_id,
+          name: "Updated Name",
+          last_updated_by: @user_id
+        }
+      }
+
+      {:ok, response} = EphemeralEnvironments.Stub.update(channel, request)
+
+      # created_at should be the original timestamp
+      original_created_at = DateTime.from_naive!(env_type.inserted_at, "Etc/UTC")
+      response_created_at = DateTime.from_unix!(response.environment_type.created_at.seconds)
+      assert DateTime.diff(response_created_at, original_created_at, :second) == 0
+
+      # updated_at should be recent
+      assert_recent_timestamp(DateTime.from_unix!(response.environment_type.updated_at.seconds))
+    end
   end
 
   describe "delete/2" do