Merge pull request #336 from alphagov/ldeb-respond-with-resource

Change API endpoints to respond with resource

app/controllers/api/v1/conditions_controller.rb

@@ -7,7 +7,7 @@ def create
     new_condition = page.routing_conditions.new(condition_params)
 
     if new_condition.save_and_update_form
-      render json: { id: new_condition.id }, status: :created
+      render json: new_condition.to_json, status: :created
     else
       render json: new_condition.errors.to_json, status: :bad_request
     end
@@ -21,15 +21,15 @@ def update
     condition.assign_attributes(condition_params)
 
     if condition.save_and_update_form
-      render json: { success: true }.to_json, status: :ok
+      render json: condition.to_json, status: :ok
     else
       render json: page.errors.to_json, status: :bad_request
     end
   end
 
   def destroy
     condition.destroy_and_update_form!
-    render json: { success: true }.to_json, status: :ok
+    render status: :no_content
   end
 
 private

app/controllers/api/v1/forms_controller.rb

@@ -13,15 +13,15 @@ def index
   def create
     new_form = Form.new(form_params)
     if new_form.save
-      render json: { id: new_form.id }, status: :created # Fixup - just returning id here, could we return whole object?
+      render json: new_form.to_json, status: :created
     else
       render json: new_form.errors.to_json, status: :bad_request
     end
   end
 
   def update
     if form.update(form_params)
-      render json: { success: true }.to_json, status: :ok
+      render json: form.to_json, status: :ok
     else
       render json: form.errors.to_json, status: :bad_request
     end
@@ -33,13 +33,13 @@ def show
 
   def destroy
     form.destroy!
-    render json: { success: true }.to_json, status: :ok
+    render status: :no_content
   end
 
   def make_live
     if form.ready_for_live
       form.make_live!
-      render json: { success: true }.to_json, status: :ok
+      render json: form.live_version, status: :ok
     else
       render json: form.incomplete_tasks.to_json, status: :forbidden
     end
@@ -56,7 +56,7 @@ def show_draft
   def update_organisation_for_creator
     params.require(%i[creator_id organisation_id])
     Form.where(creator_id: params[:creator_id]).update_all(organisation_id: params[:organisation_id], updated_at: Time.zone.now)
-    render json: { success: true }.to_json, status: :ok
+    render status: :no_content
   end
 
 private

app/controllers/api/v1/pages_controller.rb

@@ -7,7 +7,7 @@ def create
     new_page = form.pages.new(page_params)
 
     if new_page.save_and_update_form
-      render json: { id: new_page.id }, status: :created
+      render json: new_page.to_json, status: :created
     end
   end
 
@@ -19,29 +19,29 @@ def update
     page.assign_attributes(page_params)
 
     if page.save_and_update_form
-      render json: { success: true }.to_json, status: :ok
+      render json: page.to_json, status: :ok
     end
   end
 
   def destroy
     page.destroy_and_update_form!
-    render json: { success: true }.to_json, status: :ok
+    render status: :no_content
   end
 
   def move_down
     unless page.last?
       page.move_lower
       form.update!(question_section_completed: false)
     end
-    render json: { success: 1 }.to_json, status: :ok
+    render json: page.to_json, status: :ok
   end
 
   def move_up
     unless page.first?
       page.move_higher
       form.update!(question_section_completed: false)
     end
-    render json: { success: 1 }.to_json, status: :ok
+    render json: page.to_json, status: :ok
   end
 
 private

spec/request/api/v1/conditions_controller_spec.rb

@@ -50,9 +50,9 @@
     end
 
     it "returns condition id, status code 201 when new condition created" do
-      expect(response.status).to eq(201)
+      expect(response).to have_http_status(:created)
       expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq({ id: new_condition.id })
+      expect(json_body).to include(id: new_condition.id, **new_condition_params)
     end
 
     context "with a form with question_section_completed = true" do
@@ -114,9 +114,9 @@
     end
 
     it "returns correct response" do
-      expect(response.status).to eq(200)
+      expect(response).to have_http_status(:ok)
       expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq({ success: true })
+      expect(json_body).to include(**params)
       expect(condition.reload.answer_value).to eq(answer_value)
       expect(form.reload.question_section_completed).to be false
     end
@@ -128,9 +128,7 @@
     end
 
     it "removes the condition and returns the correct response" do
-      expect(response.status).to eq(200)
-      expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq({ success: true })
+      expect(response).to have_http_status(:no_content)
       expect(routing_page.routing_conditions.count).to eq(0)
       expect(form.reload.question_section_completed).to be false
     end

spec/request/api/v1/forms_controller_spec.rb

@@ -97,9 +97,9 @@
 
     context "with valid params" do
       it "returns a status code 201 when new form created" do
-        expect(response.status).to eq(201)
+        expect(response).to have_http_status(:created)
         expect(response.headers["Content-Type"]).to eq("application/json")
-        expect(json_body).to eq(id: created_form[:id])
+        expect(json_body).to include(id: created_form[:id], **new_form_params)
       end
 
       it "created the form in the DB" do
@@ -123,9 +123,9 @@
       let(:new_form_params) { { organisation_id: 1, name: "test form one", submission_email: "test@example.gov.uk", support_url: "http://example.org" } }
 
       it "returns a status code 201" do
-        expect(response.status).to eq(201)
+        expect(response).to have_http_status(:created)
         expect(response.headers["Content-Type"]).to eq("application/json")
-        expect(json_body).to eq(id: created_form[:id])
+        expect(json_body).to include(id: created_form[:id], **new_form_params)
       end
 
       it "created the form in the DB" do
@@ -211,9 +211,9 @@
     it "when given an valid id and params, updates DB and returns 200" do
       form1 = create :form
       put form_path(form1), params: { submission_email: "test@example.gov.uk" }, as: :json
-      expect(response.status).to eq(200)
+      expect(response).to have_http_status(:ok)
       expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq(success: true)
+      expect(json_body).to include(submission_email: "test@example.gov.uk")
       expect(form1.reload.submission_email).to eq("test@example.gov.uk")
     end
 
@@ -239,21 +239,25 @@
     it "when given an existing id, returns 200 and deletes the form from DB" do
       form_to_be_deleted = create :form
       delete form_path(form_to_be_deleted), as: :json
-      expect(response.status).to eq(200)
-      expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq({ success: true })
+      expect(response).to have_http_status(:no_content)
     end
 
     it "when given an existing id, returns 200 and deletes the form and any existing pages from DB" do
       form_to_be_deleted = create :form, :with_pages
       delete form_path(form_to_be_deleted), as: :json
-      expect(response.status).to eq(200)
-      expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq({ success: true })
+      expect(response).to have_http_status(:no_content)
     end
   end
 
   describe "#make_live" do
+    before do
+      freeze_time
+    end
+
+    after do
+      unfreeze_time
+    end
+
     context "when given a form with missing sections" do
       it "doesn't make the form live" do
         form_to_be_made_live = create(:form, :new_form)
@@ -269,7 +273,7 @@
       post make_live_form_path(form_to_be_made_live), as: :json
       expect(response.status).to eq(200)
       expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq({ success: true })
+      expect(json_body).to include(live_at: Time.zone.now)
     end
   end
 
@@ -332,7 +336,7 @@
 
     context "when some forms match creator ID" do
       it "updates organisation only if creator ID matches" do
-        expect(response).to have_http_status(:ok)
+        expect(response).to have_http_status(:no_content)
 
         all_forms.each do |form|
           form.reload
@@ -349,7 +353,7 @@
       let(:selected_creator_id) { 321 }
 
       it "does not update organisation" do
-        expect(response).to have_http_status(:ok)
+        expect(response).to have_http_status(:no_content)
 
         all_forms.each do |form|
           form.reload

spec/request/api/v1/pages_controller_spec.rb

@@ -58,9 +58,9 @@
     end
 
     it "returns page id, status code 201 when new page created" do
-      expect(response.status).to eq(201)
+      expect(response).to have_http_status(:created)
       expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq({ id: new_page.id })
+      expect(json_body).to include(id: new_page.id, **new_page_params.deep_symbolize_keys)
     end
 
     it "creates DB row with new_page_params, fresh id, form_id set and next_page: nil" do
@@ -157,9 +157,9 @@
     end
 
     it "returns correct response" do
-      expect(response.status).to eq(200)
+      expect(response).to have_http_status(:ok)
       expect(response.headers["Content-Type"]).to eq("application/json")
-      expect(json_body).to eq({ success: true })
+      expect(json_body).to include(**params)
       expect(page1.reload.question_text).to eq("updated page title")
       expect(form.reload.question_section_completed).to be false
     end
@@ -185,9 +185,9 @@
         let(:answer_settings) { settings }
 
         it "returns correct response" do
-          expect(response.status).to eq(200)
+          expect(response).to have_http_status(:ok)
           expect(response.headers["Content-Type"]).to eq("application/json")
-          expect(json_body).to eq({ success: true })
+          expect(json_body).to include(**params.deep_symbolize_keys)
           page1.reload
           expect(page1.answer_settings&.symbolize_keys).to eq(settings)
         end
@@ -206,11 +206,11 @@
       end
 
       it "returns correct response" do
-        expect(response.status).to eq(200)
+        expect(response).to have_http_status(:ok)
         expect(response.headers["Content-Type"]).to eq("application/json")
-        expect(json_body).to eq({ success: true })
+        expect(json_body).to include(**params.deep_symbolize_keys)
         page1.reload
-        expect(page1.answer_settings&.deep_symbolize_keys).to eq(answer_settings)
+        expect(page1.answer_settings.deep_symbolize_keys).to eq(answer_settings)
       end
     end
   end
@@ -229,9 +229,7 @@
 
     context "with existing page" do
       it "removes page and returns correct response" do
-        expect(response.status).to eq(200)
-        expect(response.headers["Content-Type"]).to eq("application/json")
-        expect(json_body).to eq({ success: true })
+        expect(response).to have_http_status(:no_content)
         expect(form.pages.count).to eq(1)
         expect(form.reload.question_section_completed).to be false
       end
@@ -276,9 +274,9 @@
 
     context "with valid page" do
       it "returns correct response" do
-        expect(response.status).to eq(200)
+        expect(response).to have_http_status(:ok)
         expect(response.headers["Content-Type"]).to eq("application/json")
-        expect(json_body).to eq({ success: 1 })
+        expect(json_body).to include(position: page_to_move.position + 1)
       end
 
       it "changes order of pages returned" do
@@ -295,9 +293,9 @@
       let(:page_to_move) { last_page }
 
       it "returns correct response" do
-        expect(response.status).to eq(200)
+        expect(response).to have_http_status(:ok)
         expect(response.headers["Content-Type"]).to eq("application/json")
-        expect(json_body).to eq({ success: 1 })
+        expect(json_body).to include(position: page_to_move.position)
       end
 
       it "does not change order of pages" do
@@ -327,9 +325,9 @@
 
     context "with valid page" do
       it "returns correct response" do
-        expect(response.status).to eq(200)
+        expect(response).to have_http_status(:ok)
         expect(response.headers["Content-Type"]).to eq("application/json")
-        expect(json_body).to eq({ success: 1 })
+        expect(json_body).to include(position: page_to_move.position - 1)
       end
 
       it "changes order of pages returned" do
@@ -346,9 +344,9 @@
       let(:page_to_move) { first_page }
 
       it "returns correct response" do
-        expect(response.status).to eq(200)
+        expect(response).to have_http_status(:ok)
         expect(response.headers["Content-Type"]).to eq("application/json")
-        expect(json_body).to eq({ success: 1 })
+        expect(json_body).to include(position: page_to_move.position)
       end
 
       it "does not change order of pages" do