From e967742c632ee990fb022e1cde5ccfb2f4438546 Mon Sep 17 00:00:00 2001
From: Ryan Kendall <ryankendallt@gmail.com>
Date: Thu, 2 Nov 2023 15:00:42 +0000
Subject: [PATCH] edit evaluations

---
 .../frameworks/application_controller.rb      |  2 +-
 .../evaluation_contacts_controller.rb         |  2 --
 .../evaluations/contacts_controller.rb        |  5 +++++
 .../frameworks/evaluations_controller.rb      | 19 +++++++++++++++--
 .../frameworks/evaluation/quick_editable.rb   |  2 +-
 .../activity_log_items/_history.html.erb      |  4 ++--
 .../evaluations/_evaluation_started.html.erb  |  0
 .../evaluations/_note_added.html.erb          |  2 +-
 .../frameworks/_category_added.html.erb       |  4 ++--
 .../frameworks/_evaluation_started.html.erb   |  6 +-----
 .../frameworks/evaluations/_form.html.erb     | 21 +++++++++++++++++++
 .../frameworks/evaluations/edit.html.erb      |  8 +++++++
 app/views/frameworks/evaluations/new.html.erb | 15 +------------
 .../frameworks/evaluations/show.html.erb      |  2 +-
 .../evaluations/show/_framework.html.erb      |  4 +++-
 ...nt_can_create_framework_evaluation_spec.rb |  2 +-
 ...ent_can_browse_frameworks_register_spec.rb |  1 -
 .../agent_can_categorise_framework_spec.rb    |  5 ++---
 18 files changed, 67 insertions(+), 37 deletions(-)
 delete mode 100644 app/controllers/frameworks/evaluation_contacts_controller.rb
 create mode 100644 app/controllers/frameworks/evaluations/contacts_controller.rb
 delete mode 100644 app/views/frameworks/activity_log_items/activity/activity_event/evaluations/_evaluation_started.html.erb
 create mode 100644 app/views/frameworks/evaluations/_form.html.erb
 create mode 100644 app/views/frameworks/evaluations/edit.html.erb

diff --git a/app/controllers/frameworks/application_controller.rb b/app/controllers/frameworks/application_controller.rb
index dcb0564e0..118248493 100644
--- a/app/controllers/frameworks/application_controller.rb
+++ b/app/controllers/frameworks/application_controller.rb
@@ -8,6 +8,6 @@ def authorize_agent_scope = :access_frameworks_portal?
   def portal_namespace = :frameworks
 
   def set_back_url
-    @back_url = back_link_param if back_link_param.present?
+    @back_url = back_link_param || request.referer
   end
 end
diff --git a/app/controllers/frameworks/evaluation_contacts_controller.rb b/app/controllers/frameworks/evaluation_contacts_controller.rb
deleted file mode 100644
index 1ea0bd09f..000000000
--- a/app/controllers/frameworks/evaluation_contacts_controller.rb
+++ /dev/null
@@ -1,2 +0,0 @@
-class Frameworks::EvaluationContactsController < Frameworks::ApplicationController
-end
diff --git a/app/controllers/frameworks/evaluations/contacts_controller.rb b/app/controllers/frameworks/evaluations/contacts_controller.rb
new file mode 100644
index 000000000..ee38515c8
--- /dev/null
+++ b/app/controllers/frameworks/evaluations/contacts_controller.rb
@@ -0,0 +1,5 @@
+class Frameworks::Evaluations::ContactsController < Frameworks::ApplicationController
+  def edit; end
+
+  def update; end
+end
diff --git a/app/controllers/frameworks/evaluations_controller.rb b/app/controllers/frameworks/evaluations_controller.rb
index 4bdf2e547..ef385a23e 100644
--- a/app/controllers/frameworks/evaluations_controller.rb
+++ b/app/controllers/frameworks/evaluations_controller.rb
@@ -1,6 +1,6 @@
 class Frameworks::EvaluationsController < Frameworks::ApplicationController
   before_action :redirect_to_register_tab, unless: :turbo_frame_request?, only: :index
-  before_action :set_form_options, only: %i[new create]
+  before_action :set_form_options, only: %i[new edit create update]
 
   content_security_policy do |policy|
     policy.style_src_attr :unsafe_inline
@@ -30,11 +30,26 @@ def create
     end
   end
 
+  def edit
+    @evaluation = Frameworks::Evaluation.find(params[:id])
+  end
+
+  def update
+    @evaluation = Frameworks::Evaluation.find(params[:id])
+
+    if @evaluation.update(evaluation_params)
+      redirect_to @evaluation
+    else
+      render :edit
+    end
+  end
+
 private
 
   def set_form_options
     @frameworks = Frameworks::Framework.for_evaluation
     @agents = Support::Agent.framework_evaluators
+    @contacts = Frameworks::ProviderContact.all
   end
 
   def filter_form_params
@@ -45,7 +60,7 @@ def filter_form_params
   end
 
   def evaluation_params
-    params.require(:frameworks_evaluation).permit(:framework_id, :assignee_id)
+    params.require(:frameworks_evaluation).permit(:framework_id, :assignee_id, :contact_id)
   end
 
   def redirect_to_register_tab
diff --git a/app/models/frameworks/evaluation/quick_editable.rb b/app/models/frameworks/evaluation/quick_editable.rb
index 7f33cb937..afe084e07 100644
--- a/app/models/frameworks/evaluation/quick_editable.rb
+++ b/app/models/frameworks/evaluation/quick_editable.rb
@@ -7,7 +7,7 @@ def quick_editor(note: latest_note&.body, next_key_date: self.next_key_date, nex
 
   def quick_edit(details)
     transaction do
-      update!(details) if details[:next_key_date_description].present?
+      update!(details.except(:note)) if details[:next_key_date_description].present?
       add_note(details[:note]) if details[:note].present? && details[:note] != latest_note&.body
     end
   end
diff --git a/app/views/frameworks/activity_log_items/_history.html.erb b/app/views/frameworks/activity_log_items/_history.html.erb
index f5838a4fd..93bb1d5d5 100644
--- a/app/views/frameworks/activity_log_items/_history.html.erb
+++ b/app/views/frameworks/activity_log_items/_history.html.erb
@@ -23,9 +23,9 @@
       </p>
 
       <% if content_for?(:"#{activity_log_item.activity_id}_description") %>
-        <p class="hods-timeline__description">
+        <div class="hods-timeline__description">
           <%= yield :"#{activity_log_item.activity_id}_description" %>
-        </p>
+        </div>
       <% end %>
     </div>
   <% end %>
diff --git a/app/views/frameworks/activity_log_items/activity/activity_event/evaluations/_evaluation_started.html.erb b/app/views/frameworks/activity_log_items/activity/activity_event/evaluations/_evaluation_started.html.erb
deleted file mode 100644
index e69de29bb..000000000
diff --git a/app/views/frameworks/activity_log_items/activity/activity_event/evaluations/_note_added.html.erb b/app/views/frameworks/activity_log_items/activity/activity_event/evaluations/_note_added.html.erb
index 99afdee5a..879c528ab 100644
--- a/app/views/frameworks/activity_log_items/activity/activity_event/evaluations/_note_added.html.erb
+++ b/app/views/frameworks/activity_log_items/activity/activity_event/evaluations/_note_added.html.erb
@@ -3,5 +3,5 @@
 <% end %>
 
 <% content_for :"#{activity.id}_description" do %>
-  <%= activity.loaded_data.body %>
+  <%= simple_format(activity.loaded_data.body) %>
 <% end %>
diff --git a/app/views/frameworks/activity_log_items/activity/activity_event/frameworks/_category_added.html.erb b/app/views/frameworks/activity_log_items/activity/activity_event/frameworks/_category_added.html.erb
index 408a2bc0f..d77e1d2be 100644
--- a/app/views/frameworks/activity_log_items/activity/activity_event/frameworks/_category_added.html.erb
+++ b/app/views/frameworks/activity_log_items/activity/activity_event/frameworks/_category_added.html.erb
@@ -1,3 +1,3 @@
-% content_for :"#{activity.id}_subject" do %>
-  Added category "<%= activity.loaded_data.support_category.title %>"
+<% content_for :"#{activity.id}_subject" do %>
+  Category "<%= activity.loaded_data.support_category.title %>" added
 <% end %>
diff --git a/app/views/frameworks/activity_log_items/activity/activity_event/frameworks/_evaluation_started.html.erb b/app/views/frameworks/activity_log_items/activity/activity_event/frameworks/_evaluation_started.html.erb
index f714f8b21..02fe09338 100644
--- a/app/views/frameworks/activity_log_items/activity/activity_event/frameworks/_evaluation_started.html.erb
+++ b/app/views/frameworks/activity_log_items/activity/activity_event/frameworks/_evaluation_started.html.erb
@@ -1,7 +1,3 @@
 <% content_for :"#{activity.id}_subject" do %>
-  Evaluation started
-<% end %>
-
-<% content_for :"#{activity.id}_description" do %>
-  <%= link_to "View Evalutaion #{activity.loaded_data.evaluation.reference}", activity.loaded_data.evaluation, class: "govuk-link", "data-turbo" => false %>
+  Evaluation started: <%= link_to activity.loaded_data.evaluation.reference, activity.loaded_data.evaluation, class: "govuk-link", "data-turbo" => false %>
 <% end %>
diff --git a/app/views/frameworks/evaluations/_form.html.erb b/app/views/frameworks/evaluations/_form.html.erb
new file mode 100644
index 000000000..e3b3ae28d
--- /dev/null
+++ b/app/views/frameworks/evaluations/_form.html.erb
@@ -0,0 +1,21 @@
+<% edit ||= false %>
+
+<%= form_for @evaluation do |f| %>
+  <%= f.govuk_select :framework_id, @frameworks.map {|framework| ["#{framework.reference_and_name} (#{framework.provider_name})", framework.id] },
+      disabled: edit,
+      options: { include_blank: "Select a framework" },
+      label: { text: "Framework", size: "m" } %>
+
+  <%= f.govuk_select :contact_id, @contacts.map {|contact| ["#{contact.name} (#{contact.provider_name})", contact.id] },
+      options: { include_blank: "Select a contact" },
+      label: { text: "Contact at provider", size: "m" } %>
+
+  <%= f.govuk_select :assignee_id, @agents.map {|agent| [agent.full_name, agent.id] },
+        options: { include_blank: "Select an agent" },
+        label: { text: "Case Owner", size: "m" } %>
+
+  <div class="govuk-button-group flex-align-center">
+    <%= f.govuk_submit edit ? "Save changes" : "Create evaluation" %>
+    <%= link_to "Cancel", @back_url, class: "govuk-link govuk-link--no-visited-state" %>
+  </div>
+<% end %>
diff --git a/app/views/frameworks/evaluations/edit.html.erb b/app/views/frameworks/evaluations/edit.html.erb
new file mode 100644
index 000000000..fd6b9029b
--- /dev/null
+++ b/app/views/frameworks/evaluations/edit.html.erb
@@ -0,0 +1,8 @@
+<%= content_for :title, "GHBS | Frameworks | Evaluation | #{@evaluation.reference} | Edit" %>
+
+<span class="govuk-caption-l">[<%= @evaluation.reference %>] Framework Evaluation</span>
+<h1 class="govuk-heading-l">Edit Basic Details</h1>
+
+<div class="govuk-!-width-two-thirds">
+  <%= render "form", edit: true %>
+</div>
diff --git a/app/views/frameworks/evaluations/new.html.erb b/app/views/frameworks/evaluations/new.html.erb
index 818343c60..477388515 100644
--- a/app/views/frameworks/evaluations/new.html.erb
+++ b/app/views/frameworks/evaluations/new.html.erb
@@ -3,18 +3,5 @@
 <h1 class="govuk-heading-l">New Framework Evaluation</h1>
 
 <div class="govuk-!-width-two-thirds">
-  <%= form_for @evaluation do |f| %>
-    <%= f.govuk_select :framework_id, @frameworks.map {|framework| [framework.reference_and_name, framework.id] },
-          options: { include_blank: "Select a framework" },
-          label: { text: "Framework" } %>
-
-    <%= f.govuk_select :assignee_id, @agents.map {|agent| [agent.full_name, agent.id] },
-          options: { include_blank: "Select an agent" },
-          label: { text: "Assignee" } %>
-
-    <div class="govuk-button-group flex-align-center">
-      <%= f.govuk_submit "Create evaluation" %>
-      <%= link_to "Cancel", @back_url, class: "govuk-link govuk-link--no-visited-state" %>
-    </div>
-  <% end %>
+  <%= render "form" %>
 </div>
diff --git a/app/views/frameworks/evaluations/show.html.erb b/app/views/frameworks/evaluations/show.html.erb
index 7d729167b..10d8bc49c 100644
--- a/app/views/frameworks/evaluations/show.html.erb
+++ b/app/views/frameworks/evaluations/show.html.erb
@@ -10,7 +10,7 @@
       <span><strong class="govuk-tag govuk-tag--red">ACTION</strong></span>
     <% end %>
     <span><%= @evaluation.display_status %></span>
-    <span class="govuk-body">Case Owner: <%= @evaluation.display_assignee %></span>
+    <span class="govuk-body">Case Owner: <%= @evaluation.display_assignee %> (<%= link_to "Change", edit_frameworks_evaluation_path(@evaluation, back_to: current_url_b64, redirect_back: current_url_b64), class: "govuk-link govuk-link--no-visited-state", "target" => "_top" %>)</span>
   </div>
 
   <div class="govuk-grid-row">
diff --git a/app/views/frameworks/evaluations/show/_framework.html.erb b/app/views/frameworks/evaluations/show/_framework.html.erb
index ea266b3bb..45847571d 100644
--- a/app/views/frameworks/evaluations/show/_framework.html.erb
+++ b/app/views/frameworks/evaluations/show/_framework.html.erb
@@ -18,7 +18,9 @@
         <%= link_to @evaluation.contact_name, frameworks_provider_contact_path(@evaluation.contact, back_to: current_url_b64(:framework)), class: "govuk-link", "data-turbo" => false %>
       <% end %>
     </dd>
-    <dd class="govuk-summary-list__actions"><%= link_to "Change", "#", class: "govuk-link", "data-turbo" => false %></dd>
+    <dd class="govuk-summary-list__actions">
+      <%= link_to "Change", edit_frameworks_evaluation_path(@evaluation, back_to: current_url_b64, redirect_back: current_url_b64), class: "govuk-link govuk-link--no-visited-state", "target" => "_top" %>
+    </dd>
   </div>
 
   <div class="govuk-summary-list__row">
diff --git a/spec/features/frameworks/evaluations/agent_can_create_framework_evaluation_spec.rb b/spec/features/frameworks/evaluations/agent_can_create_framework_evaluation_spec.rb
index 108f20d0d..a60a07b50 100644
--- a/spec/features/frameworks/evaluations/agent_can_create_framework_evaluation_spec.rb
+++ b/spec/features/frameworks/evaluations/agent_can_create_framework_evaluation_spec.rb
@@ -9,7 +9,7 @@
     visit frameworks_framework_path(framework)
     click_on "Evaluations"
     click_on "Add Evaluation"
-    select agent.full_name, from: "Assignee"
+    select agent.full_name, from: "Case Owner"
 
     expect { click_on "Create evaluation" }.to change { framework.reload.evaluations.count }.from(0).to(1)
   end
diff --git a/spec/features/frameworks/register/agent_can_browse_frameworks_register_spec.rb b/spec/features/frameworks/register/agent_can_browse_frameworks_register_spec.rb
index 5a4efc599..c80f64862 100644
--- a/spec/features/frameworks/register/agent_can_browse_frameworks_register_spec.rb
+++ b/spec/features/frameworks/register/agent_can_browse_frameworks_register_spec.rb
@@ -22,7 +22,6 @@
 
   it "can view the details of a framework" do
     visit frameworks_root_path
-    puts Frameworks::ActivityLogItem.pluck :activity_type
     click_on "books"
     expect(page).to have_title("Books & Stationary")
   end
diff --git a/spec/features/frameworks/register/agent_can_categorise_framework_spec.rb b/spec/features/frameworks/register/agent_can_categorise_framework_spec.rb
index ab8fbee4a..6075c1284 100644
--- a/spec/features/frameworks/register/agent_can_categorise_framework_spec.rb
+++ b/spec/features/frameworks/register/agent_can_categorise_framework_spec.rb
@@ -17,8 +17,7 @@
     click_on "Save changes"
 
     expect(page).to have_summary("Categories", "Laptops, Electricity")
-    click_on "History"
-    expect(page).to have_content('Added category "Laptops" to framework')
-    expect(page).to have_content('Added category "Electricity" to framework')
+    expect(page).to have_content('Category "Laptops" added')
+    expect(page).to have_content('Category "Electricity" added')
   end
 end