Merge pull request #195 from alphagov/2424-add-put-conversation-endpoint

Add PUT conversation endpoint

Author: davidgisbey

app/controllers/api/v0/conversations_controller.rb

@@ -1,17 +1,16 @@
 class Api::V0::ConversationsController < ApplicationController
   before_action { authorise_user!(SignonUser::Permissions::CONVERSATION_API) }
-  before_action :find_conversation, only: %i[show answer answer_feedback]
+  before_action :find_conversation, only: %i[show update answer answer_feedback]
 
   def create
     conversation = Conversation.new
-
-    form = Form::CreateQuestion.new(conversation:, user_question: question_params[:user_question])
+    form = Form::CreateQuestion.new(question_params.merge(conversation:))
 
     if form.valid?
       question = form.submit
       render json: QuestionBlueprint.render(question, view: :pending), status: :created
     else
-      render json: ValidationErrorBlueprint.render(message: "Could not create question", errors: form.errors.messages), status: :unprocessable_entity
+      render json: ValidationErrorBlueprint.render(errors: form.errors.messages), status: :unprocessable_entity
     end
   end
 
@@ -25,6 +24,20 @@ def show
     )
   end
 
+  def update
+    form = Form::CreateQuestion.new(question_params.merge(conversation: @conversation))
+
+    if form.valid?
+      question = form.submit
+
+      render json: QuestionBlueprint.render(question, view: :pending), status: :created
+    else
+      render json: ValidationErrorBlueprint.render(
+        errors: form.errors.messages,
+      ), status: :unprocessable_entity
+    end
+  end
+
   def answer
     question = @conversation.questions.find(params[:question_id])
     answer = question.answer

app/models/form/create_question.rb

@@ -9,6 +9,9 @@ class Form::CreateQuestion
   USER_QUESTION_PRESENCE_ERROR_MESSAGE = "Ask a question. For example, 'how do I register for VAT?'".freeze
   USER_QUESTION_LENGTH_MAXIMUM = 300
   USER_QUESTION_LENGTH_ERROR_MESSAGE = "Question must be %{count} characters or less".freeze
+  USER_QUESTION_PII_ERROR_MESSAGE = "Personal data has been detected in your question. Please remove it and try asking again.".freeze
+  QUESTION_LIMIT_REACHED_MESSAGE = "You’ve reached the message limit for the GOV.UK Chat trial. You have no messages left.".freeze
+  UNANSWERED_QUESTION_ERROR_MESSAGE = "Previous question pending. Please wait for a response".freeze
 
   before_validation :sanitise_user_question
 
@@ -51,22 +54,20 @@ def sanitise_user_question
 
   def all_questions_answered?
     if conversation.questions.unanswered.exists?
-      errors.add(:base, "Previous question pending. Please wait for a response")
+      errors.add(:base, UNANSWERED_QUESTION_ERROR_MESSAGE)
     end
   end
 
   def no_pii_present?
     if PiiValidator.invalid?(user_question)
-      error_message = "Personal data has been detected in your question. Please remove it and try asking again."
-
-      errors.add(:user_question, error_message)
+      errors.add(:user_question, USER_QUESTION_PII_ERROR_MESSAGE)
     end
   end
 
   def within_question_limit?
     return if conversation.user.nil?
     return unless conversation.user.question_limit_reached?
 
-    errors.add(:base, "You’ve reached the message limit for the GOV.UK Chat trial. You have no messages left.")
+    errors.add(:base, QUESTION_LIMIT_REACHED_MESSAGE)
   end
 end

config/routes.rb

@@ -147,6 +147,7 @@
     namespace :v0 do
       post "/conversation", to: "conversations#create", as: :create_conversation
       get "/conversation/:conversation_id", to: "conversations#show", as: :show_conversation
+      put "/conversation/:conversation_id", to: "conversations#update", as: :update_conversation
       get "/conversation/:conversation_id/questions/:question_id/answer", to: "conversations#answer", as: :answer_question
       post "/conversation/:conversation_id/answers/:answer_id/feedback", to: "conversations#answer_feedback", as: :answer_feedback
     end

spec/models/form/create_question_spec.rb

@@ -20,7 +20,15 @@
       )
       form.validate
 
-      expect(form.errors.messages[:user_question]).to eq(["Question must be 300 characters or less"])
+      expect(form.errors.messages[:user_question])
+        .to eq(
+          [
+            sprintf(
+              described_class::USER_QUESTION_LENGTH_ERROR_MESSAGE,
+              count: described_class::USER_QUESTION_LENGTH_MAXIMUM,
+            ),
+          ],
+        )
     end
 
     it "is invalid when user_question is blank" do
@@ -30,7 +38,7 @@
       )
       form.validate
 
-      expect(form.errors.messages[:user_question]).to eq(["Ask a question. For example, 'how do I register for VAT?'"])
+      expect(form.errors.messages[:user_question]).to eq([described_class::USER_QUESTION_PRESENCE_ERROR_MESSAGE])
     end
 
     it "is invalid when the conversation passed in has an unanswered question" do
@@ -39,22 +47,18 @@
       form = described_class.new(conversation:, user_question:)
       form.validate
 
-      expect(form.errors.messages[:base]).to eq(["Previous question pending. Please wait for a response"])
+      expect(form.errors.messages[:base]).to eq([described_class::UNANSWERED_QUESTION_ERROR_MESSAGE])
     end
 
     describe "personally identifiable information (pii) validation" do
-      let(:pii_error_message) do
-        "Personal data has been detected in your question. Please remove it and try asking again."
-      end
-
       it "adds an error message when pii is present" do
         form = described_class.new(
           conversation:,
           user_question: "My email address is email@gmail.com",
         )
         form.validate
 
-        expect(form.errors.messages[:user_question]).to eq([pii_error_message])
+        expect(form.errors.messages[:user_question]).to eq([described_class::USER_QUESTION_PII_ERROR_MESSAGE])
       end
 
       it "doesn't add an error message when no pii is present" do
@@ -68,13 +72,11 @@
     end
 
     describe "within question limit validation" do
-      let(:question_limit_error_message) { "You’ve reached the message limit for the GOV.UK Chat trial. You have no messages left." }
-
       it "adds a error message if over the question limit" do
         conversation.user = build(:early_access_user, questions_count: 2, individual_question_limit: 1)
         form = described_class.new(conversation:, user_question:)
         form.validate
-        expect(form.errors.messages[:base]).to eq([question_limit_error_message])
+        expect(form.errors.messages[:base]).to eq([described_class::QUESTION_LIMIT_REACHED_MESSAGE])
       end
 
       it "is valid when under the question limit" do

spec/requests/api/v0/conversations_spec.rb

@@ -6,45 +6,55 @@
     login_as(create(:signon_user, :conversation_api))
   end
 
-  shared_examples "responds with forbidden if user doesn't have conversation-api permission" do |path, method|
-    let(:route_params) { {} }
-    let(:params) { {} }
-
-    describe "responds with forbidden if user doesn't have conversation-api permission" do
-      it "returns with 403 for #{path}" do
-        login_as(create(:signon_user))
-        process(method.to_sym, public_send(path.to_sym, *route_params), params:, as: :json)
+  shared_examples "responds with forbidden if user doesn't have conversation-api permission" do |routes:|
+    before { login_as(create(:signon_user)) }
+
+    routes.each do |route|
+      describe "responds with forbidden if user doesn't have conversation-api permission" do
+        it "returns with 403 for #{route[:method]} #{route[:path]}" do
+          process(
+            route[:method.to_sym],
+            public_send(route[:path].to_sym, *(route[:route_params] || [])),
+            params: route[:params] || {},
+            as: :json,
+          )
 
-        expect(response).to have_http_status(:forbidden)
+          expect(response).to have_http_status(:forbidden)
+        end
       end
     end
   end
 
   it_behaves_like "responds with forbidden if user doesn't have conversation-api permission",
-                  :api_v0_create_conversation_path,
-                  :post do
-    let(:route_params) { [] }
-    let(:params) { { user_question: "" } }
-  end
-
-  it_behaves_like "responds with forbidden if user doesn't have conversation-api permission",
-                  :api_v0_show_conversation_path,
-                  :get do
-    let(:route_params) { [SecureRandom.uuid] }
-  end
-
-  it_behaves_like "responds with forbidden if user doesn't have conversation-api permission",
-                  :api_v0_answer_question_path,
-                  :get do
-    let(:route_params) { [SecureRandom.uuid, SecureRandom.uuid] }
-  end
-
-  it_behaves_like "responds with forbidden if user doesn't have conversation-api permission",
-                  :api_v0_answer_feedback_path,
-                  :post do
-    let(:route_params) { [SecureRandom.uuid, SecureRandom.uuid] }
-    let(:params) { { useful: true } }
-  end
+                  routes: [
+                    {
+                      path: :api_v0_create_conversation_path,
+                      method: :post,
+                      params: { user_question: "question" },
+                    },
+                    {
+                      path: :api_v0_show_conversation_path,
+                      method: :get,
+                      route_params: [SecureRandom.uuid],
+                    },
+                    {
+                      path: :api_v0_update_conversation_path,
+                      method: :put,
+                      route_params: [SecureRandom.uuid],
+                      params: { user_question: "question" },
+                    },
+                    {
+                      path: :api_v0_answer_question_path,
+                      method: :get,
+                      route_params: [SecureRandom.uuid, SecureRandom.uuid],
+                    },
+                    {
+                      path: :api_v0_answer_feedback_path,
+                      method: :post,
+                      route_params: [SecureRandom.uuid, SecureRandom.uuid],
+                      params: { useful: true },
+                    },
+                  ]
 
   describe "middleware ensures adherance to the OpenAPI specification" do
     context "when the response returned does not conform to the OpenAPI specification" do
@@ -141,6 +151,57 @@
     end
   end
 
+  describe "PUT :update" do
+    context "when the params are valid" do
+      let(:user_question) { "What is the capital of France?" }
+      let(:params) { { user_question: } }
+
+      it "returns a created status" do
+        put api_v0_update_conversation_path(conversation), params:, as: :json
+        expect(response).to have_http_status(:created)
+      end
+
+      it "creates a question on the conversation" do
+        expect {
+          put api_v0_update_conversation_path(conversation), params:, as: :json
+        }.to change(conversation.questions, :count).by(1)
+        expect(conversation.questions.strict_loading(false).last.message)
+          .to eq(user_question)
+      end
+
+      it "returns the expected JSON" do
+        put api_v0_update_conversation_path(conversation), params:, as: :json
+
+        expected_response = QuestionBlueprint.render_as_json(
+          conversation.questions.strict_loading(false).last,
+          view: :pending,
+        )
+        expect(JSON.parse(response.body)).to eq(expected_response)
+      end
+    end
+
+    context "when the params are invalid" do
+      let(:params) { { user_question: "" } }
+
+      it "returns an unprocessable_entity status" do
+        put api_v0_update_conversation_path(conversation), params:, as: :json
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+
+      it "returns the correct expected JSON" do
+        put api_v0_update_conversation_path(conversation), params:, as: :json
+
+        expect(JSON.parse(response.body))
+          .to eq(
+            {
+              "message" => "Unprocessable entity",
+              "errors" => { "user_question" => [Form::CreateQuestion::USER_QUESTION_PRESENCE_ERROR_MESSAGE] },
+            },
+          )
+      end
+    end
+  end
+
   describe "GET :answer" do
     context "when an answer has been generated for the question" do
       let!(:answer) { create(:answer, question:) }