Merge branch 'main' into 1532_robust-auth-header-parsing

Author: mwtrew

Gemfile

@@ -83,7 +83,7 @@ group :test do
   gem 'climate_control'
   gem 'database_cleaner-active_record'
   gem 'selenium-webdriver'
-  gem 'shoulda-matchers', '~> 7.0'
+  gem 'shoulda-matchers', '~> 8.0'
   gem 'webdrivers'
   gem 'webmock'
 end

Gemfile.lock

@@ -98,8 +98,8 @@ GEM
     ast (2.4.3)
     awesome_print (1.9.2)
     aws-eventstream (1.4.0)
-    aws-partitions (1.1258.0)
-    aws-sdk-core (3.251.0)
+    aws-partitions (1.1261.0)
+    aws-sdk-core (3.252.0)
       aws-eventstream (~> 1, >= 1.3.0)
       aws-partitions (~> 1, >= 1.992.0)
       aws-sigv4 (~> 1.9)
@@ -110,7 +110,7 @@ GEM
     aws-sdk-kms (1.129.0)
       aws-sdk-core (~> 3, >= 3.248.0)
       aws-sigv4 (~> 1.5)
-    aws-sdk-s3 (1.225.0)
+    aws-sdk-s3 (1.225.1)
       aws-sdk-core (~> 3, >= 3.248.0)
       aws-sdk-kms (~> 1)
       aws-sigv4 (~> 1.5)
@@ -337,7 +337,7 @@ GEM
     postmark-rails (0.22.1)
       actionmailer (>= 3.0.0)
       postmark (>= 1.21.3, < 2.0)
-    pp (0.6.3)
+    pp (0.6.4)
       prettyprint
     prettyprint (0.2.0)
     prism (1.9.0)
@@ -455,7 +455,7 @@ GEM
     rspec-support (3.13.7)
     rspec_junit_formatter (0.6.0)
       rspec-core (>= 2, < 4, != 2.12.0)
-    rubocop (1.87.0)
+    rubocop (1.88.0)
       json (~> 2.3)
       language_server-protocol (~> 3.17.0.2)
       lint_roller (~> 1.1.0)
@@ -519,15 +519,15 @@ GEM
       rubyzip (>= 1.2.2)
     semantic_logger (4.18.0)
       concurrent-ruby (~> 1.0)
-    sentry-rails (6.6.0)
+    sentry-rails (6.6.2)
       railties (>= 5.2.0)
-      sentry-ruby (~> 6.6.0)
-    sentry-ruby (6.6.0)
+      sentry-ruby (~> 6.6.2)
+    sentry-ruby (6.6.2)
       bigdecimal
       concurrent-ruby (~> 1.0, >= 1.0.2)
       logger
-    shoulda-matchers (7.0.1)
-      activesupport (>= 7.1)
+    shoulda-matchers (8.0.1)
+      activesupport (>= 7.2)
     simplecov (0.22.0)
       docile (~> 1.1)
       simplecov-html (~> 0.11)
@@ -643,7 +643,7 @@ DEPENDENCIES
   rubyzip
   selenium-webdriver
   sentry-rails
-  shoulda-matchers (~> 7.0)
+  shoulda-matchers (~> 8.0)
   simplecov
   statesman
   webdrivers

app/jobs/upload_job.rb

@@ -21,8 +21,6 @@ class DataNotFoundError < StandardError; end
 
   VALID_LOCALES = Locales.load_locales.freeze
 
-  @skip_job = false
-
   ProjectContentQuery = GithubApi::Client.parse <<-GRAPHQL
     query($owner: String!, $repository: String!, $expression: String!) {
       repository(owner: $owner, name: $repository) {
@@ -63,7 +61,7 @@ def perform(payload)
 
       projects_data.data.repository.object.entries.each do |project_dir|
         project = format_project(project_dir, locale, repository(payload), owner(payload))
-        if @skip_job
+        if project[:build] == false
           Rails.logger.warn "Build skipped for #{project[:name]}"
           next
         end
@@ -107,10 +105,8 @@ def format_project(project_dir, locale, repository, owner)
     proj_config_file = data.entries.find { |file| file.name == PROJECT_CONFIG }
     proj_config = YAML.safe_load(proj_config_file.object.text, symbolize_names: true)
 
-    if proj_config[:build] == false
-      @skip_job = true
-      return proj_config
-    end
+    # if the build is false, no need to check files, just return the config
+    return proj_config if proj_config[:build] == false
 
     files = data.entries.reject { |file| file.name == PROJECT_CONFIG }
     categorized_files = categorize_files(files, project_dir, locale, repository, owner)

app/models/ability.rb

@@ -91,7 +91,7 @@ def define_school_teacher_abilities(user:, school:)
     can(%i[create], Project) do |project|
       school_teacher_can_manage_project?(user:, school:, project:)
     end
-    can(%i[read update show_context], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
+    can(%i[read update show_context], Project, school_id: school.id, lesson: { visibility: %w[teachers students], school_class: { teachers: { teacher_id: user.id } } })
     teacher_project_ids = Project.where(
       school_id: school.id,
       remixed_from_id: nil,

spec/jobs/upload_job_spec.rb

@@ -255,6 +255,60 @@
     end
   end
 
+  context 'with multiple projects where an earlier one has build set to false' do
+    let(:raw_response) { modifiable_response.deep_dup }
+
+    before do
+      entries = raw_response['data']['repository']['object']['entries']
+
+      # Turn the existing project into a build: false project
+      build_false_dir = entries.find { |entry| entry['name'] == 'dont-collide-starter' }
+      build_false_config = build_false_dir['object']['entries'].find { |entry| entry['name'] == 'project_config.yml' }
+      build_false_config['object']['text'] += "build: false\n"
+
+      # add second project with build set to true
+      entries << {
+        'name' => 'build-me-starter',
+        'object' => {
+          '__typename' => 'Tree',
+          'entries' => [
+            {
+              'name' => 'main.py',
+              'extension' => '.py',
+              'object' => {
+                '__typename' => 'Blob',
+                'text' => "print('hello')\n",
+                'isBinary' => false
+              }
+            },
+            {
+              'name' => 'project_config.yml',
+              'extension' => '.yml',
+              'object' => {
+                '__typename' => 'Blob',
+                'text' => "name: \"Build Me\"\nidentifier: \"build-me-starter\"\ntype: \"python\"\nbuild: true\n",
+                'isBinary' => true
+              }
+            }
+          ]
+        }
+      }
+
+      allow(GithubApi::Client).to receive(:query).and_return(graphql_response)
+      allow(ProjectImporter).to receive(:new).and_call_original
+    end
+
+    it 'still imports the later buildable project' do
+      expect { described_class.perform_now(payload) }.to change(Project, :count).by(1)
+      expect(Project.find_by(identifier: 'build-me-starter', locale: 'ja-JP')).to be_present
+    end
+
+    it 'does not import the build: false project' do
+      described_class.perform_now(payload)
+      expect(Project.find_by(identifier: 'dont-collide-starter', locale: 'ja-JP')).to be_nil
+    end
+  end
+
   context 'when GitHub returns nothing for the locale' do
     let(:raw_response) { { data: { repository: nil } } }
 

spec/models/ability_spec.rb

@@ -186,15 +186,15 @@
         it { is_expected.not_to be_able_to(:destroy, project) }
       end
 
-      context 'when user is a school teacher' do
+      context 'when user is another teacher in the same school' do
         before do
           create(:teacher_role, user_id: user.id, school:)
         end
 
-        it { is_expected.to be_able_to(:read, project) }
-        it { is_expected.to be_able_to(:show_context, project) }
+        it { is_expected.not_to be_able_to(:read, project) }
+        it { is_expected.not_to be_able_to(:show_context, project) }
         it { is_expected.not_to be_able_to(:create, project) }
-        it { is_expected.to be_able_to(:update, project) }
+        it { is_expected.not_to be_able_to(:update, project) }
         it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
         it { is_expected.not_to be_able_to(:destroy, project) }
       end
@@ -248,15 +248,15 @@
         it { is_expected.not_to be_able_to(:destroy, project) }
       end
 
-      context 'when user is a school teacher' do
+      context 'when user is another teacher in the same school' do
         before do
           create(:teacher_role, user_id: user.id, school:)
         end
 
-        it { is_expected.to be_able_to(:read, project) }
-        it { is_expected.to be_able_to(:show_context, project) }
+        it { is_expected.not_to be_able_to(:read, project) }
+        it { is_expected.not_to be_able_to(:show_context, project) }
         it { is_expected.not_to be_able_to(:create, project) }
-        it { is_expected.to be_able_to(:update, project) }
+        it { is_expected.not_to be_able_to(:update, project) }
         it { is_expected.not_to be_able_to(:set_finished, project.school_project) }
         it { is_expected.not_to be_able_to(:destroy, project) }
       end