404 if project not found in Api::SchoolProjectsController

Previously when a request was made to either of the actions in
`Api::SchoolProjectsController` for a project that does not exist, the
following exception was raised:

    NoMethodError: undefined method `school_project' for nil:NilClass

Unhandled exceptions are rescued by the default
`ActionDispatch::ShowExceptions` middleware [1]. In turn this uses the
default exceptions app, `ActionDispatch::PublicExceptions` [2] to render
a static HTML error page from `public/500.html` in this case.

Since this is an API endpoint, ideally it should always respond with
JSON in the response body, otherwise it makes writing client code a lot
harder. Indeed my actual motivation for fixing this is, because we've
been seeing JSON:ParseError exceptions raised in experience-cs [3].

I've fixed the problem by using `Project.find_by!` instead of
`Project.find_by`. This means that if the project is not found, a
`ActiveRecord::RecordNotFound` exception is raised. This in turn is
rescued by the `rescue_from` block in `ApiController` [4] and the action
responds with a 404 Not Found head response, i.e. with no body.

This head response seems to be handled OK by the HTTParty code in
experience-cs [5] and thus it fixes the problem I was trying to address.

The equivalent client-side code in editor-standalone [6] was already
throwing an AxisError exception due to the 500 response code (not being
a 2XX code [7]) and it will continue to throw the same exception, albeit
with a payload reflecting the response code being 404 instead of 500 and
the body being empty instead of being the Rails 500 HTML error page.

There is no specific error-handling code in editor-standalone catching
this exception, so I assume it's just relying on the generic error
handling for the React app. Thus I think the user-facing behaviour
should be unchanged.

[1]: https://api.rubyonrails.org/v7.1.3.4/classes/ActionDispatch/ShowExceptions.html
[2]: https://api.rubyonrails.org/v7.1.3.4/classes/ActionDispatch/PublicExceptions.html
[3]: https://raspberrypifoundation.sentry.io/issues/6708738500/
[4]: https://github.com/RaspberryPiFoundation/editor-api/blob/c37ab30714edeb08beaa1929970772545f38e93d/app/controllers/api_controller.rb#L9
[5]: https://github.com/RaspberryPiFoundation/experience-cs/blob/7b559bef916e0a32b3174f2391dcc587a8c4fe3b/lib/editor_api/client.rb
[6]: https://github.com/RaspberryPiFoundation/editor-standalone/blob/f1286121460e79da7ffa7431487a2fb0872f2ae5/src/utils/apiCallHandler/projects.js#L71-L91
[7]: https://axios-http.com/docs/handling_errors

Author: floehopper

app/controllers/api/school_projects_controller.rb

@@ -6,13 +6,13 @@ class SchoolProjectsController < ApiController
     load_and_authorize_resource :project
 
     def show_finished
-      @school_project = Project.find_by(identifier: params[:id]).school_project
+      @school_project = Project.find_by!(identifier: params[:id]).school_project
       authorize! :show_finished, @school_project
       render :finished, formats: [:json], status: :ok
     end
 
     def set_finished
-      project = Project.find_by(identifier: params[:id])
+      project = Project.find_by!(identifier: params[:id])
       @school_project = project.school_project
       authorize! :set_finished, @school_project
       result = SchoolProject::SetFinished.call(school_project: @school_project, finished: params[:finished])

spec/requests/school_projects/set_finished_spec.rb

@@ -80,4 +80,14 @@
       expect(teacher_project.school_project.finished).to be_falsey
     end
   end
+
+  context 'when project does not exist' do
+    before do
+      put('/api/projects/does-not-exist/finished', headers:, params: { finished: false })
+    end
+
+    it 'returns not found response' do
+      expect(response).to have_http_status(:not_found)
+    end
+  end
 end

spec/requests/school_projects/show_finished_spec.rb

@@ -52,5 +52,15 @@
         expect(response).to have_http_status(:forbidden)
       end
     end
+
+    context 'when project does not exist' do
+      before do
+        get('/api/projects/does-not-exist/finished', headers:)
+      end
+
+      it 'returns not found response' do
+        expect(response).to have_http_status(:not_found)
+      end
+    end
   end
 end