From d6c64f5b91fafcd85d869d1935c1f681fc2c48b3 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Tue, 14 Aug 2018 16:59:58 +0200 Subject: [PATCH 1/9] Add event model --- app/models/event.rb | 5 +++++ db/migrate/20180814145059_create_events.rb | 12 ++++++++++++ db/schema.rb | 17 ++++++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 app/models/event.rb create mode 100644 db/migrate/20180814145059_create_events.rb diff --git a/app/models/event.rb b/app/models/event.rb new file mode 100644 index 00000000..bc4c41d9 --- /dev/null +++ b/app/models/event.rb @@ -0,0 +1,5 @@ +class Event < ActiveRecord::Base + belongs_to :user, polymorphic: true + belongs_to :exercise + belongs_to :file +end diff --git a/db/migrate/20180814145059_create_events.rb b/db/migrate/20180814145059_create_events.rb new file mode 100644 index 00000000..b6bc8745 --- /dev/null +++ b/db/migrate/20180814145059_create_events.rb @@ -0,0 +1,12 @@ +class CreateEvents < ActiveRecord::Migration + def change + create_table :events do |t| + t.string :type + t.string :data + t.belongs_to :user, polymorphic: true, index: true + t.belongs_to :exercise, index: true + t.belongs_to :file, index: true + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index ae1398a6..872e6b82 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: 20180703125302) do +ActiveRecord::Schema.define(version: 20180814145059) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -95,6 +95,21 @@ ActiveRecord::Schema.define(version: 20180703125302) do add_index "errors", ["submission_id"], name: "index_errors_on_submission_id", using: :btree + create_table "events", force: :cascade do |t| + t.string "type" + t.string "data" + t.integer "user_id" + t.string "user_type" + t.integer "exercise_id" + t.integer "file_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "events", ["exercise_id"], name: "index_events_on_exercise_id", using: :btree + add_index "events", ["file_id"], name: "index_events_on_file_id", using: :btree + add_index "events", ["user_type", "user_id"], name: "index_events_on_user_type_and_user_id", using: :btree + create_table "execution_environments", force: :cascade do |t| t.string "docker_image", limit: 255 t.string "name", limit: 255 From f1278a7f4874046742b0d713ea288fcfa8dde8ce Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Tue, 14 Aug 2018 17:55:17 +0200 Subject: [PATCH 2/9] Rename type column to category to avoid conflict with rails --- db/migrate/20180814154055_rename_events_type_to_category.rb | 5 +++++ db/schema.rb | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20180814154055_rename_events_type_to_category.rb diff --git a/db/migrate/20180814154055_rename_events_type_to_category.rb b/db/migrate/20180814154055_rename_events_type_to_category.rb new file mode 100644 index 00000000..f97200c8 --- /dev/null +++ b/db/migrate/20180814154055_rename_events_type_to_category.rb @@ -0,0 +1,5 @@ +class RenameEventsTypeToCategory < ActiveRecord::Migration + def change + rename_column :events, :type, :category + end +end diff --git a/db/schema.rb b/db/schema.rb index 872e6b82..bfa3ad66 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: 20180814145059) do +ActiveRecord::Schema.define(version: 20180814154055) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -96,7 +96,7 @@ ActiveRecord::Schema.define(version: 20180814145059) do add_index "errors", ["submission_id"], name: "index_errors_on_submission_id", using: :btree create_table "events", force: :cascade do |t| - t.string "type" + t.string "category" t.string "data" t.integer "user_id" t.string "user_type" From 7d7234ce63d6a984bcedcccc11b4f18efc0e3d59 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Tue, 14 Aug 2018 18:08:25 +0200 Subject: [PATCH 3/9] Add simple controller to create events --- app/controllers/events_controller.rb | 28 ++++++++++++++++++++++++++++ app/policies/event_policy.rb | 7 +++++++ config/routes.rb | 2 ++ 3 files changed, 37 insertions(+) create mode 100644 app/controllers/events_controller.rb create mode 100644 app/policies/event_policy.rb diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb new file mode 100644 index 00000000..38e57cd5 --- /dev/null +++ b/app/controllers/events_controller.rb @@ -0,0 +1,28 @@ +class EventsController < ApplicationController + + def authorize! + authorize(@event || @events) + end + private :authorize! + + def create + @event = Event.new(event_params) + authorize! + respond_to do |format| + if @event.save + format.html { head :created } + format.json { head :created } + else + format.html { head :unprocessable_entity } + format.json { head :unprocessable_entity } + end + end + end + + def event_params + params[:event].permit(:category, :data, :exercise_id, :file_id) + .merge(user_id: current_user&.id, user_type: current_user&.class.name) + end + private :event_params + +end diff --git a/app/policies/event_policy.rb b/app/policies/event_policy.rb new file mode 100644 index 00000000..a648b901 --- /dev/null +++ b/app/policies/event_policy.rb @@ -0,0 +1,7 @@ +class EventPolicy < AdminOnlyPolicy + + def create? + everyone + end + +end diff --git a/config/routes.rb b/config/routes.rb index 6703d8c1..7c1f317b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -168,6 +168,8 @@ Rails.application.routes.draw do end end + resources :events, only: [:create] + post "/evaluate", to: 'remote_evaluation#evaluate', via: [:post] end From 369b0c890831926bc3bb99dfc44a0eee06c0e2e2 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Tue, 14 Aug 2018 21:28:06 +0200 Subject: [PATCH 4/9] Send paste events to CodeOcean events API instead of LearningAnalytics --- app/assets/javascripts/editor/editor.js.erb | 53 +++++++-------------- 1 file changed, 17 insertions(+), 36 deletions(-) diff --git a/app/assets/javascripts/editor/editor.js.erb b/app/assets/javascripts/editor/editor.js.erb index aa7fee59..2a9d7fa7 100644 --- a/app/assets/javascripts/editor/editor.js.erb +++ b/app/assets/javascripts/editor/editor.js.erb @@ -29,9 +29,10 @@ var CodeOceanEditor = { lastCopyText: null, + <% self.class.include Rails.application.routes.url_helpers %> <% @config ||= CodeOcean::Config.new(:code_ocean).read(erb: false) %> - sendLearningAnalyticEvents: <%= @config['codeocean_events'] ? @config['codeocean_events']['enabled'] : false %>, - learningAnalyticsURL: "<%= @config['codeocean_events'] ? @config['codeocean_events']['url'] : "" %>", + sendEvents: <%= @config['codeocean_events'] ? @config['codeocean_events']['enabled'] : false %>, + eventURL: "<%= @config['codeocean_events'] ? events_path : '' %>", configureEditors: function () { @@ -151,12 +152,12 @@ configureEditors: function () { handlePasteEvent: function (pasteObject) { var same = (this.lastCopyText === pasteObject.text); - // if the text is not copied from within the editor (from any file), send an event to lanalytics + // if the text is not copied from within the editor (from any file), send an event to the backend if (!same) { - this.publishCodeOceanEvent("codeocean_editor_paste", { - text: pasteObject.text, - codeocean_user_id: $('#editor').data('user-id'), - exercise: $('#editor').data('exercise-id'), + this.publishCodeOceanEvent({ + category: 'editor_paste', + data: pasteObject.text, + exercise_id: $('#editor').data('exercise-id'), file_id: "1" }); } @@ -381,38 +382,18 @@ configureEditors: function () { //panel.find('.row .col-sm-9').eq(4).find('a').attr('href', '#output-' + index); }, - // move URL to config file, only fire event if desired. - publishCodeOceanEvent: function (eventName, contextData) { - if(this.sendLearningAnalyticEvents){ - - // enhance contextData hash with the user agent - contextData['user_agent'] = navigator.userAgent; - - var payload = { - user: { - uuid: $('#editor').data('user-external-id') - }, - verb: { - type: eventName - }, - resource: { - type: 'page', - uuid: document.location.href - }, - timestamp: new Date().toISOString(), - with_result: {}, - in_context: contextData - }; - - $.ajax(this.learningAnalyticsURL, { + publishCodeOceanEvent: function (payload) { + if(this.sendEvents){ + $.ajax(this.eventURL, { type: 'POST', cache: false, dataType: 'JSON', - data: payload, - success: {}, - error: {} - }) - + data: { + event: payload + }, + success: _.noop, + error: _.noop + }); } }, From f73de94f4d4c3de5b876068834e4320df9e191e1 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Tue, 14 Aug 2018 21:36:24 +0200 Subject: [PATCH 5/9] Allow file_id to be captured --- app/assets/javascripts/editor/editor.js.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/editor/editor.js.erb b/app/assets/javascripts/editor/editor.js.erb index 2a9d7fa7..190f84e0 100644 --- a/app/assets/javascripts/editor/editor.js.erb +++ b/app/assets/javascripts/editor/editor.js.erb @@ -158,7 +158,7 @@ configureEditors: function () { category: 'editor_paste', data: pasteObject.text, exercise_id: $('#editor').data('exercise-id'), - file_id: "1" + file_id: $(this).data('file-id') }); } }, @@ -248,8 +248,8 @@ configureEditors: function () { */ // editor itself - editor.on("paste", this.handlePasteEvent.bind(this)); - editor.on("copy", this.handleCopyEvent.bind(this)); + editor.on("paste", this.handlePasteEvent.bind(element)); + editor.on("copy", this.handleCopyEvent.bind(element)); // listener for autosave session.on("change", function (deltaObject) { From 16533e2d89a507b3ad85ea780d744689e112386c Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Tue, 14 Aug 2018 21:48:16 +0200 Subject: [PATCH 6/9] Work around messy architecture --- app/assets/javascripts/editor/editor.js.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/editor/editor.js.erb b/app/assets/javascripts/editor/editor.js.erb index 190f84e0..1e254dd3 100644 --- a/app/assets/javascripts/editor/editor.js.erb +++ b/app/assets/javascripts/editor/editor.js.erb @@ -150,11 +150,11 @@ configureEditors: function () { }, handlePasteEvent: function (pasteObject) { - var same = (this.lastCopyText === pasteObject.text); + var same = (CodeOceanEditor.lastCopyText === pasteObject.text); // if the text is not copied from within the editor (from any file), send an event to the backend if (!same) { - this.publishCodeOceanEvent({ + CodeOceanEditor.publishCodeOceanEvent({ category: 'editor_paste', data: pasteObject.text, exercise_id: $('#editor').data('exercise-id'), From 6bf14a0e5031adeb42a5f7f152f7356e47805724 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 15 Aug 2018 14:17:26 +0200 Subject: [PATCH 7/9] Remove indices --- db/migrate/20180815115351_remove_event_indices.rb | 7 +++++++ db/schema.rb | 6 +----- 2 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20180815115351_remove_event_indices.rb diff --git a/db/migrate/20180815115351_remove_event_indices.rb b/db/migrate/20180815115351_remove_event_indices.rb new file mode 100644 index 00000000..2be20c1d --- /dev/null +++ b/db/migrate/20180815115351_remove_event_indices.rb @@ -0,0 +1,7 @@ +class RemoveEventIndices < ActiveRecord::Migration + def change + remove_index :events, [:user_type, :user_id] + remove_index :events, :exercise_id + remove_index :events, :file_id + end +end diff --git a/db/schema.rb b/db/schema.rb index bfa3ad66..2646f080 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: 20180814154055) do +ActiveRecord::Schema.define(version: 20180815115351) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -106,10 +106,6 @@ ActiveRecord::Schema.define(version: 20180814154055) do t.datetime "updated_at", null: false end - add_index "events", ["exercise_id"], name: "index_events_on_exercise_id", using: :btree - add_index "events", ["file_id"], name: "index_events_on_file_id", using: :btree - add_index "events", ["user_type", "user_id"], name: "index_events_on_user_type_and_user_id", using: :btree - create_table "execution_environments", force: :cascade do |t| t.string "docker_image", limit: 255 t.string "name", limit: 255 From d528baceadd8270eb09453186d9619017c4ff482 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 15 Aug 2018 14:39:58 +0200 Subject: [PATCH 8/9] Validate presence of category and data attribute --- app/models/event.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models/event.rb b/app/models/event.rb index bc4c41d9..325fecb8 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -2,4 +2,7 @@ class Event < ActiveRecord::Base belongs_to :user, polymorphic: true belongs_to :exercise belongs_to :file + + validates :category, presence: true + validates :data, presence: true end From 2c9f1a29175694af84f0a54444cf280b5857a851 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 15 Aug 2018 14:40:59 +0200 Subject: [PATCH 9/9] Add tests for events controller --- app/controllers/events_controller.rb | 4 +-- spec/controllers/events_controller_spec.rb | 33 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 spec/controllers/events_controller_spec.rb diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 38e57cd5..3a3b02a4 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -20,8 +20,8 @@ class EventsController < ApplicationController end def event_params - params[:event].permit(:category, :data, :exercise_id, :file_id) - .merge(user_id: current_user&.id, user_type: current_user&.class.name) + params[:event]&.permit(:category, :data, :exercise_id, :file_id) + &.merge(user_id: current_user&.id, user_type: current_user&.class.name) end private :event_params diff --git a/spec/controllers/events_controller_spec.rb b/spec/controllers/events_controller_spec.rb new file mode 100644 index 00000000..bd9e11a8 --- /dev/null +++ b/spec/controllers/events_controller_spec.rb @@ -0,0 +1,33 @@ +require 'rails_helper' + +describe EventsController do + let(:user) { FactoryBot.create(:admin) } + let(:exercise) {FactoryBot.create(:fibonacci)} + before(:each) { allow(controller).to receive(:current_user).and_return(user) } + + describe 'POST #create' do + context 'with a valid event' do + let(:request) { proc { post :create, event: {category: 'foo', data: 'bar', exercise_id: exercise.id, file_id: exercise.files[0].id} } } + before(:each) { request.call } + + expect_assigns(event: Event) + + it 'creates the Event' do + expect { request.call }.to change(Event, :count).by(1) + end + + expect_status(201) + end + + context 'with an invalid event' do + before(:each) { post :create, event: {exercise_id: 847482} } + expect_assigns(event: Event) + expect_status(422) + end + + context 'with no event' do + before(:each) { post :create } + expect_status(422) + end + end +end