Merge branch 'master' into add_roles_via_LTI

# Conflicts:
#	app/controllers/concerns/lti.rb

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
+    // /38/statistics good for testing
-    // originally at--> localhost:3333/exercises/69/statistics
 
-  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];
+              hours = timestamp.split(":")[0];
-              minutes = time_stamp.split(":")[1];
+              minutes = timestamp.split(":")[1];
-              seconds = time_stamp.split(":")[2];
+              seconds = timestamp.split(":")[2];
 
-              minutes = parseFloat(hours * 60) + parseInt(minutes);
+              return parseFloat(hours * 60) + parseInt(minutes);
-              return 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){
+          if (getWidth() * width_ratio > 1000){
-              width_ratio = 1000/getWidth();
+              width_ratio = 1000 / getWidth();
           }
-          var height_ratio = .7; // percent of height
+          var height_ratio = .7;
-
-          // currently sets as percentage of window width, however, unfortunately
-          // is not yet responsive
 
           var margin = {top: 100, right: 20, bottom: 70, left: 70},//30,50
               width = (getWidth() * width_ratio) - margin.left - margin.right,
@@ -139,96 +134,91 @@ $(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("fill", "none")
-              .attr("d", line);//---
+              .attr("d", line);
-          //.on("mousemove", mMove)//new again
-          //.append("title");
-
-          // function type(d) {
-          //     d.frequency = +d.frequency;
-          //     return d;
-          // }
       }
 
-      draw_line_graph();
+      drawLineGraph();
-
-      // 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]);
-      //}
 
       // DRAW THE SECOND GRAPH ------------------------------------------------------------------------------
-      function draw_bar_graph() {
+      function drawBarGraph() {
-          var number_of_bars = 40;
+          var groupWidth = 5;
-          var group_increment = Math.ceil(maximum_minutes / number_of_bars); // range in minutes
+          var groupRanges = 0;
-          var group_ranges = group_increment; // just for the start
+          var workingTimeGroups = [];
-          var minutes_array_for_bar = [];
 
           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))) {
+                  if ((minutes_array[i] >= groupRanges) && (minutes_array[i] < (groupRanges + groupWidth))) {
-                      section_value++;
+                      clusterCount++;
                   }
               }
-              minutes_array_for_bar.push(section_value);
+              workingTimeGroups.push(clusterCount);
-              group_ranges += group_increment;
+              groupRanges += groupWidth;
           }
-          while (group_ranges < maximum_minutes + group_increment);
+          while (groupRanges < maximum_minutes);
+          console.log(maximum_minutes);
 
-          //console.log(minutes_array_for_bar); // this var used as the bars
+          var clusterCount = 0,
-          //minutes_array_for_bar = [39, 20, 28, 20, 39, 34, 26, 23, 16, 8];
+              sum = 0,
-
+              maxVal = 0;
-          var max_of_array = Math.max.apply(Math, minutes_array_for_bar);
+          for (var i = 0; i < minutes_array.length; i++) {
-          var min_of_array = Math.min.apply(Math, minutes_array_for_bar);
+              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;
-          if (getWidth()*width_ratio > 1000){
-              width_ratio = 1000/getWidth();
-          }
-          var height_ratio = .7; // percent of height
 
-          var margin = {top: 100, right: 20, bottom: 70, left: 70},//30,50
+          // 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},
               width = (getWidth() * width_ratio) - margin.left - margin.right,
               height = (width * height_ratio) - margin.top - margin.bottom;
 
           var x = d3.scaleBand()
-              .range([0, width], .1);
+              .rangeRound([0, width])
-
+              .paddingInner(0.1)
-          var y = d3.scaleLinear()
+              .domain(workingTimeGroups.map(function (d, i) {
-              .range([0,height-(margin.top + margin.bottom)]);
+                  return i * groupWidth;
-
+              }));
 
           var xAxis = d3.axisBottom(x)
-              .ticks(10);
-
-
-          var yAxis = d3
-              .axisLeft(d3.scaleLinear().domain([0,max_of_array]).range([height,0]))//y
               .ticks(10)
-              .tickSizeInner(-width);
+              .tickValues(x.domain().filter(function(d, i){
+                  return (d % 10) === 0
+              }))
+              .tickFormat(function(d) { return d + "-" + (d + groupWidth) });
+
+          var y = d3.scaleLinear()
+              .domain([0, maxStudentsInGroup])
+              .range([height, 0]);
+
+          var yAxis = d3.axisLeft(y)
+              .ticks(10);
 
           var tip = d3.tip()
               .attr('class', 'd3-tip')
               .offset([-10, 0])
               .html(function(d) {
-                  return "<strong>Students:</strong> <span style='color:orange'>" + d + "</span>";
+                  return "<strong>Students: </strong><span style='color:orange'>" + d + "</span>";
               });
 
           var svg = d3.select("#chart_2").append("svg")
@@ -239,17 +229,6 @@ $(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 + ")")
@@ -269,10 +248,8 @@ $(document).on('turbolinks:load', function() {
               .attr("transform", "rotate(-90)")
               .attr("y", 6)
               .attr("dy", ".71em");
-              //.style("text-anchor", "end")
-              //.text("Students");
 
-          svg.append("text") // y axis label
+          svg.append("text")
               .attr("transform", "rotate(-90)")
               .attr("x", -height / 2)
               .attr("dy", "-3em")
@@ -280,7 +257,7 @@ $(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)
@@ -289,36 +266,31 @@ $(document).on('turbolinks:load', function() {
               .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;
+              .attr("x", function(d, i) {
-                                            var bar_x = i * bar_increment;
+                  return x(i * groupWidth);
-                                            return (bar_x)})
+              })
               .attr("width", x.bandwidth())
-              .attr("y", function(d) { return height - y(d); })
+              .attr("y", function(d) { return y(d); })
-              .attr("height", function(d) { return y(d); })
+              .attr("height", function(d) { return height - y(d); })
-              .on('mouseover', tip.show)
+              .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();
   }
 
 });

app/controllers/concerns/lti.rb

@@ -69,6 +69,11 @@ module Lti
     result
   end
 
+  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
@@ -154,6 +159,14 @@ 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 set_embedding_options
     @embed_options = {}
     [:hide_navbar,

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, :set_embedding_options].each do |method_name|
+  [:require_oauth_parameters, :require_valid_consumer_key, :require_valid_oauth_signature, :require_unique_oauth_nonce, :set_current_user, :require_valid_exercise_token, :set_study_group_membership, :set_embedding_options].each do |method_name|
     before_action(method_name, only: :create_through_lti)
   end
 

app/models/concerns/user.rb

@@ -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

app/models/external_user.rb

@@ -1,5 +1,4 @@
-class ExternalUser < ApplicationRecord
+class ExternalUser < User
-  include User
 
   validates :consumer_id, presence: true
   validates :external_id, presence: true

app/models/internal_user.rb

@@ -1,5 +1,4 @@
-class InternalUser < ApplicationRecord
+class InternalUser < User
-  include User
 
   authenticates_with_sorcery!
 

app/models/request_for_comment.rb

@@ -46,11 +46,7 @@ class RequestForComment < ApplicationRecord
   end
 
   def commenters
-    commenters = []
+    comments.map(&:user).uniq
-    comments.distinct.to_a.each {|comment|
-      commenters.append comment.user
-    }
-    commenters.uniq {|user| user.id}
   end
 
   def self.with_last_activity

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

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

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

app/models/user.rb

@@ -0,0 +1,36 @@
+class User < ApplicationRecord
+  self.abstract_class = true
+
+  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
+  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

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

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
+

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

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

db/schema.rb

@@ -320,6 +320,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"
@@ -328,7 +346,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
 

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