From da5597e7f93846339cc0cdf83d638a427dda9f61 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Fri, 26 Jan 2024 11:42:19 -0500 Subject: [PATCH] lots of perf fixes and other changes! --- .../javascripts/components/diff/_modal.html | 4 +++ .../diff/diff_modal_instance_controller.js | 28 +++++++++++++++++-- .../components/export_case/_modal.html | 4 +++ .../export_case_modal_instance_controller.js | 21 ++++++++++++++ .../javascripts/controllers/mainCtrl.js | 2 +- .../javascripts/services/querySnapshotSvc.js | 19 +++++++++---- .../javascripts/services/snapshotFactory.js | 1 + app/controllers/api/v1/books_controller.rb | 11 +++++++- .../api/v1/snapshots_controller.rb | 16 +++++------ app/controllers/books_controller.rb | 9 ++++-- app/models/book.rb | 9 +++++- app/models/snapshot.rb | 2 +- app/models/snapshot_query.rb | 2 +- app/views/home/_case_summary.html.erb | 2 +- .../_moar_judgements_needed.html.erb | 5 ++-- 15 files changed, 109 insertions(+), 26 deletions(-) diff --git a/app/assets/javascripts/components/diff/_modal.html b/app/assets/javascripts/components/diff/_modal.html index 444c30245..d3da36859 100644 --- a/app/assets/javascripts/components/diff/_modal.html +++ b/app/assets/javascripts/components/diff/_modal.html @@ -34,6 +34,10 @@

Compare Your Search Results

Snapshot has the id {{ ctrl.selection }} + +
diff --git a/app/assets/javascripts/components/diff/diff_modal_instance_controller.js b/app/assets/javascripts/components/diff/diff_modal_instance_controller.js index ad964fd0a..8d7aa232e 100644 --- a/app/assets/javascripts/components/diff/diff_modal_instance_controller.js +++ b/app/assets/javascripts/components/diff/diff_modal_instance_controller.js @@ -18,8 +18,11 @@ angular.module('QuepidApp') ) { var ctrl = this; - // Attributes - ctrl.snapshots = querySnapshotSvc.snapshots; + querySnapshotSvc.getSnapshots().then(function() { + ctrl.snapshots = querySnapshotSvc.snapshots; + } + ); + ctrl.which = 'snapshot'; ctrl.selection = initialSelection; ctrl.inProgress = false; @@ -32,6 +35,8 @@ angular.module('QuepidApp') ctrl.nothingSelected = nothingSelected; ctrl.ok = ok; ctrl.toggleDel = toggleDel; + ctrl.isProcessingFile = isProcessingFile; + // Watches $scope.$watch('ctrl.selection', function(newVal, oldVal) { @@ -72,6 +77,25 @@ angular.module('QuepidApp') flash.success = 'Snapshot deleted successfully.'; }); } + + function isProcessingFile() { + if (ctrl.snapshots){ + var desiredSnapshot = null; + angular.forEach(ctrl.snapshots, function(snapshot) { + if (snapshot.id === ctrl.selection) { + desiredSnapshot = snapshot; + return; // exit the loop early + } + }); + if (desiredSnapshot){ + return desiredSnapshot.hasSnapshotFile; + } + else { + return false; + } + } + return false; + } function isNumber(num) { return !isNaN(parseInt('' + num, 10)); diff --git a/app/assets/javascripts/components/export_case/_modal.html b/app/assets/javascripts/components/export_case/_modal.html index cd21e263e..92e511bca 100644 --- a/app/assets/javascripts/components/export_case/_modal.html +++ b/app/assets/javascripts/components/export_case/_modal.html @@ -50,6 +50,10 @@

Export Case: {{ ctrl.theCase.ca > + +

diff --git a/app/assets/javascripts/components/export_case/export_case_modal_instance_controller.js b/app/assets/javascripts/components/export_case/export_case_modal_instance_controller.js index e422efe6d..7bafab114 100644 --- a/app/assets/javascripts/components/export_case/export_case_modal_instance_controller.js +++ b/app/assets/javascripts/components/export_case/export_case_modal_instance_controller.js @@ -12,6 +12,7 @@ angular.module('QuepidApp') ctrl.theCase = theCase; ctrl.supportsDetailedExport = supportsDetailedExport; + ctrl.isProcessingFile = isProcessingFile; // If called from the cases listing page, then we need the call back with the bootstrap, // otherwise on the main page the querySnapshotSvc.snapshots was bootstrapped. @@ -47,5 +48,25 @@ angular.module('QuepidApp') ctrl.cancel = function () { $uibModalInstance.dismiss('cancel'); }; + + + function isProcessingFile() { + if (ctrl.options.snapshot){ + var desiredSnapshot = null; + angular.forEach(ctrl.snapshots, function(snapshot) { + if (snapshot.id === ctrl.selection) { + desiredSnapshot = snapshot; + return; // exit the loop early + } + }); + if (desiredSnapshot){ + return desiredSnapshot.hasSnapshotFile; + } + else { + return false; + } + } + return false; + } } ]); diff --git a/app/assets/javascripts/controllers/mainCtrl.js b/app/assets/javascripts/controllers/mainCtrl.js index 8e16b7fd0..85c16c2ca 100644 --- a/app/assets/javascripts/controllers/mainCtrl.js +++ b/app/assets/javascripts/controllers/mainCtrl.js @@ -147,7 +147,7 @@ angular.module('QuepidApp') bootstrapCase() .then(function() { loadQueries(); - loadSnapshots(); + loadSnapshots(); // this is here just to set the caseNo in the querySnapshotSvc. updateCaseMetadata(); paneSvc.refreshElements(); }).catch(function(error) { diff --git a/app/assets/javascripts/services/querySnapshotSvc.js b/app/assets/javascripts/services/querySnapshotSvc.js index ccff45a0f..dc2d13690 100644 --- a/app/assets/javascripts/services/querySnapshotSvc.js +++ b/app/assets/javascripts/services/querySnapshotSvc.js @@ -64,6 +64,20 @@ angular.module('QuepidApp') }); }); }; + + // Now that we process snapshots async, we + // don't want to cache the data + this.getSnapshots = function() { + this.snapshots = {}; + + return $http.get('api/cases/' + caseNo + '/snapshots?shallow=true') + .then(function(response) { + return addSnapshotResp(response.data.snapshots) + .then(function() { + version++; + }); + }); + }; this.addSnapshot = function(name, recordDocumentFields, queries) { // we may want to refactor the payload structure in the future. @@ -130,11 +144,6 @@ angular.module('QuepidApp') .then(function() { version++; }); - // return addSnapshotResp([response.data]) - // .then(function() { - // version++; - // }); - //}); }; this.deleteSnapshot = function(snapshotId) { diff --git a/app/assets/javascripts/services/snapshotFactory.js b/app/assets/javascripts/services/snapshotFactory.js index 2584c5208..96672cd2e 100644 --- a/app/assets/javascripts/services/snapshotFactory.js +++ b/app/assets/javascripts/services/snapshotFactory.js @@ -18,6 +18,7 @@ self.id = params.id; self.name = snapshotName; self.time = params.time; + self.hasSnapshotFile = params.has_snapshot_file; self.docs = params.docs; self.queries = params.queries; diff --git a/app/controllers/api/v1/books_controller.rb b/app/controllers/api/v1/books_controller.rb index 0cc777285..5877de607 100644 --- a/app/controllers/api/v1/books_controller.rb +++ b/app/controllers/api/v1/books_controller.rb @@ -4,6 +4,7 @@ module Api module V1 + # rubocop:disable Metrics/ClassLength class BooksController < Api::ApiController before_action :set_book, only: [ :show, :update, :destroy ] before_action :check_book, only: [ :show, :update, :destroy ] @@ -28,6 +29,7 @@ def index # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity + # rubocop:disable Metrics/BlockLength api :GET, '/api/books/:book_id', 'Show the book with the given ID.' param :id, :number, @@ -40,7 +42,11 @@ def show csv_headers = %w[query docid] # Only return rateable judgements, filter out the unrateable ones. - unique_raters = @book.judgements.rateable.preload(:user).collect(&:user).uniq + # unique_raters = @book.judgements.rateable.preload(:user).collect(&:user).uniq + unique_raters = @book.judges.merge(Judgement.rateable) + + puts 'HERE COME THE RATERS' + pp unique_raters # this logic about using email versus name is kind of awful. Think about user.full_name or user.identifier? unique_raters.each do |rater| @@ -72,6 +78,8 @@ def show # rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/CyclomaticComplexity # rubocop:enable Metrics/PerceivedComplexity + # rubocop:enable Metrics/BlockLength + api :POST, '/api/books', 'Create a new book.' param_group :book def create @@ -135,5 +143,6 @@ def make_csv_safe str end end end + # rubocop:enable Metrics/ClassLength end end diff --git a/app/controllers/api/v1/snapshots_controller.rb b/app/controllers/api/v1/snapshots_controller.rb index 2da9a6813..714e2a95b 100644 --- a/app/controllers/api/v1/snapshots_controller.rb +++ b/app/controllers/api/v1/snapshots_controller.rb @@ -34,22 +34,18 @@ def show respond_with @snapshot end - # rubocop:disable Metrics/MethodLength - # rubocop:disable Metrics/AbcSize # rubocop:disable Layout/LineLength def create @snapshot = @case.snapshots.build(name: params[:snapshot][:name]) @snapshot.scorer = @case.scorer @snapshot.try = @case.tries.first - puts "Okay, checking snapshot queries: #{@snapshot.snapshot_queries.length}" if @snapshot.save - puts "Okay2, checking snapshot queries: #{@snapshot.snapshot_queries.length}" serialized_data = Marshal.dump(snapshot_params) - puts "[SnapshotController] the size of the serialized data is #{number_to_human_size(serialized_data.bytesize)}" + # puts "[SnapshotController] the size of the serialized data is #{number_to_human_size(serialized_data.bytesize)}" compressed_data = Zlib::Deflate.deflate(serialized_data) - puts "[SnapshotController] the size of the compressed data is #{number_to_human_size(compressed_data.bytesize)}" + # puts "[SnapshotController] the size of the compressed data is #{number_to_human_size(compressed_data.bytesize)}" @snapshot.snapshot_file.attach(io: StringIO.new(compressed_data), filename: "snapshot_#{@snapshot.id}.bin.zip", content_type: 'application/zip') PopulateSnapshotJob.perform_later @snapshot @@ -62,11 +58,14 @@ def create render json: @snapshot.errors, status: :bad_request end end - # rubocop:enable Metrics/MethodLength - # rubocop:enable Metrics/AbcSize # rubocop:enable Layout/LineLength def destroy + # SnapshotDoc.joins(snapshot_query: :snapshot) + # .where(snapshot_queries: { snapshot: @snapshot }) + # .delete_all + @snapshot.snapshot_docs.delete_all + @snapshot.snapshot_queries.delete_all @snapshot.destroy Analytics::Tracker.track_snapshot_deleted_event current_user, @snapshot @@ -78,7 +77,6 @@ def destroy def set_snapshot @snapshot = @case.snapshots .where(id: params[:id]) - .includes([ snapshot_queries: [ :snapshot_docs ] ]) .first end diff --git a/app/controllers/books_controller.rb b/app/controllers/books_controller.rb index 5651d6db5..ad4fa0284 100644 --- a/app/controllers/books_controller.rb +++ b/app/controllers/books_controller.rb @@ -28,11 +28,16 @@ def show @book.cases.each do |kase| @count_of_anonymous_case_judgements += kase.ratings.where(user: nil).count end + + @moar_judgements_needed = @book.judgements.where(user: current_user).count < @book.query_doc_pairs.count @cases = @book.cases @leaderboard_data = [] @stats_data = [] - unique_judges = @book.judgements.preload(:user).collect(&:user).uniq - unique_judges.each do |judge| + + unique_judge_ids = @book.query_doc_pairs.joins(:judgements) + .distinct.pluck(:user_id) + unique_judge_ids.each do |judge_id| + judge = User.find(judge_id) unless judge_id.nil? @leaderboard_data << { judge: judge.nil? ? 'anonymous' : judge.name, judgements: @book.judgements.where(user: judge).count } @stats_data << { diff --git a/app/models/book.rb b/app/models/book.rb index 4832066ad..55934325d 100644 --- a/app/models/book.rb +++ b/app/models/book.rb @@ -35,10 +35,17 @@ class Book < ApplicationRecord belongs_to :selection_strategy belongs_to :scorer has_many :query_doc_pairs, dependent: :destroy, autosave: true - has_many :judgements, -> { order('query_doc_pair_id') }, + has_many :ordered_judgements, -> { order('query_doc_pair_id') }, + through: :query_doc_pairs, + dependent: :destroy, + class_name: 'Judgement' + + has_many :judgements, through: :query_doc_pairs, dependent: :destroy + # has_many :judges, -> { distinct }, through: :judgements, class_name: 'User', source: :user + has_many :cases, dependent: :nullify has_many :rated_query_doc_pairs, -> { has_judgements }, diff --git a/app/models/snapshot.rb b/app/models/snapshot.rb index 178f30475..44179f7bc 100644 --- a/app/models/snapshot.rb +++ b/app/models/snapshot.rb @@ -28,7 +28,7 @@ class Snapshot < ApplicationRecord belongs_to :case, optional: true # shouldn't be optional! belongs_to :try, optional: true # shouldn't be optional! belongs_to :scorer, optional: true # shouldn't be optional! - has_many :snapshot_queries, dependent: :destroy + has_many :snapshot_queries, dependent: :delete_all has_many :snapshot_docs, through: :snapshot_queries diff --git a/app/models/snapshot_query.rb b/app/models/snapshot_query.rb index 06b29ec0e..d28d2e0bd 100644 --- a/app/models/snapshot_query.rb +++ b/app/models/snapshot_query.rb @@ -26,6 +26,6 @@ class SnapshotQuery < ApplicationRecord belongs_to :snapshot, optional: true # shouldn't be belongs_to :query, optional: true # shouldn't be has_many :snapshot_docs, -> { order(position: :asc) }, - dependent: :destroy, + dependent: :delete_all, inverse_of: :snapshot_query end diff --git a/app/views/home/_case_summary.html.erb b/app/views/home/_case_summary.html.erb index d49b0d96a..12d387879 100644 --- a/app/views/home/_case_summary.html.erb +++ b/app/views/home/_case_summary.html.erb @@ -5,7 +5,7 @@
-
<%= kase.last_score.score unless kase.scores.empty? %> <%= kase.scorer.name %>
+
<%= number_with_precision(kase.last_score.score, precision: 2) unless kase.scores.empty? %> <%= kase.scorer.name %>

<%= kase.created_at.to_date.to_fs(:short) %> - <%= kase.last_score.updated_at.to_date.to_fs(:short) unless kase.scores.empty?%>

<% diff --git a/app/views/judgements/_moar_judgements_needed.html.erb b/app/views/judgements/_moar_judgements_needed.html.erb index 9b9d7f8d6..4135272dd 100644 --- a/app/views/judgements/_moar_judgements_needed.html.erb +++ b/app/views/judgements/_moar_judgements_needed.html.erb @@ -1,4 +1,5 @@ -<% if SelectionStrategy.random_query_doc_based_on_strategy(book, current_user) %> +hi: <%= @moar_judgements_needed %> +<% if @moar_judgements_needed %> @@ -35,7 +36,7 @@ <% else %> <% end %> <% end %>