From 293bcccc807045a23e813d947104ae0dc8ba5659 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Tue, 16 Feb 2021 14:24:35 +0100
Subject: [PATCH] Introduce new `enabled` option for CodeHarbor

* Fix tests to ensure they work independent of config option
---
 .../codeharbor_links_controller.rb            |  7 +-
 app/policies/codeharbor_link_policy.rb        | 14 ++--
 config/code_ocean.yml.ci                      |  2 +-
 config/code_ocean.yml.example                 |  3 +
 config/locales/de.yml                         | 30 +++----
 config/locales/en.yml                         | 28 +++----
 .../codeharbor_links_controller_spec.rb       | 25 ++++--
 spec/policies/codeharbor_link_policy_spec.rb  | 78 ++++++++++++++-----
 8 files changed, 125 insertions(+), 62 deletions(-)

diff --git a/app/controllers/codeharbor_links_controller.rb b/app/controllers/codeharbor_links_controller.rb
index 802448f6..087f2736 100644
--- a/app/controllers/codeharbor_links_controller.rb
+++ b/app/controllers/codeharbor_links_controller.rb
@@ -2,10 +2,11 @@
 
 class CodeharborLinksController < ApplicationController
   include CommonBehavior
+  before_action :verify_codeharbor_activation
   before_action :set_codeharbor_link, only: %i[show edit update destroy]
 
   def new
-    base_url = CodeOcean::Config.new(:code_ocean).read[:codeharbor][:url]
+    base_url = CodeOcean::Config.new(:code_ocean).read[:codeharbor][:url] || ''
     @codeharbor_link = CodeharborLink.new(push_url: base_url + '/import_exercise', check_uuid_url: base_url + '/import_uuid_check')
     authorize!
   end
@@ -36,6 +37,10 @@ class CodeharborLinksController < ApplicationController
     authorize @codeharbor_link
   end
 
+  def verify_codeharbor_activation
+    raise Pundit::NotAuthorizedError unless policy(CodeharborLink).enabled?
+  end
+
   def set_codeharbor_link
     @codeharbor_link = CodeharborLink.find(params[:id])
     @codeharbor_link.user = current_user
diff --git a/app/policies/codeharbor_link_policy.rb b/app/policies/codeharbor_link_policy.rb
index 3dd4ff54..b0dd9a8a 100644
--- a/app/policies/codeharbor_link_policy.rb
+++ b/app/policies/codeharbor_link_policy.rb
@@ -8,23 +8,27 @@ class CodeharborLinkPolicy < ApplicationPolicy
   end
 
   def new?
-    teacher? || admin?
+    enabled? && (teacher? || admin?)
   end
 
   def create?
-    teacher? || admin?
+    enabled? && (teacher? || admin?)
   end
 
   def edit?
-    owner?
+    enabled? && owner?
   end
 
   def update?
-    owner?
+    enabled? && owner?
   end
 
   def destroy?
-    owner?
+    enabled? && owner?
+  end
+
+  def enabled?
+    CodeOcean::Config.new(:code_ocean).read[:codeharbor][:enabled]
   end
 
   private
diff --git a/config/code_ocean.yml.ci b/config/code_ocean.yml.ci
index 09eb8cb5..c232e663 100644
--- a/config/code_ocean.yml.ci
+++ b/config/code_ocean.yml.ci
@@ -4,6 +4,6 @@ test:
   code_pilot:
     enabled: false
   codeharbor:
-    url: http://test.url
+    enabled: true
   codeocean_events:
     enabled: false
diff --git a/config/code_ocean.yml.example b/config/code_ocean.yml.example
index 36d238ff..78c705a5 100644
--- a/config/code_ocean.yml.example
+++ b/config/code_ocean.yml.example
@@ -4,6 +4,8 @@ default: &default
     answers_per_query: 3
   code_pilot:
     enabled: false
+  codeharbor:
+    enabled: false
   codeocean_events:
     enabled: false
 
@@ -15,6 +17,7 @@ development:
     enabled: false
     url: //localhost:3000
   codeharbor:
+    enabled: true
     url: https://codeharbor.openhpi.de
 
 production:
diff --git a/config/locales/de.yml b/config/locales/de.yml
index d21a2e1d..c1b87949 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -161,7 +161,7 @@ de:
       exercise_collection_item:
         exercise: "Aufgabe"
     models:
-      code_harbor_link:
+      codeharbor_link:
         one: CodeHarbor-Link
         other: CodeHarbor-Links
       consumer:
@@ -269,13 +269,13 @@ de:
   codeharbor_link:
     generate: Generieren
     info:
-      push_url: Die Url von Codeharbor zu der die Aufgabe exportiert werden soll. Bei Unklarheiten bitte an einen Admin wenden.
-      check_uuid_url: Die Url von Codeharbor an der geprüft werden kann, ob die Aufgabe schon existiert. Bei Unklarheiten bitte an einen Admin wenden.
-      api_key: Wird zum Authentifizieren gegenüber Codeharbor genutzt. Auf Codeharbor muss der gleiche Key eingetragen sein.
-    profile_label: Codeharbor Link
-    new: Neue Codeharbor Link anlegen
-    edit: Codeharbor Link bearbeiten
-    delete: Codeharbor Link entfernen
+      push_url: Die Url von CodeHarbor zu der die Aufgabe exportiert werden soll. Bei Unklarheiten bitte an einen Admin wenden.
+      check_uuid_url: Die Url von CodeHarbor an der geprüft werden kann, ob die Aufgabe schon existiert. Bei Unklarheiten bitte an einen Admin wenden.
+      api_key: Wird zum Authentifizieren gegenüber CodeHarbor genutzt. Auf CodeHarbor muss der gleiche Key eingetragen sein.
+    profile_label: CodeHarbor-Link
+    new: Neuen CodeHarbor-Link anlegen
+    edit: CodeHarbor-Link bearbeiten
+    delete: CodeHarbor-Link entfernen
   execution_environments:
     form:
       hints:
@@ -350,21 +350,21 @@ de:
         invalid: Fehlerhafte Aufgabe
         internal_error: Beim Import der Aufgabe ist ein interner Fehler aufgetreten.
     export_codeharbor:
-      label: Zu Codeharbor exportieren
-      dialogtitle: Zu Codeharbor exportieren
+      label: Zu CodeHarbor exportieren
+      dialogtitle: Zu CodeHarbor exportieren
       successfully_exported: 'Aufgabe wurde erfolgreich exportiert.<br>ID: %{id}<br>Title: %{title}'
       export_failed: 'Export ist fehlgeschlagen.<br>ID: %{id}<br>Title: %{title}<br><br>Error: %{error}'
-      error: Es ist ein Fehler bei der Kommunikation mit Codeharbor aufgetreten.
-      checking_codeharbor: Es wird geprüft, ob auf Codeharbor eine korrespondierende Aufgabe gefunden werden kann.
+      error: Es ist ein Fehler bei der Kommunikation mit CodeHarbor aufgetreten.
+      checking_codeharbor: Es wird geprüft, ob auf CodeHarbor eine korrespondierende Aufgabe gefunden werden kann.
       buttons:
         retry: Erneut probieren
         export: Export
         close: Schließen
         abort: Abbrechen
       check:
-        no_exercise: Auf Codeharbor wurde keine entsprechende Aufgabe gefunden. Mit dem Export der Aufgabe wird eine neue auf Codeharbor angelegt, die mit dieser verbunden ist. Anschließend können Veränderungen an der Aufgabe von beiden Systemen aus jeweils in das andere Übertragen werden.
-        exercise_found: Auf Codeharbor wurde eine entsprechende Aufgabe gefunden. Mit dem Export der Aufgabe werden alle Veränderungen, die auf Codeocean vorgenommen wurden, exportiert und die Aufgabe auf Codeharbor überschrieben.
-        exercise_found_no_right: Auf Codeharbor wurde eine entsprechende Aufgabe gefunden, Sie haben aber keine Rechte sie zu bearbeiten. Bitte wenden Sie sich an einen Admin, wenn Sie denken, dass Sie die Rechte dazu besitzen sollten.
+        no_exercise: Auf CodeHarbor wurde keine entsprechende Aufgabe gefunden. Mit dem Export der Aufgabe wird eine neue auf CodeHarbor angelegt, die mit dieser verbunden ist. Anschließend können Veränderungen an der Aufgabe von beiden Systemen aus jeweils in das andere Übertragen werden.
+        exercise_found: Auf CodeHarbor wurde eine entsprechende Aufgabe gefunden. Mit dem Export der Aufgabe werden alle Veränderungen, die auf Codeocean vorgenommen wurden, exportiert und die Aufgabe auf CodeHarbor überschrieben.
+        exercise_found_no_right: Auf CodeHarbor wurde eine entsprechende Aufgabe gefunden, Sie haben aber keine Rechte sie zu bearbeiten. Bitte wenden Sie sich an einen Admin, wenn Sie denken, dass Sie die Rechte dazu besitzen sollten.
     file_form:
       hints:
         feedback_message: Diese Nachricht wird als Hinweis zu fehlschlagenden Tests angezeigt.
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 2f8f2413..c1f9cc64 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -161,7 +161,7 @@ en:
       exercise_collection_item:
         exercise: "Exercise"
     models:
-      code_harbor_link:
+      codeharbor_link:
         one: CodeHarbor Link
         other: CodeHarbor Links
       consumer:
@@ -269,13 +269,13 @@ en:
   codeharbor_link:
     generate: Generate
     info:
-      push_url: The url from Codeharbor where your exercise can be exported to. If you don't know what to write here, ask an admin.
-      check_uuid_url: The url from Codeharbor where we can check if the exercise already exists. If you don't know what to write here, ask an admin.
+      push_url: The url from CodeHarbor where your exercise can be exported to. If you don't know what to write here, ask an admin.
+      check_uuid_url: The url from CodeHarbor where we can check if the exercise already exists. If you don't know what to write here, ask an admin.
       api_key: Will be used to authenticate your export request. Has to be the same on both sides.
-    profile_label: Codeharbor Link
-    new: Create link to Codeharbor
-    edit: Edit existing link
-    delete: Remove Codeharbor link
+    profile_label: CodeHarbor Link
+    new: Create link to CodeHarbor
+    edit: Edit existing CodeHarbor link
+    delete: Remove CodeHarbor link
   execution_environments:
     form:
       hints:
@@ -350,21 +350,21 @@ en:
         invalid: Invalid exercise
         internal_error: An internal error occurred on CodeOcean while importing the exercise.
     export_codeharbor:
-      label: Export to Codeharbor
-      dialogtitle: Export to Codeharbor
+      label: Export to CodeHarbor
+      dialogtitle: Export to CodeHarbor
       successfully_exported: 'Exercise has been successfully exported.<br>ID: %{id}<br>Title: %{title}'
       export_failed: 'Export has failed.<br>ID: %{id}<br>Title: %{title}<br><br>Error: %{error}'
-      error: 'An error occurred while contacting Codeharbor'
-      checking_codeharbor: Checking if the exercise exists on Codeharbor.
+      error: 'An error occurred while contacting CodeHarbor'
+      checking_codeharbor: Checking if the exercise exists on CodeHarbor.
       buttons:
         retry: Retry
         export: Export
         close: Close
         abort: Abort
       check:
-        no_exercise: No corresponding exercise found on Codeharbor. Pushing this exercise will create a new exercise on Codeharbor, which will be linked to this one on Codeocean. Any changes to either one can be pushed to the respective other platform.
-        exercise_found: 'A corresponding exercise has been found on Codeharbor. You can export the exercise to transfer all changes made on Codeocean to Codeharbor. Careful: This will overwrite all potential changes made on Codeharbor.'
-        exercise_found_no_right: A corresponding exercise has been found on Codeharbor, but you don't have the rights to edit it. Please contact an Admin if you think you should be able to edit the exercise on Codeharbor.
+        no_exercise: No corresponding exercise found on CodeHarbor. Pushing this exercise will create a new exercise on CodeHarbor, which will be linked to this one on Codeocean. Any changes to either one can be pushed to the respective other platform.
+        exercise_found: 'A corresponding exercise has been found on CodeHarbor. You can export the exercise to transfer all changes made on Codeocean to CodeHarbor. Careful: This will overwrite all potential changes made on CodeHarbor.'
+        exercise_found_no_right: A corresponding exercise has been found on CodeHarbor, but you don't have the rights to edit it. Please contact an Admin if you think you should be able to edit the exercise on CodeHarbor.
     file_form:
       hints:
         feedback_message: This message is used as a hint for failing tests.
diff --git a/spec/controllers/codeharbor_links_controller_spec.rb b/spec/controllers/codeharbor_links_controller_spec.rb
index b057c32f..41363805 100644
--- a/spec/controllers/codeharbor_links_controller_spec.rb
+++ b/spec/controllers/codeharbor_links_controller_spec.rb
@@ -1,11 +1,23 @@
+# frozen_string_literal: true
+
 require 'rails_helper'
 
 describe CodeharborLinksController do
   let(:user) { FactoryBot.create(:teacher) }
-  before(:each) { allow(controller).to receive(:current_user).and_return(user) }
 
-  describe 'GET #new' do
-    before { get :new }
+  let(:codeocean_config) { instance_double(CodeOcean::Config) }
+  let(:codeharbor_config) { {codeharbor: {enabled: true, url: 'http://test.url'}} }
+
+  before do
+    allow(CodeOcean::Config).to receive(:new).with(:code_ocean).and_return(codeocean_config)
+    allow(codeocean_config).to receive(:read).and_return(codeharbor_config)
+    allow(controller).to receive(:current_user).and_return(user)
+  end
+
+  context 'GET #new' do
+    before do
+      get :new
+    end
 
     expect_status(200)
   end
@@ -13,7 +25,7 @@ describe CodeharborLinksController do
   describe 'GET #edit' do
     let(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) }
 
-    before { get :edit, params: { id: codeharbor_link.id } }
+    before { get :edit, params: {id: codeharbor_link.id} }
 
     expect_status(200)
   end
@@ -52,9 +64,11 @@ describe CodeharborLinksController do
     it 'updates push_url' do
       expect { put_request }.to change { codeharbor_link.reload.push_url }.to('http://foo.bar/push')
     end
+
     it 'updates check_uuid_url' do
       expect { put_request }.to change { codeharbor_link.reload.check_uuid_url }.to('http://foo.bar/check')
     end
+
     it 'updates api_key' do
       expect { put_request }.to change { codeharbor_link.reload.api_key }.to('api_key')
     end
@@ -67,7 +81,7 @@ describe CodeharborLinksController do
       let(:params) { {push_url: '', check_uuid_url: '', api_key: ''} }
 
       it 'does not change codeharbor_link' do
-        expect { put_request }.not_to change { codeharbor_link.reload.attributes }
+        expect { put_request }.not_to(change { codeharbor_link.reload.attributes })
       end
 
       it 'redirects to user show' do
@@ -90,4 +104,3 @@ describe CodeharborLinksController do
     end
   end
 end
-
diff --git a/spec/policies/codeharbor_link_policy_spec.rb b/spec/policies/codeharbor_link_policy_spec.rb
index dcb6ace3..90646f9c 100644
--- a/spec/policies/codeharbor_link_policy_spec.rb
+++ b/spec/policies/codeharbor_link_policy_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'rails_helper'
 
 describe CodeharborLinkPolicy do
@@ -5,37 +7,73 @@ describe CodeharborLinkPolicy do
 
   let(:codeharbor_link) { FactoryBot.create(:codeharbor_link) }
 
-  %i[index? show?].each do |action|
-    permissions(action) do
-      it 'does not grant access any user' do
-        %i[external_user teacher admin].each do |factory_name|
-          expect(policy).not_to permit(FactoryBot.create(factory_name), codeharbor_link)
+  context 'when CodeHarbor link is enabled' do
+    let(:codeocean_config) { instance_double(CodeOcean::Config) }
+    let(:codeharbor_config) { {codeharbor: {enabled: true}} }
+
+    before do
+      allow(CodeOcean::Config).to receive(:new).with(:code_ocean).and_return(codeocean_config)
+      allow(codeocean_config).to receive(:read).and_return(codeharbor_config)
+    end
+
+    %i[index? show?].each do |action|
+      permissions(action) do
+        it 'does not grant access any user' do
+          %i[external_user teacher admin].each do |factory_name|
+            expect(policy).not_to permit(FactoryBot.create(factory_name), codeharbor_link)
+          end
         end
       end
     end
-  end
 
-  %i[new? create?].each do |action|
-    permissions(action) do
-      it 'grants access to teachers' do
-        %i[teacher admin].each do |factory_name|
+    %i[new? create?].each do |action|
+      permissions(action) do
+        it 'grants access to teachers' do
+          %i[teacher admin].each do |factory_name|
+            expect(policy).to permit(FactoryBot.create(factory_name), codeharbor_link)
+          end
+        end
+
+        it 'does not grant access to all other users' do
+          expect(policy).not_to permit(FactoryBot.create(:external_user), codeharbor_link)
+        end
+      end
+    end
+
+    %i[destroy? edit? update?].each do |action|
+      permissions(action) do
+        it 'grants access to the owner of the link' do
+          expect(policy).to permit(codeharbor_link.user, codeharbor_link)
+        end
+
+        it 'does not grant access to arbitrary users' do
+          %i[external_user admin teacher].each do |factory_name|
+            expect(policy).not_to permit(FactoryBot.create(factory_name), codeharbor_link)
+          end
+        end
+      end
+    end
+
+    permissions(:enabled?) do
+      it 'reflects the config option' do
+        %i[external_user admin teacher].each do |factory_name|
           expect(policy).to permit(FactoryBot.create(factory_name), codeharbor_link)
         end
       end
-
-      it 'does not grant access to all other users' do
-        expect(policy).not_to permit(FactoryBot.create(:external_user), codeharbor_link)
-      end
     end
   end
 
-  %i[destroy? edit? update?].each do |action|
-    permissions(action) do
-      it 'grants access to the owner of the link' do
-        expect(policy).to permit(codeharbor_link.user, codeharbor_link)
-      end
+  context 'when CodeHabor link is disabled' do
+    let(:codeocean_config) { instance_double(CodeOcean::Config) }
+    let(:codeharbor_config) { {codeharbor: {enabled: false}} }
 
-      it 'does not grant access to arbitrary users' do
+    before do
+      allow(CodeOcean::Config).to receive(:new).with(:code_ocean).and_return(codeocean_config)
+      allow(codeocean_config).to receive(:read).and_return(codeharbor_config)
+    end
+
+    permissions(:enabled?) do
+      it 'reflects the config option' do
         %i[external_user admin teacher].each do |factory_name|
           expect(policy).not_to permit(FactoryBot.create(factory_name), codeharbor_link)
         end