From b6ff7ede99a90f4dfc0b338b5c6d9f28bee9d320 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 28 Nov 2018 17:21:50 +0100 Subject: [PATCH 1/6] Rebase `repair-charts` onto `master` --- app/assets/javascripts/working_time_graphs.js | 186 ++++++------------ app/views/exercises/statistics.html.slim | 2 +- 2 files changed, 62 insertions(+), 126 deletions(-) diff --git a/app/assets/javascripts/working_time_graphs.js b/app/assets/javascripts/working_time_graphs.js index 69b02e9f..c0ef0bbd 100644 --- a/app/assets/javascripts/working_time_graphs.js +++ b/app/assets/javascripts/working_time_graphs.js @@ -1,18 +1,16 @@ $(document).on('turbolinks:load', function() { - // http://localhost:3333/exercises/38/statistics good for testing - // originally at--> localhost:3333/exercises/69/statistics + // /38/statistics good for testing - if ($.isController('exercises') && $('.graph-functions').isPresent()) { + if ($.isController('exercises') && $('.working-time-graphs').isPresent()) { var working_times = $('#data').data('working-time'); - function get_minutes (time_stamp){ + function get_minutes (timestamp){ try{ - hours = time_stamp.split(":")[0]; - minutes = time_stamp.split(":")[1]; - seconds = time_stamp.split(":")[2]; + hours = timestamp.split(":")[0]; + minutes = timestamp.split(":")[1]; + seconds = timestamp.split(":")[2]; - minutes = parseFloat(hours * 60) + parseInt(minutes); - return minutes + return parseFloat(hours * 60) + parseInt(minutes); } catch (err){ return 0; } @@ -20,7 +18,7 @@ $(document).on('turbolinks:load', function() { } // GET ALL THE DATA ------------------------------------------------------------------------------ - minutes_array = _.map(working_times,function(item){return get_minutes(item)}); + minutes_array = _.map(working_times, function(item){return get_minutes(item)}); minutes_array_length = minutes_array.length; maximum_minutes = _.max(minutes_array); @@ -39,7 +37,7 @@ $(document).on('turbolinks:load', function() { } function getWidth() { - if (self.innerHeight) { + if (self.innerWidth) { return self.innerWidth; } @@ -53,15 +51,12 @@ $(document).on('turbolinks:load', function() { } // DRAW THE LINE GRAPH ------------------------------------------------------------------------------ - function draw_line_graph() { + function drawLineGraph() { var width_ratio = .8; - if (getWidth()*width_ratio > 1000){ - width_ratio = 1000/getWidth(); + if (getWidth() * width_ratio > 1000){ + width_ratio = 1000 / getWidth(); } - var height_ratio = .7; // percent of height - - // currently sets as percentage of window width, however, unfortunately - // is not yet responsive + var height_ratio = .7; var margin = {top: 100, right: 20, bottom: 70, left: 70},//30,50 width = (getWidth() * width_ratio) - margin.left - margin.right, @@ -139,96 +134,66 @@ $(document).on('turbolinks:load', function() { svg.append("path") .datum(minutes_count) .attr("class", "line") - .attr('id', 'myPath')// new + .attr('id', 'myPath') .attr("stroke", "orange") .attr("stroke-width", 5) - .attr("fill", "none")// end new - .attr("d", line);//--- - //.on("mousemove", mMove)//new again - //.append("title"); - - // function type(d) { - // d.frequency = +d.frequency; - // return d; - // } + .attr("fill", "none") + .attr("d", line); } - draw_line_graph(); - - // THIS SHOULD DISPLAY THE X AND Y VALUES BUT - // THE RESULTS ARE WRONG AT THE END FOR SOME REASON - - //function mMove() { - // var x_width = getWidth() * width_ratio; - // //var x_value = m[0]*(minutes_count.length/x_width); - // - // var y_height = x_width * height_ratio; - // //var y_value = (((y_height - m[1])/y_height)*100); - // - // //console.log('y is: ' + y_value); - // var m = d3.mouse(this); - // d3.select("#myPath").select("title") - // .text((y_height-m[1])/(y_height) * 100 + "% of Students" +"\n"+ - // (m[0]*(minutes_count.length/x_width)) +" Minutes");//text(m[1]); - //} + drawLineGraph(); // DRAW THE SECOND GRAPH ------------------------------------------------------------------------------ - function draw_bar_graph() { - var number_of_bars = 40; - var group_increment = Math.ceil(maximum_minutes / number_of_bars); // range in minutes - var group_ranges = group_increment; // just for the start - var minutes_array_for_bar = []; + function drawBarGraph() { + var groupWidth = 5; + var groupRanges = 0; + var workingTimeGroups = []; do { - var section_value = 0; + var clusterCount = 0; for (var i = 0; i < minutes_array.length; i++) { - if ((minutes_array[i] < group_ranges) && (minutes_array[i] >= (group_ranges - group_increment))) { - section_value++; + if ((minutes_array[i] > groupRanges) && (minutes_array[i] < (groupRanges + groupWidth))) { + clusterCount++; } } - minutes_array_for_bar.push(section_value); - group_ranges += group_increment; + workingTimeGroups.push(clusterCount); + groupRanges += groupWidth; } - while (group_ranges < maximum_minutes + group_increment); - - //console.log(minutes_array_for_bar); // this var used as the bars - //minutes_array_for_bar = [39, 20, 28, 20, 39, 34, 26, 23, 16, 8]; - - var max_of_array = Math.max.apply(Math, minutes_array_for_bar); - var min_of_array = Math.min.apply(Math, minutes_array_for_bar); + while (groupRanges < maximum_minutes); + var maxStudentsInGroup = Math.max.apply(Math, workingTimeGroups); var width_ratio = .8; - if (getWidth()*width_ratio > 1000){ - width_ratio = 1000/getWidth(); - } - var height_ratio = .7; // percent of height + var height_ratio = .7; - var margin = {top: 100, right: 20, bottom: 70, left: 70},//30,50 + var margin = {top: 100, right: 20, bottom: 70, left: 70}, width = (getWidth() * width_ratio) - margin.left - margin.right, height = (width * height_ratio) - margin.top - margin.bottom; var x = d3.scaleBand() - .range([0, width], .1); - - var y = d3.scaleLinear() - .range([0,height-(margin.top + margin.bottom)]); - + .rangeRound([0, width]) + .paddingInner(0.1) + .domain(workingTimeGroups.map(function (d, i) { + return i * groupWidth; + })); var xAxis = d3.axisBottom(x) + .tickValues(x.domain().filter(function(d, i){ + return (d % 50) === 0 + })); + + var y = d3.scaleLinear() + .domain([0, maxStudentsInGroup]) + .range([height, 0]); + + var yAxis = d3.axisLeft(y) .ticks(10); - - var yAxis = d3 - .axisLeft(d3.scaleLinear().domain([0,max_of_array]).range([height,0]))//y - .ticks(10) - .tickSizeInner(-width); - var tip = d3.tip() .attr('class', 'd3-tip') .offset([-10, 0]) .html(function(d) { - return "Students: " + d + ""; + return "Students:" + d + ""; }); var svg = d3.select("#chart_2").append("svg") @@ -239,40 +204,16 @@ $(document).on('turbolinks:load', function() { svg.call(tip); - x.domain(minutes_array_for_bar.map(function (d, i) { - i++; - var high_side = i * group_increment; - var low_side = high_side - group_increment; - return (low_side+"-"+high_side); - })); - - y.domain(minutes_array_for_bar.map(function (d) { - return (d); - })); - svg.append("g") .attr("class", "x axis") .attr("transform", "translate(0," + height + ")") - .call(xAxis) - .selectAll("text") - .style("text-anchor", "end") - .attr("dx", "-.8em") - .attr("dy", ".15em") - .attr("transform", function(d) { - return "rotate(-45)" - }); + .call(xAxis); svg.append("g") .attr("class", "y axis") - .call(yAxis) - .append("text") - .attr("transform", "rotate(-90)") - .attr("y", 6) - .attr("dy", ".71em"); - //.style("text-anchor", "end") - //.text("Students"); + .call(yAxis); - svg.append("text") // y axis label + svg.append("text") .attr("transform", "rotate(-90)") .attr("x", -height / 2) .attr("dy", "-3em") @@ -280,45 +221,40 @@ $(document).on('turbolinks:load', function() { .text("Students") .style('font-size', 14); - svg.append("text")// x axis label + svg.append("text") .attr("class", "x axis") .attr("text-anchor", "middle") .attr("x", width / 2) .attr("y", height) - .attr("dy", ((height / 20) + 40) + 'px') + .attr("dy", ((height / 20) + 20) + 'px') .text("Working Time (Minutes)") .style('font-size', 14); - y = d3.scaleLinear() - .domain([(0),max_of_array]) - .range([0,height]); - - svg.selectAll(".bar") - .data(minutes_array_for_bar) + .data(workingTimeGroups) .enter().append("rect") .attr("class", "bar") - .attr("x", function(d,i) { var bar_increment = width / minutes_array_for_bar.length; - var bar_x = i * bar_increment; - return (bar_x)}) + .attr("x", function(d, i) { + return x(i * groupWidth); + }) .attr("width", x.bandwidth()) - .attr("y", function(d) { return height - y(d); }) - .attr("height", function(d) { return y(d); }) - .on('mouseover', tip.show) + .attr("y", function(d) { return y(d); }) + .attr("height", function(d) { return height - y(d); }) + .on('mouseenter', tip.show) .on('mouseout', tip.hide); - svg.append("text")// Title + svg.append("text") .attr("class", "x axis") .attr("text-anchor", "middle") - .attr("x", (width / 2))//+300) + .attr("x", (width / 2)) .attr("y", 0) .attr("dy", '-1.5em') .text("Distribution of Time Spent by Students") .style('font-size', 20) .style('text-decoration', 'underline'); - } - draw_bar_graph(); + + drawBarGraph(); } }); diff --git a/app/views/exercises/statistics.html.slim b/app/views/exercises/statistics.html.slim index 5455162b..2fdaa467 100644 --- a/app/views/exercises/statistics.html.slim +++ b/app/views/exercises/statistics.html.slim @@ -33,7 +33,7 @@ h1 = @exercise -working_time_array.push working_time hr .d-none#data data-working-time=ActiveSupport::JSON.encode(working_time_array) - .graph-functions + .working-time-graphs div#chart_1 hr div#chart_2 From 95503c7b7d2e50dc157dd6d5bc4768e069351e04 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Sun, 2 Dec 2018 15:09:39 +0100 Subject: [PATCH 2/6] Include some improvements from current master --- app/assets/javascripts/working_time_graphs.js | 50 ++++++++++++++++--- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/working_time_graphs.js b/app/assets/javascripts/working_time_graphs.js index c0ef0bbd..10ec05b1 100644 --- a/app/assets/javascripts/working_time_graphs.js +++ b/app/assets/javascripts/working_time_graphs.js @@ -152,7 +152,7 @@ $(document).on('turbolinks:load', function() { do { var clusterCount = 0; for (var i = 0; i < minutes_array.length; i++) { - if ((minutes_array[i] > groupRanges) && (minutes_array[i] < (groupRanges + groupWidth))) { + if ((minutes_array[i] >= groupRanges) && (minutes_array[i] < (groupRanges + groupWidth))) { clusterCount++; } } @@ -160,10 +160,33 @@ $(document).on('turbolinks:load', function() { groupRanges += groupWidth; } while (groupRanges < maximum_minutes); + console.log(maximum_minutes); + + var clusterCount = 0, + sum = 0, + maxVal = 0; + for (var i = 0; i < minutes_array.length; i++) { + if (minutes_array[i] > maximum_minutes) { + currentValue = minutes_array[i]; + sum += currentValue; + if (currentValue > maxVal) { + maxVal = currentValue; + } + clusterCount++; + } + } + // ToDo: Take care of x axis description if this is added + // workingTimeGroups.push(clusterCount); var maxStudentsInGroup = Math.max.apply(Math, workingTimeGroups); var width_ratio = .8; + + // Scale width to fit into bootsrap container + if (getWidth() * width_ratio > 1000){ + width_ratio = 1000 / getWidth(); + } + var height_ratio = .7; var margin = {top: 100, right: 20, bottom: 70, left: 70}, @@ -178,9 +201,11 @@ $(document).on('turbolinks:load', function() { })); var xAxis = d3.axisBottom(x) + .ticks(10) .tickValues(x.domain().filter(function(d, i){ - return (d % 50) === 0 - })); + return (d % 10) === 0 + })) + .tickFormat(function(d) { return d + "-" + (d + groupWidth) }); var y = d3.scaleLinear() .domain([0, maxStudentsInGroup]) @@ -193,7 +218,7 @@ $(document).on('turbolinks:load', function() { .attr('class', 'd3-tip') .offset([-10, 0]) .html(function(d) { - return "Students:" + d + ""; + return "Students: " + d + ""; }); var svg = d3.select("#chart_2").append("svg") @@ -207,11 +232,22 @@ $(document).on('turbolinks:load', function() { svg.append("g") .attr("class", "x axis") .attr("transform", "translate(0," + height + ")") - .call(xAxis); + .call(xAxis) + .selectAll("text") + .style("text-anchor", "end") + .attr("dx", "-.8em") + .attr("dy", ".15em") + .attr("transform", function(d) { + return "rotate(-45)" + }); svg.append("g") .attr("class", "y axis") - .call(yAxis); + .call(yAxis) + .append("text") + .attr("transform", "rotate(-90)") + .attr("y", 6) + .attr("dy", ".71em"); svg.append("text") .attr("transform", "rotate(-90)") @@ -226,7 +262,7 @@ $(document).on('turbolinks:load', function() { .attr("text-anchor", "middle") .attr("x", width / 2) .attr("y", height) - .attr("dy", ((height / 20) + 20) + 'px') + .attr("dy", ((height / 20) + 40) + 'px') .text("Working Time (Minutes)") .style('font-size', 14); From 141450a84056ac9466e7cdd360a9fd71481520b4 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Sun, 25 Nov 2018 20:22:43 +0100 Subject: [PATCH 3/6] Migrate User to abstract class --- app/models/concerns/user.rb | 32 -------------------------------- app/models/external_user.rb | 3 +-- app/models/internal_user.rb | 3 +-- app/models/user.rb | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 36 deletions(-) delete mode 100644 app/models/concerns/user.rb create mode 100644 app/models/user.rb diff --git a/app/models/concerns/user.rb b/app/models/concerns/user.rb deleted file mode 100644 index 78299717..00000000 --- a/app/models/concerns/user.rb +++ /dev/null @@ -1,32 +0,0 @@ -module User - extend ActiveSupport::Concern - - ROLES = %w(admin teacher) - - included do - belongs_to :consumer - has_many :exercises, as: :user - has_many :file_types, as: :user - has_many :submissions, as: :user - has_many :participations, through: :submissions, source: :exercise, as: :user - has_many :user_proxy_exercise_exercises, as: :user - has_many :user_exercise_interventions, as: :user - has_many :interventions, through: :user_exercise_interventions - accepts_nested_attributes_for :user_proxy_exercise_exercises - - - scope :with_submissions, -> { where('id IN (SELECT user_id FROM submissions)') } - end - - ROLES.each do |role| - define_method("#{role}?") { try(:role) == role } - end - - [ExternalUser, InternalUser].each do |klass| - define_method("#{klass.name.underscore}?") { is_a?(klass) } - end - - def to_s - displayname - end -end diff --git a/app/models/external_user.rb b/app/models/external_user.rb index b877f08d..29c52692 100644 --- a/app/models/external_user.rb +++ b/app/models/external_user.rb @@ -1,5 +1,4 @@ -class ExternalUser < ApplicationRecord - include User +class ExternalUser < User validates :consumer_id, presence: true validates :external_id, presence: true diff --git a/app/models/internal_user.rb b/app/models/internal_user.rb index e7adda6d..9cef2cb4 100644 --- a/app/models/internal_user.rb +++ b/app/models/internal_user.rb @@ -1,5 +1,4 @@ -class InternalUser < ApplicationRecord - include User +class InternalUser < User authenticates_with_sorcery! diff --git a/app/models/user.rb b/app/models/user.rb new file mode 100644 index 00000000..5c045101 --- /dev/null +++ b/app/models/user.rb @@ -0,0 +1,34 @@ +class User < ApplicationRecord + self.abstract_class = true + + ROLES = %w(admin teacher) + + belongs_to :consumer + has_many :exercises, as: :user + has_many :file_types, as: :user + has_many :submissions, as: :user + has_many :participations, through: :submissions, source: :exercise, as: :user + has_many :user_proxy_exercise_exercises, as: :user + has_many :user_exercise_interventions, as: :user + has_many :interventions, through: :user_exercise_interventions + accepts_nested_attributes_for :user_proxy_exercise_exercises + + + scope :with_submissions, -> { where('id IN (SELECT user_id FROM submissions)') } + + ROLES.each do |role| + define_method("#{role}?") { try(:role) == role } + end + + def internal_user? + is_a?(InternalUser) + end + + def external_user? + is_a?(ExternalUser) + end + + def to_s + displayname + end +end From b137e64020294415d4e0032242d794cf24c569a5 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Mon, 26 Nov 2018 17:06:35 +0100 Subject: [PATCH 4/6] Add StudyGroups with ExternalUsers only --- app/controllers/concerns/lti.rb | 12 +++++++++++ app/controllers/sessions_controller.rb | 2 +- app/models/external_user.rb | 8 ++++---- app/models/study_group.rb | 11 ++++++++++ app/models/study_group_membership.rb | 8 ++++++++ app/models/submission.rb | 1 + app/models/user.rb | 2 ++ .../20181122084546_create_study_groups.rb | 13 ++++++++++++ ...22090243_create_study_group_memberships.rb | 8 ++++++++ ...122090244_add_study_group_to_submission.rb | 5 +++++ db/schema.rb | 20 +++++++++++++++++++ spec/controllers/sessions_controller_spec.rb | 2 +- 12 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 app/models/study_group.rb create mode 100644 app/models/study_group_membership.rb create mode 100644 db/migrate/20181122084546_create_study_groups.rb create mode 100644 db/migrate/20181122090243_create_study_group_memberships.rb create mode 100644 db/migrate/20181122090244_add_study_group_to_submission.rb diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index e8241af3..155e8895 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -52,6 +52,11 @@ module Lti end private :external_user_name + def mooc_course + # All Xikolo platforms set the custom_course to the course code + params[:custom_course] + end + def refuse_lti_launch(options = {}) return_to_consumer(lti_errorlog: options[:message], lti_errormsg: t('sessions.oauth.failure')) end @@ -133,6 +138,13 @@ module Lti end private :set_current_user + def set_study_group_membership + return if mooc_course + group = StudyGroup.find_or_create_by(external_id: @provider.resource_link_id, consumer: @consumer) + group.users |= [@current_user] # add current user if not already member of the group + group.save + end + def store_lti_session_data(options = {}) lti_parameters = LtiParameter.find_or_create_by(consumers_id: options[:consumer].id, external_users_id: @current_user.id, diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 8f698d1a..24b2e6b1 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,7 +1,7 @@ class SessionsController < ApplicationController include Lti - [:require_oauth_parameters, :require_valid_consumer_key, :require_valid_oauth_signature, :require_unique_oauth_nonce, :set_current_user, :require_valid_exercise_token].each do |method_name| + [:require_oauth_parameters, :require_valid_consumer_key, :require_valid_oauth_signature, :require_unique_oauth_nonce, :set_current_user, :set_study_group_membership, :require_valid_exercise_token].each do |method_name| before_action(method_name, only: :create_through_lti) end diff --git a/app/models/external_user.rb b/app/models/external_user.rb index 29c52692..053d3795 100644 --- a/app/models/external_user.rb +++ b/app/models/external_user.rb @@ -4,11 +4,11 @@ class ExternalUser < User validates :external_id, presence: true def displayname - result = name - if(result == nil || result == "") - result = "User " + id.to_s + if name.blank? + "User " + id.to_s + else + name end - result end end diff --git a/app/models/study_group.rb b/app/models/study_group.rb new file mode 100644 index 00000000..b86f4969 --- /dev/null +++ b/app/models/study_group.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class StudyGroup < ApplicationRecord + has_many :study_group_memberships + # Use `ExternalUser` as `source_type` for now. + # Using `User` will lead ActiveRecord to access the inexistent table `users`. + # Issue created: https://github.com/rails/rails/issues/34531 + has_many :users, through: :study_group_memberships, source_type: 'ExternalUser' + has_many :submissions + belongs_to :consumer +end diff --git a/app/models/study_group_membership.rb b/app/models/study_group_membership.rb new file mode 100644 index 00000000..1f7aad91 --- /dev/null +++ b/app/models/study_group_membership.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class StudyGroupMembership < ApplicationRecord + belongs_to :user, polymorphic: true + belongs_to :study_group + + validates_uniqueness_of :user_id, :scope => [:user_type, :study_group_id] +end diff --git a/app/models/submission.rb b/app/models/submission.rb index e9d3b235..fe765543 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -6,6 +6,7 @@ class Submission < ApplicationRecord FILENAME_URL_PLACEHOLDER = '{filename}' belongs_to :exercise + belongs_to :study_group, optional: true has_many :testruns has_many :structured_errors diff --git a/app/models/user.rb b/app/models/user.rb index 5c045101..a9c9dab2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,6 +4,8 @@ class User < ApplicationRecord ROLES = %w(admin teacher) belongs_to :consumer + has_many :study_group_memberships, as: :user + has_many :study_groups, through: :study_group_memberships, as: :user has_many :exercises, as: :user has_many :file_types, as: :user has_many :submissions, as: :user diff --git a/db/migrate/20181122084546_create_study_groups.rb b/db/migrate/20181122084546_create_study_groups.rb new file mode 100644 index 00000000..63919d75 --- /dev/null +++ b/db/migrate/20181122084546_create_study_groups.rb @@ -0,0 +1,13 @@ +class CreateStudyGroups < ActiveRecord::Migration[5.2] + def change + create_table :study_groups do |t| + t.string :name + t.string :external_id + t.belongs_to :consumer + t.timestamps + end + + add_index :study_groups, [:external_id, :consumer_id], unique: true + end +end + diff --git a/db/migrate/20181122090243_create_study_group_memberships.rb b/db/migrate/20181122090243_create_study_group_memberships.rb new file mode 100644 index 00000000..8cb34dd3 --- /dev/null +++ b/db/migrate/20181122090243_create_study_group_memberships.rb @@ -0,0 +1,8 @@ +class CreateStudyGroupMemberships < ActiveRecord::Migration[5.2] + def change + create_table :study_group_memberships do |t| + t.belongs_to :study_group + t.belongs_to :user, polymorphic: true + end + end +end diff --git a/db/migrate/20181122090244_add_study_group_to_submission.rb b/db/migrate/20181122090244_add_study_group_to_submission.rb new file mode 100644 index 00000000..94cd30d6 --- /dev/null +++ b/db/migrate/20181122090244_add_study_group_to_submission.rb @@ -0,0 +1,5 @@ +class AddStudyGroupToSubmission < ActiveRecord::Migration[5.2] + def change + add_reference :submissions, :study_group, index: true, null: true, foreign_key: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 82ec19bf..3435e610 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -315,6 +315,24 @@ ActiveRecord::Schema.define(version: 2018_11_29_093207) do t.index ["submission_id"], name: "index_structured_errors_on_submission_id" end + create_table "study_group_memberships", force: :cascade do |t| + t.bigint "study_group_id" + t.string "user_type" + t.bigint "user_id" + t.index ["study_group_id"], name: "index_study_group_memberships_on_study_group_id" + t.index ["user_type", "user_id"], name: "index_study_group_memberships_on_user_type_and_user_id" + end + + create_table "study_groups", force: :cascade do |t| + t.string "name" + t.string "external_id" + t.bigint "consumer_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["consumer_id"], name: "index_study_groups_on_consumer_id" + t.index ["external_id", "consumer_id"], name: "index_study_groups_on_external_id_and_consumer_id", unique: true + end + create_table "submissions", force: :cascade do |t| t.integer "exercise_id" t.float "score" @@ -323,7 +341,9 @@ ActiveRecord::Schema.define(version: 2018_11_29_093207) do t.datetime "updated_at" t.string "cause", limit: 255 t.string "user_type", limit: 255 + t.bigint "study_group_id" t.index ["exercise_id"], name: "index_submissions_on_exercise_id" + t.index ["study_group_id"], name: "index_submissions_on_study_group_id" t.index ["user_id"], name: "index_submissions_on_user_id" end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index c1ea7ba1..6692e56b 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -66,7 +66,7 @@ describe SessionsController do it 'refuses the LTI launch' do expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) expect(controller).to receive(:refuse_lti_launch).with(message: I18n.t('sessions.oauth.invalid_exercise_token')).and_call_original - post :create_through_lti, params: { custom_token: '', oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex } + post :create_through_lti, params: { custom_token: '', oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex, user_id: '123' } end end From 8558c5041f6279338a8a0cfe0b0235107db04879 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Thu, 13 Dec 2018 16:16:12 +0100 Subject: [PATCH 5/6] fix active record induced error on postgres by removing distinct. Inlining the code did not turn out well, as we have to cope with the polymorphic association of user here.. --- app/models/request_for_comment.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/request_for_comment.rb b/app/models/request_for_comment.rb index fe15409b..2461ca7b 100644 --- a/app/models/request_for_comment.rb +++ b/app/models/request_for_comment.rb @@ -47,10 +47,10 @@ class RequestForComment < ApplicationRecord def commenters commenters = [] - comments.distinct.to_a.each {|comment| + comments.each {|comment| commenters.append comment.user } - commenters.uniq {|user| user.id} + commenters.uniq end def self.with_last_activity From 76079bb47d6c8618531bc702e9d6f649097ce69a Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Fri, 14 Dec 2018 13:36:24 +0100 Subject: [PATCH 6/6] one line the loop, thanks for the hint @MrSerth --- app/models/request_for_comment.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/request_for_comment.rb b/app/models/request_for_comment.rb index 2461ca7b..b4fa819b 100644 --- a/app/models/request_for_comment.rb +++ b/app/models/request_for_comment.rb @@ -46,11 +46,7 @@ class RequestForComment < ApplicationRecord end def commenters - commenters = [] - comments.each {|comment| - commenters.append comment.user - } - commenters.uniq + comments.map(&:user).uniq end def self.with_last_activity