From 72def4a661c8852da24e094695c62cc3ca0c5f41 Mon Sep 17 00:00:00 2001 From: yqbk Date: Thu, 30 Jun 2016 12:00:54 +0200 Subject: [PATCH 01/17] delete class team --- app/models/team.rb | 10 ---------- app/views/application/_navigation.html.slim | 2 +- 2 files changed, 1 insertion(+), 11 deletions(-) delete mode 100644 app/models/team.rb diff --git a/app/models/team.rb b/app/models/team.rb deleted file mode 100644 index a0dcb8d6..00000000 --- a/app/models/team.rb +++ /dev/null @@ -1,10 +0,0 @@ -class Team < ActiveRecord::Base - has_and_belongs_to_many :internal_users - alias_method :members, :internal_users - - validates :name, presence: true - - def to_s - name - end -end diff --git a/app/views/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index 4ab39e30..3c260cb8 100644 --- a/app/views/application/_navigation.html.slim +++ b/app/views/application/_navigation.html.slim @@ -8,7 +8,7 @@ - if current_user.admin? li = link_to(t('breadcrumbs.dashboard.show'), admin_dashboard_path) li.divider - - models = [ExecutionEnvironment, Exercise, Consumer, CodeHarborLink, ExternalUser, FileType, InternalUser, Submission, Team].sort_by { |model| model.model_name.human(count: 2) } + - models = [ExecutionEnvironment, Exercise, Consumer, CodeHarborLink, ExternalUser, FileType, InternalUser, Submission].sort_by { |model| model.model_name.human(count: 2) } - models.each do |model| - if policy(model).index? li = link_to(model.model_name.human(count: 2), send(:"#{model.model_name.collection}_path")) From a1023eb9fa07dadf504ca9ef502e4fcda585931a Mon Sep 17 00:00:00 2001 From: yqbk Date: Thu, 30 Jun 2016 12:02:44 +0200 Subject: [PATCH 02/17] delete teams view --- app/views/teams/_form.html.slim | 9 --------- app/views/teams/edit.html.slim | 3 --- app/views/teams/index.html.slim | 20 -------------------- app/views/teams/new.html.slim | 3 --- app/views/teams/show.html.slim | 9 --------- 5 files changed, 44 deletions(-) delete mode 100644 app/views/teams/_form.html.slim delete mode 100644 app/views/teams/edit.html.slim delete mode 100644 app/views/teams/index.html.slim delete mode 100644 app/views/teams/new.html.slim delete mode 100644 app/views/teams/show.html.slim diff --git a/app/views/teams/_form.html.slim b/app/views/teams/_form.html.slim deleted file mode 100644 index 47f547dd..00000000 --- a/app/views/teams/_form.html.slim +++ /dev/null @@ -1,9 +0,0 @@ -= form_for(@team) do |f| - = render('shared/form_errors', object: @team) - .form-group - = f.label(:name) - = f.text_field(:name, class: 'form-control', required: true) - .form-group - = f.label(:internal_user_ids) - = f.collection_select(:internal_user_ids, InternalUser.all.order(:name), :id, :name, {}, {class: 'form-control', multiple: true}) - .actions = render('shared/submit_button', f: f, object: @team) diff --git a/app/views/teams/edit.html.slim b/app/views/teams/edit.html.slim deleted file mode 100644 index 0c1f4dac..00000000 --- a/app/views/teams/edit.html.slim +++ /dev/null @@ -1,3 +0,0 @@ -h1 = @hint - -= render('form') diff --git a/app/views/teams/index.html.slim b/app/views/teams/index.html.slim deleted file mode 100644 index 4eb6640b..00000000 --- a/app/views/teams/index.html.slim +++ /dev/null @@ -1,20 +0,0 @@ -h1 = Team.model_name.human(count: 2) - -.table-responsive - table.table - thead - tr - th = t('activerecord.attributes.team.name') - th = t('activerecord.attributes.team.internal_user_ids') - th colspan=3 = t('shared.actions') - tbody - - @teams.each do |team| - tr - td = team.name - td = team.members.count - td = link_to(t('shared.show'), team_path(team.id)) - td = link_to(t('shared.edit'), edit_team_path(team.id)) - td = link_to(t('shared.destroy'), team_path(team.id), data: {confirm: t('shared.confirm_destroy')}, method: :delete) - -= render('shared/pagination', collection: @teams) -p = render('shared/new_button', model: Team, path: new_team_path) diff --git a/app/views/teams/new.html.slim b/app/views/teams/new.html.slim deleted file mode 100644 index 8c8f2aab..00000000 --- a/app/views/teams/new.html.slim +++ /dev/null @@ -1,3 +0,0 @@ -h1 = t('shared.new_model', model: Team.model_name.human) - -= render('form') diff --git a/app/views/teams/show.html.slim b/app/views/teams/show.html.slim deleted file mode 100644 index 1b37d931..00000000 --- a/app/views/teams/show.html.slim +++ /dev/null @@ -1,9 +0,0 @@ -h1 - = @team - = render('shared/edit_button', object: @team, path: edit_team_path(@team.id)) - -= row(label: 'team.name', value: @team.name) -= row(label: 'team.internal_user_ids') do - ul.list-unstyled - - @team.members.order(:name).each do |internal_user| - li = link_to(internal_user, internal_user) From 0046706fce6f1660df06cd89a9b7c91864453aa3 Mon Sep 17 00:00:00 2001 From: yqbk Date: Thu, 30 Jun 2016 12:04:16 +0200 Subject: [PATCH 03/17] remove team policy --- app/policies/team_policy.rb | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 app/policies/team_policy.rb diff --git a/app/policies/team_policy.rb b/app/policies/team_policy.rb deleted file mode 100644 index ff05c0c3..00000000 --- a/app/policies/team_policy.rb +++ /dev/null @@ -1,14 +0,0 @@ -class TeamPolicy < ApplicationPolicy - [:create?, :index?, :new?].each do |action| - define_method(action) { admin? } - end - - [:destroy?, :edit?, :show?, :update?].each do |action| - define_method(action) { admin? || member? } - end - - def member? - @record.members.include?(@user) - end - private :member? -end From 2ab0829cb3d160a767763d21614c338d3b2e34c8 Mon Sep 17 00:00:00 2001 From: yqbk Date: Thu, 30 Jun 2016 12:05:25 +0200 Subject: [PATCH 04/17] remove team controller --- app/controllers/teams_controller.rb | 51 ----------------------------- 1 file changed, 51 deletions(-) delete mode 100644 app/controllers/teams_controller.rb diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb deleted file mode 100644 index 1ca8f1fe..00000000 --- a/app/controllers/teams_controller.rb +++ /dev/null @@ -1,51 +0,0 @@ -class TeamsController < ApplicationController - include CommonBehavior - - before_action :set_team, only: MEMBER_ACTIONS - - def authorize! - authorize(@team || @teams) - end - private :authorize! - - def create - @team = Team.new(team_params) - authorize! - create_and_respond(object: @team) - end - - def destroy - destroy_and_respond(object: @team) - end - - def edit - end - - def index - @teams = Team.all.includes(:internal_users).order(:name).paginate(page: params[:page]) - authorize! - end - - def new - @team = Team.new - authorize! - end - - def set_team - @team = Team.find(params[:id]) - authorize! - end - private :set_team - - def show - end - - def team_params - params[:team].permit(:name, internal_user_ids: []) - end - private :team_params - - def update - update_and_respond(object: @team, params: team_params) - end -end From b0d468c0e055035606725332688f9552e7e23757 Mon Sep 17 00:00:00 2001 From: yqbk Date: Thu, 30 Jun 2016 12:17:19 +0200 Subject: [PATCH 05/17] remove rest of teams code -- NEED TO REMOVE TEAMS FROM DATABASE! --- app/controllers/exercises_controller.rb | 8 +------- app/models/exercise.rb | 3 +-- app/models/internal_user.rb | 2 -- app/policies/exercise_policy.rb | 8 ++------ app/views/exercises/_form.html.slim | 3 --- app/views/exercises/show.html.slim | 1 - config/locales/de.yml | 8 -------- config/locales/en.yml | 8 -------- config/routes.rb | 1 - 9 files changed, 4 insertions(+), 38 deletions(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 0304abb4..45fd04d9 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -9,7 +9,6 @@ class ExercisesController < ApplicationController before_action :set_exercise, only: MEMBER_ACTIONS + [:clone, :implement, :run, :statistics, :submit, :reload] before_action :set_external_user, only: [:statistics] before_action :set_file_types, only: [:create, :edit, :new, :update] - before_action :set_teams, only: [:create, :edit, :new, :update] skip_before_filter :verify_authenticity_token, only: [:import_proforma_xml] skip_after_action :verify_authorized, only: [:import_proforma_xml] @@ -119,7 +118,7 @@ class ExercisesController < ApplicationController private :user_by_code_harbor_token def exercise_params - params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :team_id, :title, files_attributes: file_attributes).merge(user_id: current_user.id, user_type: current_user.class.name) + params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :title, files_attributes: file_attributes).merge(user_id: current_user.id, user_type: current_user.class.name) end private :exercise_params @@ -195,11 +194,6 @@ class ExercisesController < ApplicationController end private :set_file_types - def set_teams - @teams = Team.all.order(:name) - end - private :set_teams - def show end diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 4a1e0486..24570dc6 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -10,8 +10,7 @@ class Exercise < ActiveRecord::Base after_initialize :set_default_values belongs_to :execution_environment - has_many :submissions - belongs_to :team + has_many :submission has_many :external_users, source: :user, source_type: ExternalUser, through: :submissions has_many :internal_users, source: :user, source_type: InternalUser, through: :submissions diff --git a/app/models/internal_user.rb b/app/models/internal_user.rb index e5cebde9..8f1bf04b 100644 --- a/app/models/internal_user.rb +++ b/app/models/internal_user.rb @@ -3,8 +3,6 @@ class InternalUser < ActiveRecord::Base authenticates_with_sorcery! - has_and_belongs_to_many :teams - validates :email, presence: true, uniqueness: true validates :password, confirmation: true, if: :password_void?, on: :update, presence: true validates :role, inclusion: {in: ROLES} diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index 29a1c570..d30c4aff 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -13,23 +13,19 @@ class ExercisePolicy < AdminOrAuthorPolicy end [:clone?, :destroy?, :edit?, :statistics?, :update?].each do |action| - define_method(action) { admin? || author? || team_member? } + define_method(action) { admin? || author?} end [:implement?, :submit?, :reload?].each do |action| define_method(action) { everyone } end - def team_member? - @record.team.try(:members, []).include?(@user) if @record.team - end - private :team_member? - class Scope < Scope def resolve if @user.admin? @scope.all elsif @user.internal_user? + #need to remove team query @scope.where('user_id = ? OR public = TRUE OR (team_id IS NOT NULL AND team_id IN (SELECT t.id FROM teams t JOIN internal_users_teams iut ON t.id = iut.team_id WHERE iut.internal_user_id = ?))', @user.id, @user.id) else @scope.none diff --git a/app/views/exercises/_form.html.slim b/app/views/exercises/_form.html.slim index f0f69f7a..968540af 100644 --- a/app/views/exercises/_form.html.slim +++ b/app/views/exercises/_form.html.slim @@ -17,9 +17,6 @@ = f.label(:instructions) = f.hidden_field(:instructions) .form-control.markdown - /.form-group - = f.label(:team_id) - = f.collection_select(:team_id, @teams, :id, :name, {include_blank: true}, class: 'form-control') .checkbox label = f.check_box(:public) diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index b4b72932..5c554da8 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -12,7 +12,6 @@ h1 = row(label: 'exercise.description', value: render_markdown(@exercise.description)) = row(label: 'exercise.execution_environment', value: link_to_if(policy(@exercise.execution_environment).show?, @exercise.execution_environment, @exercise.execution_environment)) /= row(label: 'exercise.instructions', value: render_markdown(@exercise.instructions)) -= row(label: 'exercise.team', value: @exercise.team ? link_to(@exercise.team, @exercise.team) : nil) = row(label: 'exercise.maximum_score', value: @exercise.maximum_score) = row(label: 'exercise.public', value: @exercise.public?) = row(label: 'exercise.hide_file_tree', value: @exercise.hide_file_tree?) diff --git a/config/locales/de.yml b/config/locales/de.yml index 53df1858..ce8ba98f 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -34,8 +34,6 @@ de: instructions: Anweisungen maximum_score: Erreichbare Punktzahl public: Öffentlich - team: Team - team_id: Team title: Titel user: Autor allow_file_creation: "Dateierstellung erlauben" @@ -91,9 +89,6 @@ de: files: Dateien score: Punktzahl user: Autor - team: - internal_user_ids: Mitglieder - name: Name models: code_harbor_link: one: CodeHarbor-Link @@ -128,9 +123,6 @@ de: submission: one: Abgabe other: Abgaben - team: - one: Team - other: Teams errors: messages: together: 'muss zusammen mit %{attribute} definiert werden' diff --git a/config/locales/en.yml b/config/locales/en.yml index 1e72cb34..86acbb15 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -34,8 +34,6 @@ en: instructions: Instructions maximum_score: Maximum Score public: Public - team: Team - team_id: Team title: Title user: Author allow_file_creation: "Allow file creation" @@ -91,9 +89,6 @@ en: files: Files score: Score user: Author - team: - internal_user_ids: Members - name: Name models: code_harbor_link: one: CodeHarbor Link @@ -128,9 +123,6 @@ en: submission: one: Submission other: Submissions - team: - one: Team - other: Teams errors: messages: together: 'has to be set along with %{attribute}' diff --git a/config/routes.rb b/config/routes.rb index a6ca545c..f13d1c9f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -99,5 +99,4 @@ Rails.application.routes.draw do end end - resources :teams end From 19f214caf1d9d2f2bf4f4d2b0419d9946e363bfb Mon Sep 17 00:00:00 2001 From: yqbk Date: Mon, 4 Jul 2016 16:36:23 +0200 Subject: [PATCH 06/17] remove teams from databse --- db/migrate/20160704142512_drop_teams.rb | 7 +++++++ db/migrate/20160704143402_remove_teams.rb | 7 +++++++ 2 files changed, 14 insertions(+) create mode 100644 db/migrate/20160704142512_drop_teams.rb create mode 100644 db/migrate/20160704143402_remove_teams.rb diff --git a/db/migrate/20160704142512_drop_teams.rb b/db/migrate/20160704142512_drop_teams.rb new file mode 100644 index 00000000..32a0532a --- /dev/null +++ b/db/migrate/20160704142512_drop_teams.rb @@ -0,0 +1,7 @@ +class DropTeams < ActiveRecord::Migration + def change + remove_column :exercises, :team_id, :integer + drop_table :teams + drop_table :internal_users_teams + end +end diff --git a/db/migrate/20160704143402_remove_teams.rb b/db/migrate/20160704143402_remove_teams.rb new file mode 100644 index 00000000..9c01871a --- /dev/null +++ b/db/migrate/20160704143402_remove_teams.rb @@ -0,0 +1,7 @@ +class RemoveTeams < ActiveRecord::Migration + def change + remove_column :exercises, :team_id, :integer + drop_table :teams + drop_table :internal_users_teams + end +end From a98107479e58d19464e0510911d4a9f2f2493eb2 Mon Sep 17 00:00:00 2001 From: yqbk Date: Mon, 4 Jul 2016 16:37:59 +0200 Subject: [PATCH 07/17] remove additional files --- db/migrate/20160704142512_drop_teams.rb | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 db/migrate/20160704142512_drop_teams.rb diff --git a/db/migrate/20160704142512_drop_teams.rb b/db/migrate/20160704142512_drop_teams.rb deleted file mode 100644 index 32a0532a..00000000 --- a/db/migrate/20160704142512_drop_teams.rb +++ /dev/null @@ -1,7 +0,0 @@ -class DropTeams < ActiveRecord::Migration - def change - remove_column :exercises, :team_id, :integer - drop_table :teams - drop_table :internal_users_teams - end -end From 2bb1362520bda92423d6d3750af350c79e6451d7 Mon Sep 17 00:00:00 2001 From: yqbk Date: Mon, 4 Jul 2016 16:59:30 +0200 Subject: [PATCH 08/17] further delete (spec files) --- db/migrate/20160704143402_remove_teams.rb | 2 +- db/seeds/development.rb | 3 - spec/controllers/teams_controller_spec.rb | 93 ----------------------- spec/factories/team.rb | 6 -- spec/features/authorization_spec.rb | 6 +- spec/models/team_spec.rb | 9 --- spec/policies/exercise_policy_spec.rb | 20 +---- spec/policies/team_policy_spec.rb | 41 ---------- 8 files changed, 6 insertions(+), 174 deletions(-) delete mode 100644 spec/controllers/teams_controller_spec.rb delete mode 100644 spec/factories/team.rb delete mode 100644 spec/models/team_spec.rb delete mode 100644 spec/policies/team_policy_spec.rb diff --git a/db/migrate/20160704143402_remove_teams.rb b/db/migrate/20160704143402_remove_teams.rb index 9c01871a..20b8a204 100644 --- a/db/migrate/20160704143402_remove_teams.rb +++ b/db/migrate/20160704143402_remove_teams.rb @@ -1,6 +1,6 @@ class RemoveTeams < ActiveRecord::Migration def change - remove_column :exercises, :team_id, :integer + remove_reference :exercises, :team drop_table :teams drop_table :internal_users_teams end diff --git a/db/seeds/development.rb b/db/seeds/development.rb index 74783d39..6fc8e050 100644 --- a/db/seeds/development.rb +++ b/db/seeds/development.rb @@ -22,6 +22,3 @@ Hint.create_factories # submissions FactoryGirl.create(:submission, exercise: @exercises[:fibonacci]) - -# teams -FactoryGirl.create(:team, internal_users: InternalUser.limit(10)) diff --git a/spec/controllers/teams_controller_spec.rb b/spec/controllers/teams_controller_spec.rb deleted file mode 100644 index 78dfc60d..00000000 --- a/spec/controllers/teams_controller_spec.rb +++ /dev/null @@ -1,93 +0,0 @@ -require 'rails_helper' - -describe TeamsController do - let(:team) { FactoryGirl.create(:team) } - let(:user) { FactoryGirl.create(:admin) } - before(:each) { allow(controller).to receive(:current_user).and_return(user) } - - describe 'POST #create' do - context 'with a valid team' do - let(:request) { proc { post :create, team: FactoryGirl.attributes_for(:team) } } - before(:each) { request.call } - - expect_assigns(team: Team) - - it 'creates the team' do - expect { request.call }.to change(Team, :count).by(1) - end - - expect_redirect(Team.last) - end - - context 'with an invalid team' do - before(:each) { post :create, team: {} } - - expect_assigns(team: Team) - expect_status(200) - expect_template(:new) - end - end - - describe 'DELETE #destroy' do - before(:each) { delete :destroy, id: team.id } - - expect_assigns(team: Team) - - it 'destroys the team' do - team = FactoryGirl.create(:team) - expect { delete :destroy, id: team.id }.to change(Team, :count).by(-1) - end - - expect_redirect(:teams) - end - - describe 'GET #edit' do - before(:each) { get :edit, id: team.id } - - expect_assigns(team: Team) - expect_status(200) - expect_template(:edit) - end - - describe 'GET #index' do - before(:all) { FactoryGirl.create_pair(:team) } - before(:each) { get :index } - - expect_assigns(teams: Team.all) - expect_status(200) - expect_template(:index) - end - - describe 'GET #new' do - before(:each) { get :new } - - expect_assigns(team: Team) - expect_status(200) - expect_template(:new) - end - - describe 'GET #show' do - before(:each) { get :show, id: team.id } - - expect_assigns(team: :team) - expect_status(200) - expect_template(:show) - end - - describe 'PUT #update' do - context 'with a valid team' do - before(:each) { put :update, team: FactoryGirl.attributes_for(:team), id: team.id } - - expect_assigns(team: Team) - expect_redirect(:team) - end - - context 'with an invalid team' do - before(:each) { put :update, team: {name: ''}, id: team.id } - - expect_assigns(team: Team) - expect_status(200) - expect_template(:edit) - end - end -end diff --git a/spec/factories/team.rb b/spec/factories/team.rb deleted file mode 100644 index f34d323e..00000000 --- a/spec/factories/team.rb +++ /dev/null @@ -1,6 +0,0 @@ -FactoryGirl.define do - factory :team do - internal_users { build_pair :teacher } - name 'The A-Team' - end -end diff --git a/spec/features/authorization_spec.rb b/spec/features/authorization_spec.rb index 6a7eaad0..7f0ff04f 100644 --- a/spec/features/authorization_spec.rb +++ b/spec/features/authorization_spec.rb @@ -5,7 +5,7 @@ describe 'Authorization' do let(:user) { FactoryGirl.create(:admin) } before(:each) { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } - [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser, Team].each do |model| + [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser].each do |model| expect_permitted_path(:"new_#{model.model_name.singular}_path") end end @@ -14,7 +14,7 @@ describe 'Authorization' do let(:user) { FactoryGirl.create(:external_user) } before(:each) { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } - [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser, Team].each do |model| + [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser].each do |model| expect_forbidden_path(:"new_#{model.model_name.singular}_path") end end @@ -27,7 +27,7 @@ describe 'Authorization' do expect_forbidden_path(:"new_#{model.model_name.singular}_path") end - [ExecutionEnvironment, Exercise, FileType, Team].each do |model| + [ExecutionEnvironment, Exercise, FileType].each do |model| expect_permitted_path(:"new_#{model.model_name.singular}_path") end end diff --git a/spec/models/team_spec.rb b/spec/models/team_spec.rb deleted file mode 100644 index 777efd32..00000000 --- a/spec/models/team_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'rails_helper' - -describe Team do - let(:team) { described_class.create } - - it 'validates the presence of a name' do - expect(team.errors[:name]).to be_present - end -end diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index c9762f9e..1b25fbca 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -3,8 +3,6 @@ require 'rails_helper' describe ExercisePolicy do subject { described_class } - let(:exercise) { FactoryGirl.build(:dummy, team: FactoryGirl.create(:team)) } - permissions :batch_update? do it 'grants access to admins only' do expect(subject).to permit(FactoryGirl.build(:admin), exercise) @@ -40,10 +38,6 @@ describe ExercisePolicy do expect(subject).to permit(exercise.author, exercise) end - it 'grants access to team members' do - expect(subject).to permit(exercise.team.members.first, exercise) - end - it 'does not grant access to all other users' do [:external_user, :teacher].each do |factory_name| expect(subject).not_to permit(FactoryGirl.build(factory_name), exercise) @@ -71,9 +65,7 @@ describe ExercisePolicy do [@admin, @teacher].each do |user| [true, false].each do |public| - [@team, nil].each do |team| - FactoryGirl.create(:dummy, public: public, team: team, user_id: user.id, user_type: InternalUser.class.name) - end + FactoryGirl.create(:dummy, public: public, user_id: user.id, user_type: InternalUser.class.name) end end end @@ -95,10 +87,6 @@ describe ExercisePolicy do end context 'for teachers' do - before(:each) do - @team = FactoryGirl.create(:team) - @team.members << @teacher - end let(:scope) { Pundit.policy_scope!(@teacher, Exercise) } @@ -110,12 +98,8 @@ describe ExercisePolicy do expect(scope.map(&:id)).to include(*Exercise.where(public: false, user_id: @teacher.id).map(&:id)) end - it "includes all of team members' non-public exercises" do - expect(scope.map(&:id)).to include(*Exercise.where(public: false, team_id: @teacher.teams.first.id).map(&:id)) - end - it "does not include other authors' non-public exercises" do - expect(scope.map(&:id)).not_to include(*Exercise.where(public: false).where("team_id <> #{@team.id} AND user_id <> #{@teacher.id}").map(&:id)) + expect(scope.map(&:id)).not_to include(*Exercise.where(public: false).where(user_id <> #{@teacher.id}").map(&:id)) end end end diff --git a/spec/policies/team_policy_spec.rb b/spec/policies/team_policy_spec.rb deleted file mode 100644 index aa3ba1d8..00000000 --- a/spec/policies/team_policy_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -require 'rails_helper' - -describe TeamPolicy do - subject { described_class } - - let(:team) { FactoryGirl.build(:team) } - - [:create?, :index?, :new?].each do |action| - permissions(action) do - it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), team) - end - - it 'grants access to teachers' do - expect(subject).to permit(FactoryGirl.build(:teacher), team) - end - - it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), team) - end - end - end - - [:destroy?, :edit?, :show?, :update?].each do |action| - permissions(action) do - it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), team) - end - - it 'grants access to members' do - expect(subject).to permit(team.members.last, team) - end - - it 'does not grant access to all other users' do - [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), team) - end - end - end - end -end From 34fc1c450486ec2c786e6054c082d47eb30bb69b Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 4 Jul 2016 17:36:26 +0200 Subject: [PATCH 09/17] On exit, augment the stacktrace and link it to the editors --- app/assets/javascripts/editor.js.erb | 60 +++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/editor.js.erb b/app/assets/javascripts/editor.js.erb index 1e64195e..51d61031 100644 --- a/app/assets/javascripts/editor.js.erb +++ b/app/assets/javascripts/editor.js.erb @@ -19,6 +19,15 @@ $(function() { var SERVER_SEND_EVENT = 2; var editors = []; + var editor_for_file = new Map(); + + var tracepositions_regexes = { + python: /File "(.+?)", line (\d+)/g, + java: /(.*\.java):(\d):/g + }; + + var tracepositions_regex; + var active_file = undefined; var active_frame = undefined; var running = false; @@ -404,12 +413,23 @@ $(function() { editor.setTheme(THEME); editor.commands.bindKey("ctrl+alt+0", null); editors.push(editor); + editor_for_file.set($(element).parent().data('filename'), editor); var session = editor.getSession(); session.setMode($(element).data('mode')); session.setTabSize($(element).data('indent-size')); session.setUseSoftTabs(true); session.setUseWrapMode(true); + // set regex for parsing error traces based on the mode of the main file. + if( $(element).parent().data('role') == "main_file"){ + var mode = $(element).data('mode'); + if(mode == "ace/mode/python"){ + tracepositions_regex = tracepositions_regexes.python; + } else if (mode = "ace/mode/java"){ + tracepositions_regex = tracepositions_regexes.java; + } + } + var file_id = $(element).data('id'); /* @@ -876,7 +896,9 @@ $(function() { } var showWorkspaceTab = function(event) { - event.preventDefault(); + if(event){ + event.preventDefault(); + } showTab(0); }; @@ -1036,6 +1058,7 @@ $(function() { case 'exit': killWebsocketAndContainer(); handleStderrOutputForFlowr(); + augmentStacktraceInOutput(); break; case 'timeout': // just show the timeout message here. Another exit command is sent by the rails backend when the socket to the docker container closes. @@ -1047,6 +1070,41 @@ $(function() { } }; + + var jumpToSourceLine = function(event){ + var file = $(event.target).data('file'); + var line = $(event.target).data('line'); + + showWorkspaceTab(null); + // set active file ?!?! + + var frame = $('div.frame[data-filename="' + file + '"]'); + showFrame(frame); + + var editor = editor_for_file.get(file); + editor.gotoLine(line, 0); + + }; + + var augmentStacktraceInOutput = function() { + if(tracepositions_regex){ + var element = $('#output>pre'); + var text = element.text(); + element.on( "click", "a", jumpToSourceLine); + + var matches; + + while(matches = tracepositions_regex.exec(text)){ + var frame = $('div.frame[data-filename="' + matches[1] + '"]') + + if(frame.length > 0){ + element.html(text.replace(matches[0], "" + matches[0] + "")); + } + } + } + + }; + var renderWebsocketOutput = function(msg){ var element = findOrCreateRenderElement(0); element.append(msg.data); From b962400fab8cd9dffdaf13d9306f7d9dc91a0b4f Mon Sep 17 00:00:00 2001 From: yqbk Date: Mon, 4 Jul 2016 17:40:01 +0200 Subject: [PATCH 10/17] further delete (db queries) --- app/policies/exercise_policy.rb | 3 +-- db/schema.rb | 15 --------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index d30c4aff..114e7d6e 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -25,8 +25,7 @@ class ExercisePolicy < AdminOrAuthorPolicy if @user.admin? @scope.all elsif @user.internal_user? - #need to remove team query - @scope.where('user_id = ? OR public = TRUE OR (team_id IS NOT NULL AND team_id IN (SELECT t.id FROM teams t JOIN internal_users_teams iut ON t.id = iut.team_id WHERE iut.internal_user_id = ?))', @user.id, @user.id) + @scope.where('user_id = ? OR public = TRUE', @user.id, @user.id) else @scope.none end diff --git a/db/schema.rb b/db/schema.rb index 67b3b35c..2f50d81f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -87,7 +87,6 @@ ActiveRecord::Schema.define(version: 20160624130951) do t.boolean "public" t.string "user_type" t.string "token" - t.integer "team_id" t.boolean "hide_file_tree" t.boolean "allow_file_creation" end @@ -173,14 +172,6 @@ ActiveRecord::Schema.define(version: 20160624130951) do add_index "internal_users", ["remember_me_token"], name: "index_internal_users_on_remember_me_token", using: :btree add_index "internal_users", ["reset_password_token"], name: "index_internal_users_on_reset_password_token", using: :btree - create_table "internal_users_teams", force: true do |t| - t.integer "internal_user_id" - t.integer "team_id" - end - - add_index "internal_users_teams", ["internal_user_id"], name: "index_internal_users_teams_on_internal_user_id", using: :btree - add_index "internal_users_teams", ["team_id"], name: "index_internal_users_teams_on_team_id", using: :btree - create_table "request_for_comments", force: true do |t| t.integer "user_id", null: false t.integer "exercise_id", null: false @@ -203,12 +194,6 @@ ActiveRecord::Schema.define(version: 20160624130951) do t.string "user_type" end - create_table "teams", force: true do |t| - t.string "name" - t.datetime "created_at" - t.datetime "updated_at" - end - create_table "testruns", force: true do |t| t.boolean "passed" t.text "output" From eb1fc76a32c15d2e95aaf688b2946fd1427c530e Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 4 Jul 2016 17:47:49 +0200 Subject: [PATCH 11/17] removed flowr hints for normal stdout --- app/assets/javascripts/editor.js.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/editor.js.erb b/app/assets/javascripts/editor.js.erb index 1e64195e..48cfaa62 100644 --- a/app/assets/javascripts/editor.js.erb +++ b/app/assets/javascripts/editor.js.erb @@ -587,7 +587,7 @@ $(function() { } else if (output.stdout) { //if (output_mode_is_streaming){ element.addClass('text-success').append(output.stdout); - flowrOutputBuffer += output.stdout; + // flowrOutputBuffer += output.stdout; //}else{ // element.addClass('text-success'); // element.data('content_buffer' , element.data('content_buffer') + output.stdout); From 4651c87491b29d327443306cc490a264f8bb255a Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 4 Jul 2016 17:48:52 +0200 Subject: [PATCH 12/17] Testrun output is now shown when hovering over the status icons (round red or green circles) --- app/views/exercises/external_users/statistics.html.slim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/exercises/external_users/statistics.html.slim b/app/views/exercises/external_users/statistics.html.slim index 56bd614b..3c755be6 100644 --- a/app/views/exercises/external_users/statistics.html.slim +++ b/app/views/exercises/external_users/statistics.html.slim @@ -50,9 +50,9 @@ h1 = "#{@exercise} (external user #{@external_user})" td -submission.testruns.each do |run| - if run.passed - .unit-test-result.positive-result + .unit-test-result.positive-result title=run.output - else - .unit-test-result.negative-result + .unit-test-result.negative-result title=run.output td = Time.at(deltas[1..index].inject(:+)).utc.strftime("%H:%M:%S") if index > 0 -working_times_until.push((Time.at(deltas[1..index].inject(:+)).utc.strftime("%H:%M:%S") if index > 0)) p = t('.addendum') From 16206cf58f019f963f9b7df766204d366bd5d1be Mon Sep 17 00:00:00 2001 From: yqbk Date: Mon, 4 Jul 2016 17:56:57 +0200 Subject: [PATCH 13/17] restore one line --- db/schema.rb | 2 +- spec/policies/exercise_policy_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 2f50d81f..d304b1d5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160624130951) do +ActiveRecord::Schema.define(version: 20160704143402) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index 1b25fbca..7b5aeabf 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe ExercisePolicy do subject { described_class } +let(:exercise) { FactoryGirl.build(:dummy) } + permissions :batch_update? do it 'grants access to admins only' do expect(subject).to permit(FactoryGirl.build(:admin), exercise) From 4bb8c79150a7e5e0cf7ca14beba0bacdd2b8e629 Mon Sep 17 00:00:00 2001 From: yqbk Date: Tue, 5 Jul 2016 15:27:49 +0200 Subject: [PATCH 14/17] typo in submissionS --- app/models/exercise.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 24570dc6..5b7ff9b2 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -10,7 +10,7 @@ class Exercise < ActiveRecord::Base after_initialize :set_default_values belongs_to :execution_environment - has_many :submission + has_many :submissions has_many :external_users, source: :user, source_type: ExternalUser, through: :submissions has_many :internal_users, source: :user, source_type: InternalUser, through: :submissions From 605042d2cfc43c395d07685a39d0fa7a8bd19129 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Thu, 7 Jul 2016 14:27:00 +0200 Subject: [PATCH 15/17] cleanup of code for linking stacktraces as suggested by tom. --- app/assets/javascripts/editor.js.erb | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/editor.js.erb b/app/assets/javascripts/editor.js.erb index 2dc790e0..f9900eec 100644 --- a/app/assets/javascripts/editor.js.erb +++ b/app/assets/javascripts/editor.js.erb @@ -20,12 +20,7 @@ $(function() { var editors = []; var editor_for_file = new Map(); - - var tracepositions_regexes = { - python: /File "(.+?)", line (\d+)/g, - java: /(.*\.java):(\d):/g - }; - + var regex_for_language = new Map(); var tracepositions_regex; var active_file = undefined; @@ -422,12 +417,7 @@ $(function() { // set regex for parsing error traces based on the mode of the main file. if( $(element).parent().data('role') == "main_file"){ - var mode = $(element).data('mode'); - if(mode == "ace/mode/python"){ - tracepositions_regex = tracepositions_regexes.python; - } else if (mode = "ace/mode/java"){ - tracepositions_regex = tracepositions_regexes.java; - } + tracepositions_regex = regex_for_language.get($(element).data('mode')); } var file_id = $(element).data('id'); @@ -477,6 +467,12 @@ $(function() { $('#request-for-comments').on('click', requestComments); }; + + var initializeRegexes = function(){ + regex_for_language.set("ace/mode/python", /File "(.+?)", line (\d+)/g); + regex_for_language.set("ace/mode/java", /(.*\.java):(\d):/g); + } + var initializeTooltips = function() { $('[data-tooltip]').tooltip(); }; @@ -1265,6 +1261,7 @@ $(function() { if ($('#editor').isPresent()) { if (isBrowserSupported()) { + initializeRegexes(); initializeCodePilot(); $('.score, #development-environment').show(); configureEditors(); From f80d1fc7fe5b33b6a00aa90b733bc0637011daea Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Thu, 7 Jul 2016 15:22:20 +0200 Subject: [PATCH 16/17] fixed regex for java... --- app/assets/javascripts/editor.js.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/editor.js.erb b/app/assets/javascripts/editor.js.erb index f9900eec..2a526aff 100644 --- a/app/assets/javascripts/editor.js.erb +++ b/app/assets/javascripts/editor.js.erb @@ -470,7 +470,7 @@ $(function() { var initializeRegexes = function(){ regex_for_language.set("ace/mode/python", /File "(.+?)", line (\d+)/g); - regex_for_language.set("ace/mode/java", /(.*\.java):(\d):/g); + regex_for_language.set("ace/mode/java", /(.*\.java):(\d+):/g); } var initializeTooltips = function() { From 3bf03a418bec70fd8eb59590a36de2f056a80489 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Sun, 10 Jul 2016 22:00:42 +0200 Subject: [PATCH 17/17] fix policy scope statement for internal users --- app/policies/exercise_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index 114e7d6e..55f7d16b 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -25,7 +25,7 @@ class ExercisePolicy < AdminOrAuthorPolicy if @user.admin? @scope.all elsif @user.internal_user? - @scope.where('user_id = ? OR public = TRUE', @user.id, @user.id) + @scope.where('user_id = ? OR public = TRUE', @user.id) else @scope.none end