Merge pull request #995 from OpenFn/issue951

Ensure credentials are reloaded after a new credential was made
OpenFn · Jul 28, 2023 · 48f45b2 · 48f45b2
2 parents 505fdb2 + 29ed8bb
commit 48f45b2
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 49 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,10 @@ and this project adheres to
 
 ### Fixed
 
+- Fix issue creating a new credential from the Job editor where the new
+  credential was not being set on the job.
+  [#951](https://github.com/OpenFn/Lightning/issues/951)
+
 ## [v0.7.0-pre5] - 2023-07-28
 
 ### Added

diff --git a/lib/lightning_web/live/job_live/credential_picker.ex b/lib/lightning_web/live/job_live/credential_picker.ex
@@ -8,10 +8,10 @@ defmodule LightningWeb.JobLive.CredentialPicker do
 
   alias LightningWeb.Components.Form
 
-  attr :form, :map, required: true
-  attr :disabled, :boolean, default: false
-  attr :credentials, :list, required: true
-  attr :on_change, :any, default: nil
+  attr(:form, :map, required: true)
+  attr(:disabled, :boolean, default: false)
+  attr(:credentials, :list, required: true)
+  attr(:on_change, :any, default: nil)
 
   @impl true
   def render(assigns) do
@@ -28,14 +28,12 @@ defmodule LightningWeb.JobLive.CredentialPicker do
         form={@form}
         field={:project_credential_id}
         title="Credential"
-        for="credentialField"
         tooltip="If the system you're working with requires authentication, choose a credential with login details (secrets) that will allow this job to connect. If you're not connecting to an external system you don't need a credential."
       />
       <%= error_tag(@form, :project_credential_id, class: "block w-full") %>
       <Form.select_field
         form={@form}
         name={:project_credential_id}
-        id="credentialField"
         prompt=""
         values={@credential_options}
         disabled={@disabled}
@@ -56,13 +54,39 @@ defmodule LightningWeb.JobLive.CredentialPicker do
     """
   end
 
+  @impl true
+  def mount(socket) do
+    {:ok, socket |> assign(credentials: [], selected_project_credential_id: nil)}
+  end
+
   @impl true
   def update(%{project_user: project_user} = assigns, socket) do
     socket =
       socket
       |> assign(assigns)
-      |> assign_new(:credentials, fn ->
-        Lightning.Projects.list_project_credentials(project_user.project)
+      |> update(:selected_project_credential_id, fn _, %{form: form} ->
+        form.source |> Ecto.Changeset.get_field(:project_credential_id)
+      end)
+      |> then(fn socket ->
+        %{
+          credentials: credentials,
+          selected_project_credential_id: selected_project_credential_id
+        } = socket.assigns
+
+        selected_in_list? =
+          credentials
+          |> Enum.any?(&match?(%{id: ^selected_project_credential_id}, &1))
+
+        if selected_in_list? &&
+             !changed?(socket, :selected_project_credential_id) do
+          socket
+        else
+          socket
+          |> assign(
+            credentials:
+              Lightning.Projects.list_project_credentials(project_user.project)
+          )
+        end
       end)
 
     {:ok, socket}

diff --git a/lib/lightning_web/live/workflow_live/edit.ex b/lib/lightning_web/live/workflow_live/edit.ex
@@ -146,7 +146,7 @@ defmodule LightningWeb.WorkflowLive.Edit do
               <:footer>
                 <div class="flex flex-row">
                   <.link
-                    patch={ "#{@base_url}?s=#{@selected_job.id}&m=expand" }
+                    patch={"#{@base_url}?s=#{@selected_job.id}&m=expand"}
                     class="inline-flex items-center rounded-md bg-white px-3.5 py-2.5 text-sm font-semibold text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 hover:bg-gray-50"
                   >
                     <Heroicons.code_bracket class="w-4 h-4 -ml-0.5" />

diff --git a/test/lightning_web/live/credential_live_test.exs b/test/lightning_web/live/credential_live_test.exs
@@ -2,6 +2,7 @@ defmodule LightningWeb.CredentialLiveTest do
   use LightningWeb.ConnCase, async: false
 
   import Phoenix.LiveViewTest
+  import LightningWeb.CredentialLiveHelpers
 
   import Lightning.{
     JobsFixtures,
@@ -179,8 +180,7 @@ defmodule LightningWeb.CredentialLiveTest do
       assert new_live |> submit_disabled()
 
       assert new_live
-             |> form("#credential-form")
-             |> render_submit() =~ "can&#39;t be blank"
+             |> click_save() =~ "can&#39;t be blank"
 
       refute_redirected(new_live, ~p"/credentials")
 
@@ -203,13 +203,11 @@ defmodule LightningWeb.CredentialLiveTest do
              )
 
       assert new_live
-             |> form("#credential-form",
-               credential: %{
-                 name: "My Credential",
-                 body: %{username: "foo", password: "bar", hostUrl: "baz"}
-               }
-             )
-             |> render_change() =~ "expected to be a URI"
+             |> fill_credential(%{
+               name: "My Credential",
+               body: %{username: "foo", password: "bar", hostUrl: "baz"}
+             }) =~
+               "expected to be a URI"
 
       assert new_live
              |> form("#credential-form",
@@ -221,8 +219,7 @@ defmodule LightningWeb.CredentialLiveTest do
 
       {:ok, _index_live, _html} =
         new_live
-        |> form("#credential-form")
-        |> render_submit()
+        |> click_save()
         |> follow_redirect(conn, ~p"/credentials")
 
       {_path, flash} = assert_redirect(new_live)
@@ -252,8 +249,7 @@ defmodule LightningWeb.CredentialLiveTest do
       assert new_live |> submit_disabled()
 
       assert new_live
-             |> form("#credential-form")
-             |> render_submit() =~ "can&#39;t be blank"
+             |> click_save() =~ "can&#39;t be blank"
 
       refute_redirected(new_live, ~p"/credentials")
 
@@ -273,8 +269,7 @@ defmodule LightningWeb.CredentialLiveTest do
 
       {:ok, _index_live, _html} =
         new_live
-        |> form("#credential-form")
-        |> render_submit()
+        |> click_save()
         |> follow_redirect(
           conn,
           ~p"/credentials"
@@ -306,12 +301,8 @@ defmodule LightningWeb.CredentialLiveTest do
 
       {:ok, _index_live, html} =
         form_live
-        |> form("#credential-form", credential: @update_attrs)
-        |> render_submit()
-        |> follow_redirect(
-          conn,
-          ~p"/credentials"
-        )
+        |> click_save(%{credential: @update_attrs})
+        |> follow_redirect(conn, ~p"/credentials")
 
       {_path, flash} = assert_redirect(form_live)
       assert flash == %{"info" => "Credential updated successfully"}
@@ -987,22 +978,4 @@ defmodule LightningWeb.CredentialLiveTest do
   defp submit_disabled(live) do
     live |> has_element?("button[disabled][type=submit]")
   end
-
-  defp select_credential_type(live, type) do
-    live
-    |> form("#credential-type-picker", type: %{selected: type})
-    |> render_change()
-  end
-
-  defp click_continue(live) do
-    live
-    |> element("button", "Continue")
-    |> render_click()
-  end
-
-  defp fill_credential(live, params) when is_map(params) do
-    live
-    |> form("#credential-form", credential: params)
-    |> render_change()
-  end
 end
diff --git a/test/lightning_web/live/workflow_live/edit_test.exs b/test/lightning_web/live/workflow_live/edit_test.exs
@@ -3,6 +3,8 @@ defmodule LightningWeb.WorkflowLive.EditTest do
   import Phoenix.LiveViewTest
   import Lightning.WorkflowLive.Helpers
 
+  alias LightningWeb.CredentialLiveHelpers
+
   setup :register_and_log_in_user
   setup :create_project_for_current_user
 
@@ -53,10 +55,28 @@ defmodule LightningWeb.WorkflowLive.EditTest do
 
       view |> fill_job_fields(job, %{name: "My Job"})
 
-      refute view |> element("#adaptor-version option[selected]") |> render() =~
+      refute view |> selected_adaptor_version_element(job) |> render() =~
                ~r(value="@openfn/[a-z-]+@latest"),
              "should not have @latest selected by default"
 
+      view |> element("#new-credential-launcher") |> render_click()
+
+      view |> CredentialLiveHelpers.select_credential_type("dhis2")
+
+      view |> CredentialLiveHelpers.click_continue()
+
+      # Creating a new credential from the Job panel
+      view
+      |> CredentialLiveHelpers.fill_credential(%{
+        name: "My Credential",
+        body: %{username: "foo", password: "bar", hostUrl: "baz"}
+      })
+
+      view |> CredentialLiveHelpers.click_save()
+
+      assert view |> selected_credential(job) =~ "My Credential"
+
+      # Editing the Jobs' body
       view |> click_edit(job)
 
       view |> change_editor_text("some body")

diff --git a/test/support/credential_live_helpers.ex b/test/support/credential_live_helpers.ex
@@ -0,0 +1,27 @@
+defmodule LightningWeb.CredentialLiveHelpers do
+  import Phoenix.LiveViewTest
+
+  def select_credential_type(live, type) do
+    live
+    |> form("#credential-type-picker", type: %{selected: type})
+    |> render_change()
+  end
+
+  def click_continue(live) do
+    live
+    |> element("button", "Continue")
+    |> render_click()
+  end
+
+  def fill_credential(live, params) when is_map(params) do
+    live
+    |> form("#credential-form", credential: params)
+    |> render_change()
+  end
+
+  def click_save(live, form_data \\ %{}) do
+    live
+    |> form("#credential-form", form_data)
+    |> render_submit()
+  end
+end
diff --git a/test/support/workflow_live_helpers.ex b/test/support/workflow_live_helpers.ex
@@ -284,6 +284,16 @@ defmodule Lightning.WorkflowLive.Helpers do
     view |> element("div[phx-hook=WorkflowEditor]")
   end
 
+  def selected_adaptor_version_element(view, job) do
+    view |> element("#job-pane-#{job.id} #adaptor-version option[selected]")
+  end
+
+  def selected_credential(view, job) do
+    view
+    |> element("#job-pane-#{job.id} select[id$=credential_id] option[selected]")
+    |> render()
+  end
+
   def job_panel_element(view, job) do
     view |> element("#job-pane-#{job.id}")
   end