Merge pull request #288 from alphagov/2530-associate-signon-user-with-web-conversation

Associate web conversation with Signon user

Author: davidgisbey

app/controllers/conversations_controller.rb

@@ -37,7 +37,7 @@ def clear_confirm
   end
 
   def update
-    @conversation ||= Conversation.new
+    @conversation ||= Conversation.new(signon_user: current_user, source: :web)
     @create_question = Form::CreateQuestion.new(user_question_params.merge(conversation: @conversation))
 
     if @create_question.valid?
@@ -111,7 +111,8 @@ def find_conversation
     return if cookies[:conversation_id].blank?
 
     @conversation = Conversation.active
-                                .find_by!(id: cookies[:conversation_id], source: :web)
+                                .where(signon_user: current_user, source: :web)
+                                .find_by!(id: cookies[:conversation_id])
     set_conversation_cookie(@conversation)
   rescue ActiveRecord::RecordNotFound
     cookies.delete(:conversation_id)

app/helpers/admin/questions_helper.rb

@@ -62,9 +62,9 @@ def question_show_summary_list_rows(question, answer, question_number, total_que
       ].compact
 
       signon_user = conversation.signon_user
-      if signon_user.present? && conversation.source_api?
+      if signon_user.present?
         rows << {
-          field: "API user",
+          field: "Signon user",
           value: safe_join([
             signon_user.name,
             " (",

spec/helpers/admin/questions_helper_spec.rb

@@ -143,10 +143,10 @@
 
     it "returns a row with a link to filter the questions table by the signon user" do
       signon_user = create(:signon_user)
-      conversation.update!(signon_user:, source: :api)
+      conversation.update!(signon_user:)
       result = helper.question_show_summary_list_rows(question, nil, 1, 1)
 
-      row = result.find { |r| r[:field] == "API user" }
+      row = result.find { |r| r[:field] == "Signon user" }
       expect(row[:value])
         .to include(conversation.signon_user.name)
         .and have_link(

spec/requests/conversations_spec.rb

@@ -1,5 +1,8 @@
 RSpec.describe "ConversationsController" do
   delegate :helpers, to: ConversationsController
+  let(:signon_user) { create(:signon_user) }
+
+  before { login_as(signon_user) }
 
   it_behaves_like "handles a request for a user who hasn't completed onboarding",
                   routes: { show_conversation_path: %i[get], update_conversation_path: %i[post] }
@@ -80,7 +83,7 @@
     end
 
     context "when the conversation is active" do
-      let(:conversation) { create(:conversation, :not_expired) }
+      let(:conversation) { create(:conversation, :not_expired, signon_user:) }
 
       before do
         cookies[:conversation_id] = conversation.id
@@ -103,7 +106,7 @@
       end
 
       context "and there is a question without an answer" do
-        let(:conversation) { create(:conversation) }
+        let(:conversation) { create(:conversation, signon_user:) }
 
         it "renders the question and pending answer url correctly" do
           question = create(:question, conversation:)
@@ -117,7 +120,7 @@
       end
 
       context "and there is a question with an answer that doesn't have feedback" do
-        let(:conversation) { create(:conversation) }
+        let(:conversation) { create(:conversation, signon_user:) }
 
         it "renders the answer and an answer feedback form" do
           question = create(:question, :with_answer, conversation:)
@@ -135,7 +138,7 @@
       end
 
       context "and there is a question with an answer that has feedback" do
-        let(:conversation) { create(:conversation) }
+        let(:conversation) { create(:conversation, signon_user:) }
 
         it "doesn't render a feedback form" do
           question = create(:question, :with_answer, conversation:)
@@ -150,7 +153,7 @@
       end
 
       context "and there is a question with an answer that has sources" do
-        let(:conversation) { create(:conversation) }
+        let(:conversation) { create(:conversation, signon_user:) }
 
         it "renders the sources correctly for answers with the success status" do
           question = create(:question, conversation:)
@@ -193,7 +196,7 @@
       end
 
       context "and there are more questions than the max number of questions" do
-        let(:conversation) { create(:conversation) }
+        let(:conversation) { create(:conversation, signon_user:) }
 
         it "only renders the max number of question from rails config" do
           allow(Rails.configuration.conversations).to receive(:max_question_count).and_return(1)
@@ -209,7 +212,7 @@
 
       context "and the response format is JSON" do
         before do
-          conversation = create(:conversation, :not_expired)
+          conversation = create(:conversation, :not_expired, signon_user:)
           cookies[:conversation_id] = conversation.id
         end
 
@@ -233,7 +236,6 @@
 
   describe "POST :update" do
     include_context "with onboarding completed"
-    let(:conversation) { create(:conversation, :not_expired) }
 
     it "sets the converation_id cookie with valid params" do
       freeze_time do
@@ -253,6 +255,13 @@
           .to have_selector("h1", text: "GOV.UK Chat is generating an answer")
       end
 
+      it "associates the signon user with the conversation" do
+        post update_conversation_path, params: { create_question: { user_question: "How much tax should I be paying?" } }
+
+        conversation = Conversation.includes(:signon_user).last
+        expect(conversation.signon_user).to eq(signon_user)
+      end
+
       context "and the params are invalid while the last question is answered" do
         it "renders the conversation with an error" do
           post update_conversation_path, params: { create_question: { user_question: "" } }
@@ -267,7 +276,7 @@
       end
 
       context "and the params are invalid while the last question is not answered" do
-        let(:conversation) { create(:conversation, questions: [create(:question)]) }
+        let(:conversation) { create(:conversation, signon_user:, questions: [create(:question)]) }
 
         before do
           cookies[:conversation_id] = conversation.id
@@ -284,6 +293,8 @@
       end
 
       context "and the converation_id cookie is present" do
+        let(:conversation) { create(:conversation, :not_expired, signon_user:) }
+
         before do
           cookies[:conversation_id] = conversation.id
         end
@@ -304,6 +315,8 @@
     end
 
     context "when the request format is JSON" do
+      let(:conversation) { create(:conversation, :not_expired, signon_user:) }
+
       before do
         cookies[:conversation_id] = conversation.id
       end
@@ -340,7 +353,7 @@
 
   describe "GET :answer" do
     include_context "with onboarding completed"
-    let(:conversation) { create(:conversation) }
+    let(:conversation) { create(:conversation, signon_user:) }
 
     before do
       cookies[:conversation_id] = conversation.id
@@ -444,7 +457,7 @@
   describe "POST :answer_feedback" do
     include_context "with onboarding completed"
 
-    let(:conversation) { create(:conversation) }
+    let(:conversation) { create(:conversation, signon_user:) }
     let(:question) { create(:question, conversation:) }
 
     before do
@@ -530,7 +543,7 @@
     include_context "with onboarding completed"
 
     context "when the conversation is active" do
-      let(:conversation) { create(:conversation, :not_expired) }
+      let(:conversation) { create(:conversation, :not_expired, signon_user:) }
 
       before do
         cookies[:conversation_id] = conversation.id
@@ -550,7 +563,7 @@
     include_context "with onboarding completed"
 
     context "when the conversation is active" do
-      let(:conversation) { create(:conversation, :not_expired) }
+      let(:conversation) { create(:conversation, :not_expired, signon_user:) }
 
       before do
         cookies[:conversation_id] = conversation.id

spec/support/system_spec_helpers.rb

@@ -1,4 +1,9 @@
 module SystemSpecHelpers
+  def given_i_am_signed_in
+    @signon_user = create(:signon_user)
+    login_as(@signon_user)
+  end
+
   def given_i_am_using_the_claude_structured_answer_strategy
     allow(Rails.configuration)
       .to receive(:answer_strategy)

spec/system/conversation_js_features_spec.rb

@@ -1,6 +1,7 @@
 RSpec.describe "Conversation JavaScript features", :chunked_content_index, :dismiss_cookie_banner, :js do
   scenario "questions with answers" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     when_i_enter_a_first_question
     then_i_see_the_first_question_was_accepted
 
@@ -15,7 +16,8 @@
   end
 
   scenario "client side validation" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     when_i_enter_an_empty_question
     then_i_see_a_presence_validation_message
 
@@ -24,7 +26,8 @@
   end
 
   scenario "server side validation" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     when_i_enter_a_question_with_pii
     then_i_see_a_pii_validation_message
 
@@ -33,7 +36,8 @@
   end
 
   scenario "reloading the page while an answer is pending" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     when_i_enter_a_first_question
     then_i_see_the_first_question_was_accepted
 
@@ -43,7 +47,8 @@
   end
 
   scenario "User gives feedback on an answer" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     when_i_enter_a_first_question
     then_i_see_the_first_question_was_accepted
 
@@ -53,7 +58,8 @@
   end
 
   scenario "character limits" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     when_i_type_in_a_question_approaching_the_character_count_limit
     then_i_see_a_character_count_warning
 
@@ -62,7 +68,8 @@
   end
 
   scenario "loading messages" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     when_i_enter_a_first_question_with_a_slow_response
     then_i_see_a_question_loading_message
 
@@ -75,7 +82,8 @@
   end
 
   scenario "showing clear chat link in navigation" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     then_i_cant_see_the_clear_chat_link
 
     when_i_enter_a_first_question
@@ -84,7 +92,8 @@
   end
 
   scenario "print link is added to navigation" do
-    given_i_have_confirmed_i_understand_chat_risks
+    given_i_am_signed_in
+    and_i_have_confirmed_i_understand_chat_risks
     then_i_see_a_print_link_in_the_menu
   end
 
@@ -97,7 +106,7 @@ def when_i_enter_a_first_question
 
   def when_i_enter_a_first_question_with_a_slow_response
     @first_question = "How do I setup a workplace pension?"
-    conversation = build(:conversation)
+    conversation = build(:conversation, signon_user: @signon_user)
     prepared_question = create(:question, message: @first_question, conversation:)
 
     allow(Question).to receive(:new).and_return(prepared_question)
@@ -287,10 +296,6 @@ def then_i_see_a_print_link_in_the_menu
     expect(page).to have_button("Print or save this chat")
   end
 
-  def and_i_have_an_active_conversation
-    create(:conversation)
-  end
-
   def when_i_visit_the_conversation_page
     visit show_conversation_path
   end

spec/system/user_clears_chat_spec.rb

@@ -1,6 +1,7 @@
 RSpec.describe "User clears chat" do
   scenario do
-    given_i_have_an_active_conversation_with_an_answered_question
+    given_i_am_signed_in
+    and_i_have_an_active_conversation_with_an_answered_question
     and_i_visit_the_conversation_page
     when_i_click_the_clear_chat_link
     and_i_cancel
@@ -10,8 +11,8 @@
     then_i_cannot_see_my_previous_questions_and_answer
   end
 
-  def given_i_have_an_active_conversation_with_an_answered_question
-    @conversation = create(:conversation)
+  def and_i_have_an_active_conversation_with_an_answered_question
+    @conversation = create(:conversation, signon_user: @signon_user)
     set_rack_cookie(:conversation_id, @conversation.id)
     answer = build(:answer, message: "Example answer")
     create(:question, answer:, conversation: @conversation, message: "Example question")

spec/system/user_gives_feedback_on_answer_spec.rb

@@ -1,13 +1,14 @@
 RSpec.describe "User gives feedback on an answer" do
   scenario do
-    given_i_have_an_active_conversation_with_an_answered_question
+    given_i_am_signed_in
+    and_i_have_an_active_conversation_with_an_answered_question
     when_i_visit_the_conversation_page
     and_i_click_that_the_answer_was_useful
     then_i_see_that_my_feedback_was_submitted
   end
 
-  def given_i_have_an_active_conversation_with_an_answered_question
-    @conversation = create(:conversation)
+  def and_i_have_an_active_conversation_with_an_answered_question
+    @conversation = create(:conversation, signon_user: @signon_user)
     set_rack_cookie(:conversation_id, @conversation.id)
     create(:question, :with_answer, conversation: @conversation)
   end