From dc8dd51d1eb5f0131862e4433d4cc91964cee730 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Sat, 23 Jan 2021 22:18:57 -0300 Subject: [PATCH 01/17] 1st optimization: preload lesson's exercises --- app/views/chapters/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/chapters/show.html.erb b/app/views/chapters/show.html.erb index d8ab2bf487..1fe8be2073 100644 --- a/app/views/chapters/show.html.erb +++ b/app/views/chapters/show.html.erb @@ -26,7 +26,7 @@

<%= t(:lessons) %>

- <% @chapter.lessons.each do |lesson| %> + <% @chapter.lessons.includes(guide: :exercises).each do |lesson| %>

<%= lesson.number %>. <%= link_to_path_element lesson, mode: :plain %>

<%= render partial: 'layouts/progress_listing', locals: { exercises: lesson.exercises } %> <% end %> From c66abf816e9394530986bd21a2bc31676bc6ce12 Mon Sep 17 00:00:00 2001 From: Franco Leonardo Bulgarelli Date: Tue, 26 Jan 2021 13:59:19 -0300 Subject: [PATCH 02/17] Point to domain branch --- Gemfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Gemfile b/Gemfile index 7e379383cc..f38fc0840b 100644 --- a/Gemfile +++ b/Gemfile @@ -40,3 +40,6 @@ group :development, :test do gem 'teaspoon-jasmine' gem 'selenium-webdriver' end + + +gem "mumuki-domain", github: "mumuki/mumuki-domain", branch: "chore-database-optimization" \ No newline at end of file From 04c15142449c619e408c2d183788d95acf0e37f6 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Sat, 23 Jan 2021 22:19:40 -0300 Subject: [PATCH 03/17] 2nd optimization: Use assignments_for & reuse them --- app/helpers/icons_helper.rb | 10 +++------- app/views/chapters/show.html.erb | 2 +- app/views/layouts/_guide.html.erb | 2 +- app/views/layouts/_progress_listing.html.erb | 10 +++++----- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb index 0c92b68c59..356aa88970 100644 --- a/app/helpers/icons_helper.rb +++ b/app/helpers/icons_helper.rb @@ -7,9 +7,9 @@ def fixed_fa_icon(name, options = {}) fa_icon name, options.merge(class: 'fa-fw fixed-icon') end - def exercise_status_icon(exercise) - link_to exercise_status_fa_icon(exercise), - exercise_path(exercise) if current_user? + def assignment_status_icon(assignment) + link_to contextualization_fa_icon(assignment), + exercise_path(assignment.exercise) if current_user? end def language_icon(language) @@ -22,10 +22,6 @@ def contextualization_fa_icon(contextualization) fa_icon(*icon_for(contextualization)) end - def exercise_status_fa_icon(exercise) - contextualization_fa_icon(exercise.assignment_for(current_user)) - end - def discussion_status_fa_icon(discussion) contextualization_fa_icon(discussion) end diff --git a/app/views/chapters/show.html.erb b/app/views/chapters/show.html.erb index 1fe8be2073..4a3dfb5141 100644 --- a/app/views/chapters/show.html.erb +++ b/app/views/chapters/show.html.erb @@ -28,7 +28,7 @@ <% @chapter.lessons.includes(guide: :exercises).each do |lesson| %>

<%= lesson.number %>. <%= link_to_path_element lesson, mode: :plain %>

- <%= render partial: 'layouts/progress_listing', locals: { exercises: lesson.exercises } %> + <%= render partial: 'layouts/progress_listing', locals: { guide: lesson.guide } %> <% end %>
<% end %> diff --git a/app/views/layouts/_guide.html.erb b/app/views/layouts/_guide.html.erb index 4726ac48e2..46822c0d68 100644 --- a/app/views/layouts/_guide.html.erb +++ b/app/views/layouts/_guide.html.erb @@ -13,7 +13,7 @@ <%= restart_guide_link(@guide) if current_user && @guide.started?(current_user) && @guide.resettable? %> -<%= render partial: 'layouts/progress_listing', locals: {exercises: @guide.exercises} %> +<%= render partial: 'layouts/progress_listing', locals: { guide: @guide } %> <% if @stats&.done? %>
diff --git a/app/views/layouts/_progress_listing.html.erb b/app/views/layouts/_progress_listing.html.erb index d0bace21b7..6cce14a771 100644 --- a/app/views/layouts/_progress_listing.html.erb +++ b/app/views/layouts/_progress_listing.html.erb @@ -1,9 +1,9 @@
From b78a45a04f889942abd253786cc056d968d3342e Mon Sep 17 00:00:00 2001 From: Franco Leonardo Bulgarelli Date: Sat, 30 Jan 2021 19:39:42 -0300 Subject: [PATCH 12/17] Update domain --- Gemfile | 3 --- mumuki-laboratory.gemspec | 2 +- spec/dummy/db/schema.rb | 1 + 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index f38fc0840b..7e379383cc 100644 --- a/Gemfile +++ b/Gemfile @@ -40,6 +40,3 @@ group :development, :test do gem 'teaspoon-jasmine' gem 'selenium-webdriver' end - - -gem "mumuki-domain", github: "mumuki/mumuki-domain", branch: "chore-database-optimization" \ No newline at end of file diff --git a/mumuki-laboratory.gemspec b/mumuki-laboratory.gemspec index 4c7612867b..82246dad79 100644 --- a/mumuki-laboratory.gemspec +++ b/mumuki-laboratory.gemspec @@ -18,7 +18,7 @@ Gem::Specification.new do |s| s.add_dependency "rails", "~> 5.1.6" - s.add_dependency 'mumuki-domain', '~> 8.3.0' + s.add_dependency 'mumuki-domain', '~> 8.4.0' s.add_dependency 'mumukit-bridge', '~> 4.1' s.add_dependency 'mumukit-login', '~> 7.3' s.add_dependency 'mumukit-nuntius', '~> 6.1' diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 40f638bb85..df36d31055 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -482,6 +482,7 @@ t.datetime "legal_terms_accepted_at" t.datetime "forum_terms_accepted_at" t.boolean "banned_from_forum" + t.boolean "uppercase_mode" t.index ["avatar_type", "avatar_id"], name: "index_users_on_avatar_type_and_avatar_id" t.index ["disabled_at"], name: "index_users_on_disabled_at" t.index ["last_organization_id"], name: "index_users_on_last_organization_id" From b2d471d1f63a393fbb997d8c5b993f737976e909 Mon Sep 17 00:00:00 2001 From: Franco Leonardo Bulgarelli Date: Sat, 30 Jan 2021 19:40:26 -0300 Subject: [PATCH 13/17] 4th optimization: cache progress items --- app/views/layouts/_progress_bar.html.erb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/app/views/layouts/_progress_bar.html.erb b/app/views/layouts/_progress_bar.html.erb index 168707b110..7f072d87a5 100644 --- a/app/views/layouts/_progress_bar.html.erb +++ b/app/views/layouts/_progress_bar.html.erb @@ -2,13 +2,15 @@ <% assignments = guide.assignments_for(current_user) %> <% assignments.each do |assignment| %> <% exercise = assignment.exercise %> - - href="<%= exercise_path(exercise)%>" - aria-label="<%= exercise.navigable_name %>" - title="<%= exercise.navigable_name %>" - data-mu-exercise-id="<%= exercise.id %>" - class="<%= class_for_progress_list_item(assignment, exercise == current)%>"> - + <% cache [:progress_list_item, exercise, assignment.status, Organization.current, exercise == current] do %> + + href="<%= exercise_path(exercise)%>" + aria-label="<%= exercise.navigable_name %>" + title="<%= exercise.navigable_name %>" + data-mu-exercise-id="<%= exercise.id %>" + class="<%= class_for_progress_list_item(assignment, exercise == current)%>"> + + <% end %> <% end %> - + \ No newline at end of file From c7018650c52eeeac1dd9cf23ddb0c999edc4ba44 Mon Sep 17 00:00:00 2001 From: Franco Leonardo Bulgarelli Date: Sat, 30 Jan 2021 22:34:22 -0300 Subject: [PATCH 14/17] Load some deps only once --- app/controllers/exercises_controller.rb | 19 +++++++++++++------ app/views/exercises/show.html.erb | 10 +++------- app/views/layouts/_progress_bar.html.erb | 7 +++---- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index b38dd46163..4485adf579 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -3,8 +3,10 @@ class ExercisesController < ApplicationController include Mumuki::Laboratory::Controllers::ExerciseSeed include Mumuki::Laboratory::Controllers::ImmersiveNavigation - before_action :set_guide!, only: :show + before_action :set_parents!, only: :show + before_action :set_progress!, only: :show, if: :current_user? before_action :set_assignment!, only: :show, if: :current_user? + before_action :validate_accessible!, only: :show before_action :start!, only: :show @@ -20,15 +22,15 @@ def show_transparently private def subject - @exercise ||= Exercise.find_by(id: params[:id]) + @exercise ||= Exercise.find(params[:id]) end def accessible_subject - subject.navigable_parent + @navigable_parent end def start! - @exercise.navigable_parent.start! current_user + @navigable_parent.start! current_user end def set_assignment! @@ -38,9 +40,14 @@ def set_assignment! @default_content = @assignment.default_content end - def set_guide! - raise Mumuki::Domain::NotFoundError if @exercise.nil? + def set_parents! @guide = @exercise.guide + @navigable_parent = @exercise.navigable_parent + end + + def set_progress! + @stats = @guide.stats_for(current_user) + @assignments = @guide.assignments_for(current_user) end def exercise_params diff --git a/app/views/exercises/show.html.erb b/app/views/exercises/show.html.erb index 3bb077f1cc..4b5340f92c 100644 --- a/app/views/exercises/show.html.erb +++ b/app/views/exercises/show.html.erb @@ -6,10 +6,8 @@ <%= render partial: 'layouts/authoring', locals: {guide: @guide} %> -<% @stats = @exercise.stats_for(current_user) %> - -<% if @exercise.navigable_parent.timed? && !current_user.teacher? %> - <%= render partial: 'layouts/timer', locals: {end_time: @exercise.navigable_parent.real_end_time(current_user)} %> +<% if @navigable_parent.timed? && !current_user.teacher? %> + <%= render partial: 'layouts/timer', locals: {end_time: @navigable_parent.real_end_time(current_user)} %> <% end %> <% unless @exercise.input_kids? %> @@ -29,9 +27,7 @@ <% end %>
- <% if @stats && standalone_mode? %> - <%= render partial: 'layouts/progress_bar', locals: {current: @exercise, guide: @exercise.guide, stats: @stats} %> - <% end %> + <%= render partial: 'layouts/progress_bar' if current_user? && standalone_mode? %>
<% content_for :assignment do %> diff --git a/app/views/layouts/_progress_bar.html.erb b/app/views/layouts/_progress_bar.html.erb index 7f072d87a5..d98313e1cd 100644 --- a/app/views/layouts/_progress_bar.html.erb +++ b/app/views/layouts/_progress_bar.html.erb @@ -1,15 +1,14 @@
- <% assignments = guide.assignments_for(current_user) %> - <% assignments.each do |assignment| %> + <% @assignments.each do |assignment| %> <% exercise = assignment.exercise %> - <% cache [:progress_list_item, exercise, assignment.status, Organization.current, exercise == current] do %> + <% cache [:progress_list_item, exercise, assignment.status, Organization.current, exercise == @exercise] do %> href="<%= exercise_path(exercise)%>" aria-label="<%= exercise.navigable_name %>" title="<%= exercise.navigable_name %>" data-mu-exercise-id="<%= exercise.id %>" - class="<%= class_for_progress_list_item(assignment, exercise == current)%>"> + class="<%= class_for_progress_list_item(assignment, exercise == @exercise)%>"> <% end %> <% end %> From 3930d9457432599052b3fa40cf4dcd38ac8f9cb6 Mon Sep 17 00:00:00 2001 From: Franco Leonardo Bulgarelli Date: Sun, 31 Jan 2021 01:21:23 -0300 Subject: [PATCH 15/17] Use assignments_and_stats_for --- app/controllers/exercises_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 4485adf579..2e2d4f5927 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -46,8 +46,7 @@ def set_parents! end def set_progress! - @stats = @guide.stats_for(current_user) - @assignments = @guide.assignments_for(current_user) + @assignments, @stats = @guide.assignments_and_stats_for(current_user) end def exercise_params From ce12fb9772cf74123cc071fc6e12d967e3773e52 Mon Sep 17 00:00:00 2001 From: Franco Leonardo Bulgarelli Date: Sun, 31 Jan 2021 01:44:14 -0300 Subject: [PATCH 16/17] Avoid recheck for usage --- app/controllers/exercises_controller.rb | 11 ++++++++++- lib/mumuki/laboratory/controllers/content.rb | 6 +++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 2e2d4f5927..9de8d7057a 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -1,9 +1,10 @@ class ExercisesController < ApplicationController + before_action :set_parents!, only: :show + include Mumuki::Laboratory::Controllers::Content include Mumuki::Laboratory::Controllers::ExerciseSeed include Mumuki::Laboratory::Controllers::ImmersiveNavigation - before_action :set_parents!, only: :show before_action :set_progress!, only: :show, if: :current_user? before_action :set_assignment!, only: :show, if: :current_user? @@ -56,4 +57,12 @@ def exercise_params :guide_id, :number, :layout, :expectations_yaml) end + + + def subject_used_here? + # overriden because of performance reasons + # this method will be called only on show, so it is safe + # to check for navigable_parent presence + @navigable_parent.present? + end end diff --git a/lib/mumuki/laboratory/controllers/content.rb b/lib/mumuki/laboratory/controllers/content.rb index 31921f3354..480d0a81bd 100644 --- a/lib/mumuki/laboratory/controllers/content.rb +++ b/lib/mumuki/laboratory/controllers/content.rb @@ -7,6 +7,10 @@ module Mumuki::Laboratory::Controllers::Content # ensures contents are in current organization's path def validate_used_here! #TODO refactor subject logic (e.g. we can't move validate_accessible! here because ExerciseSolutionsController does not declare a subject) - raise Mumuki::Domain::NotFoundError unless subject&.used_in?(Organization.current) + raise Mumuki::Domain::NotFoundError unless subject_used_here? + end + + def subject_used_here? + subject&.used_in?(Organization.current) end end From 3c99e1d415947612443f212cde5c761b2d6824d6 Mon Sep 17 00:00:00 2001 From: Franco Leonardo Bulgarelli Date: Mon, 1 Feb 2021 10:37:06 -0300 Subject: [PATCH 17/17] Avoid loading stats multiple times With this change stats are loaded at most twice. After it, stats were loaded exactly 3 times --- app/controllers/exercise_solutions_controller.rb | 10 ++++++++++ app/views/exercise_solutions/_results.html.erb | 5 ----- lib/mumuki/laboratory/controllers/results_rendering.rb | 7 +------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/controllers/exercise_solutions_controller.rb b/app/controllers/exercise_solutions_controller.rb index a39b7028ef..8c44efa5da 100644 --- a/app/controllers/exercise_solutions_controller.rb +++ b/app/controllers/exercise_solutions_controller.rb @@ -4,6 +4,8 @@ class ExerciseSolutionsController < AjaxController include Mumuki::Laboratory::Controllers::ExerciseSeed before_action :set_messages, only: :create + before_action :set_guide!, only: :create + before_action :set_stats!, only: :create before_action :validate_accessible!, only: :create def create @@ -27,4 +29,12 @@ def solution_params client_result: params[:client_result].try { |it| it.permit(:status, test_results: [:title, :status, :result, :summary]).to_h } } end + + def set_guide! + @guide = @exercise.guide + end + + def set_stats! + @stats = @guide.stats_for(current_user) + end end diff --git a/app/views/exercise_solutions/_results.html.erb b/app/views/exercise_solutions/_results.html.erb index bb4a343fe5..1090b83394 100644 --- a/app/views/exercise_solutions/_results.html.erb +++ b/app/views/exercise_solutions/_results.html.erb @@ -1,8 +1,3 @@ -<% - @guide = @exercise.guide - @stats = @guide.stats_for(current_user) -%> - <% if assignment.results_hidden? %> <%= render partial: 'exercise_solutions/results_hidden', locals: {assignment: assignment} %> <% elsif !assignment.attempts_left? %> diff --git a/lib/mumuki/laboratory/controllers/results_rendering.rb b/lib/mumuki/laboratory/controllers/results_rendering.rb index b2b8dec4a6..97172d5f8f 100644 --- a/lib/mumuki/laboratory/controllers/results_rendering.rb +++ b/lib/mumuki/laboratory/controllers/results_rendering.rb @@ -3,8 +3,6 @@ module Mumuki::Laboratory::Controllers::ResultsRendering included do include ProgressBarHelper, ExerciseInputHelper - - before_action :set_guide_previously_done! end def render_results_json(assignment, results = {}) @@ -56,10 +54,7 @@ def render_results_button(assignment) end def guide_finished_by_solution? - !@guide_previously_done && @exercise.guide_done_for?(current_user) + @stats.almost_but_not_done? && @exercise.guide_done_for?(current_user) end - def set_guide_previously_done! - @guide_previously_done = @exercise.guide_done_for?(current_user) - end end