Merge branch 'main' into 1130_store-and-return-scratch-data

mwtrew · web-flow · commit 23854d017b0b · 2026-03-12T13:33:41.000Z
diff --git a/.clabot b/.clabot
@@ -19,6 +19,7 @@
     "jamiebenstead",
     "jrmhaig",
     "KristinaDudnyk",
+    "kylec-rpf",
     "loiswells97",
     "lpmi-13",
     "m-bowley",
@@ -35,4 +36,4 @@
     "zetter-rpf"
   ],
   "message": "We require contributors to sign our Contributor License Agreement, and we don't have you on file. In order for us to review and merge your code, please complete [this form](https://form.raspberrypi.org/4873530) and we'll get you added and review your contribution as soon as possible."
-}
+}
diff --git a/.erdconfig b/.erdconfig
@@ -0,0 +1,2 @@
+filename: EditorAPI-ERD
+title: 'Editor API Entity-Relationship Diagram (ERD)'
diff --git a/Gemfile b/Gemfile
@@ -28,7 +28,7 @@ gem 'image_processing'
 gem 'importmap-rails'
 gem 'jbuilder'
 gem 'kaminari'
-gem 'omniauth-rails_csrf_protection', '~> 1.0.1'
+gem 'omniauth-rails_csrf_protection', '~> 2.0.1'
 gem 'omniauth-rpi',
     github: 'RaspberryPiFoundation/omniauth-rpi',
     tag: 'v1.3.1'
@@ -80,4 +80,4 @@ group :test do
   gem 'webmock'
 end
 
-gem 'flipper-ui', '~> 1.3'
+gem 'flipper-ui', '~> 1.4'
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -178,7 +178,7 @@ GEM
     factory_bot_rails (6.5.1)
       factory_bot (~> 6.5)
       railties (>= 6.1.0)
-    faker (3.5.2)
+    faker (3.6.0)
       i18n (>= 1.8.11, < 2)
     faraday (2.14.1)
       faraday-net_http (>= 2.0, < 3.5)
@@ -190,14 +190,14 @@ GEM
     ffi (1.17.2-arm64-darwin)
     ffi (1.17.2-x86_64-linux-gnu)
     fiber-storage (1.0.1)
-    flipper (1.3.6)
+    flipper (1.4.0)
       concurrent-ruby (< 2)
-    flipper-active_record (1.3.6)
+    flipper-active_record (1.4.0)
       activerecord (>= 4.2, < 9)
-      flipper (~> 1.3.6)
-    flipper-ui (1.3.6)
+      flipper (~> 1.4.0)
+    flipper-ui (1.4.0)
       erubi (>= 1.0.0, < 2.0.0)
-      flipper (~> 1.3.6)
+      flipper (~> 1.4.0)
       rack (>= 1.4, < 4)
       rack-protection (>= 1.5.3, < 5.0.0)
       rack-session (>= 1.0.2, < 3.0.0)
@@ -228,7 +228,8 @@ GEM
       activesupport (>= 3.0)
       graphql (>= 1.13.0)
     hashdiff (1.2.0)
-    hashie (5.0.0)
+    hashie (5.1.0)
+      logger
     i18n (1.14.8)
       concurrent-ruby (~> 1.0)
     image_processing (1.14.0)
@@ -315,14 +316,15 @@ GEM
       rack (>= 1.2, < 4)
       snaky_hash (~> 2.0, >= 2.0.3)
       version_gem (>= 1.1.8, < 3)
-    omniauth (2.1.3)
+    omniauth (2.1.4)
       hashie (>= 3.4.6)
+      logger
       rack (>= 2.2.3)
       rack-protection
     omniauth-oauth2 (1.8.0)
       oauth2 (>= 1.4, < 3)
       omniauth (~> 2.0)
-    omniauth-rails_csrf_protection (1.0.2)
+    omniauth-rails_csrf_protection (2.0.1)
       actionpack (>= 4.2)
       omniauth (~> 2.0)
     open-uri (0.5.0)
@@ -366,7 +368,7 @@ GEM
     rack-cors (3.0.0)
       logger
       rack (>= 3.0.14)
-    rack-protection (4.1.1)
+    rack-protection (4.2.1)
       base64 (>= 0.1.0)
       logger (>= 1.6.0)
       rack (>= 3.0.0, < 4)
@@ -468,7 +470,7 @@ GEM
     rubocop-capybara (2.22.1)
       lint_roller (~> 1.1)
       rubocop (~> 1.72, >= 1.72.1)
-    rubocop-factory_bot (2.27.1)
+    rubocop-factory_bot (2.28.0)
       lint_roller (~> 1.1)
       rubocop (~> 1.72, >= 1.72.1)
     rubocop-graphql (1.5.6)
@@ -609,7 +611,7 @@ DEPENDENCIES
   faraday
   flipper (~> 1.3)
   flipper-active_record (~> 1.3)
-  flipper-ui (~> 1.3)
+  flipper-ui (~> 1.4)
   github_webhook (~> 1.4)
   globalid
   good_job (~> 4.3)
@@ -621,7 +623,7 @@ DEPENDENCIES
   importmap-rails
   jbuilder
   kaminari
-  omniauth-rails_csrf_protection (~> 1.0.1)
+  omniauth-rails_csrf_protection (~> 2.0.1)
   omniauth-rpi!
   open-uri
   paper_trail
diff --git a/README.md b/README.md
@@ -151,6 +151,20 @@ Handled in `config/initializers/cors.rb`.
 
 This API receives push event data from the [Raspberry Pi Learning](https://github.com/raspberrypilearning) organisation via webhooks. This data is used to create or update code projects related to the [Code Club Projects Site](https://projects.raspberrypi.org), and is processed using the `github_webhooks` gem in the `github_webhooks_controller`. For development purposes, these webhooks are mediated locally through `smee`, which runs in a Docker container.
 
+### Database Structure
+
+Once you have completed the initial setup and have everything running in a container, you can generate an Entity-Relationship Diagram (ERD) to help you to understand the database structure. In order to do this, start things up as usual in one terminal:
+
+`docker-compose up`
+
+then in a second terminal, create the ERD using:
+
+`docker-compose exec api bundle exec erd`
+
+then copy the created ERD out of the container with:
+
+`docker cp "$(docker-compose ps -q api)":/app/EditorAPI-ERD.pdf .`
+
 ## Usage
 
 ### Projects
diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb
@@ -9,8 +9,8 @@ class RemixesController < ApiController
 
       def index
         projects = Project.where(remixed_from_id: project.id).accessible_by(current_ability)
-        @projects_with_users = projects.includes(:school_project).with_users(current_user)
-        render index: @projects_with_users, formats: [:json]
+        @projects_with_students = projects.includes(:school_project).with_students(project.school, current_user)
+        render index: @projects_with_students, formats: [:json]
       end
 
       def show
diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb
@@ -21,7 +21,7 @@ def index
 
     def show
       if !@project.school_id.nil? && @project.lesson_id.nil?
-        project_with_user = @project.with_user(current_user)
+        project_with_user = @project.with_student(current_user)
         @user = project_with_user[1]
       end
 
diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
@@ -23,8 +23,12 @@ def authorize_user
     render json: { error: 'Unauthorized' }, status: :unauthorized unless current_user
   end
 
-  def denied(exception)
-    render_error_as_json(exception, :forbidden)
+  def denied(_exception)
+    if current_user
+      render json: { error: 'Forbidden' }, status: :forbidden
+    else
+      render json: { error: 'Unauthorized' }, status: :unauthorized
+    end
   end
 
   def not_found(exception)
diff --git a/app/models/project.rb b/app/models/project.rb
@@ -45,21 +45,19 @@ module Types
     }
   )
 
-  def self.users(current_user)
-    school = School.find_by(id: pluck(:school_id))
+  def self.students(school, current_user)
     SchoolStudent::List.call(school:, token: current_user.token, student_ids: pluck(:user_id).uniq)[:school_students] || []
   end
 
-  def self.with_users(current_user)
-    by_id = users(current_user).index_by(&:id)
+  def self.with_students(school, current_user)
+    by_id = students(school, current_user).index_by(&:id)
     all.map { |instance| [instance, by_id[instance.user_id]] }
   end
 
-  def with_user(current_user)
+  def with_student(current_user)
     # students cannot call the Profile API so do not even try
     return [self, nil] if current_user&.student?
 
-    school = School.find_by(id: school_id)
     students = SchoolStudent::List.call(school:, token: current_user.token,
                                         student_ids: [user_id])[:school_students] || []
     [self, students.first]
diff --git a/app/views/api/projects/remixes/index.json.jbuilder b/app/views/api/projects/remixes/index.json.jbuilder
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-json.array!(@projects_with_users) do |project, user|
+json.array!(@projects_with_students) do |project, student|
   json.call(
     project,
     :identifier,
@@ -11,7 +11,7 @@ json.array!(@projects_with_users) do |project, user|
     :last_edited_at
   )
 
-  json.user_name(user&.name)
+  json.user_name(student&.name)
   json.finished(project.school_project&.finished)
   json.status(project.school_project&.status)
 end
diff --git a/config/application.rb b/config/application.rb
@@ -60,5 +60,9 @@ class Application < Rails::Application
 
     config.middleware.insert_before 0, CorpMiddleware
     config.generators.system_tests = nil
+
+    config.active_record.encryption.primary_key = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY')
+    config.active_record.encryption.deterministic_key = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY')
+    config.active_record.encryption.key_derivation_salt = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT')
   end
 end
diff --git a/config/initializers/encryption.rb b/config/initializers/encryption.rb
diff --git a/erd.pdf b/erd.pdf
diff --git a/lib/concepts/school_student/list.rb b/lib/concepts/school_student/list.rb
@@ -7,7 +7,7 @@ def call(school:, token:, student_ids: nil)
         response = OperationResponse.new
         response[:school_students] = list_students(school, token, student_ids)
         response
-      rescue StandardError => e
+      rescue Faraday::Error, ProfileApiClient::Error => e
         Sentry.capture_exception(e)
         response[:error] = "Error listing school students: #{e}"
         response
@@ -17,6 +17,8 @@ def call(school:, token:, student_ids: nil)
 
       def list_students(school, token, student_ids)
         student_ids ||= Role.student.where(school:).map(&:user_id)
+        return [] if student_ids.empty?
+
         students = ProfileApiClient.list_school_students(token:, school_id: school.id, student_ids:)
         students.map do |student|
           User.new(student.to_h.slice(:id, :username, :name, :email).merge(sso_providers: student.ssoProviders))
diff --git a/spec/concepts/school_student/list_spec.rb b/spec/concepts/school_student/list_spec.rb
@@ -84,13 +84,19 @@
         expect(response[:school_students]).to include(expected_user)
       end
     end
+
+    it 'skips the API call if student_ids is empty' do
+      response = described_class.call(school:, token:, student_ids: [])
+      expect(ProfileApiClient).not_to have_received(:list_school_students)
+      expect(response[:school_students].size).to eq(0)
+    end
   end
 
-  context 'when listing fails' do
+  context 'when listing fails due to a Faraday error' do
     let(:student_ids) { [123] }
 
     before do
-      allow(ProfileApiClient).to receive(:list_school_students).and_raise('Some API error')
+      allow(ProfileApiClient).to receive(:list_school_students).and_raise(Faraday::Error.new('Some API error'))
       allow(Sentry).to receive(:capture_exception)
     end
 
@@ -106,7 +112,7 @@
 
     it 'sent the exception to Sentry' do
       described_class.call(school:, token:, student_ids:)
-      expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
+      expect(Sentry).to have_received(:capture_exception).with(kind_of(Faraday::Error))
     end
   end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
diff --git a/spec/requests/api_controller_spec.rb b/spec/requests/api_controller_spec.rb
diff --git a/spec/requests/projects/remix_spec.rb b/spec/requests/projects/remix_spec.rb
diff --git a/spec/requests/projects/show_spec.rb b/spec/requests/projects/show_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+filename: EditorAPI-ERD`
	`2`	`+title: 'Editor API Entity-Relationship Diagram (ERD)'`