From 47a5dc5e3efa9a773ba00a71e043ebcfbca55928 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Fri, 2 Jun 2023 17:04:21 -0400 Subject: [PATCH 01/38] check uniqueness of user rating --- app/controllers/api/v1/books_controller.rb | 10 ++++++++-- app/models/judgement.rb | 1 + test/models/judgement_test.rb | 17 +++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v1/books_controller.rb b/app/controllers/api/v1/books_controller.rb index 41796976c..ec5f06bf3 100644 --- a/app/controllers/api/v1/books_controller.rb +++ b/app/controllers/api/v1/books_controller.rb @@ -22,9 +22,15 @@ def show # Only return rateable judgements, filter out the unrateable ones. unique_raters = @book.judgements.rateable.collect(&:user).uniq - + # this logic about using email versus name is kind of awful. Think about user.full_name or user.identifier? - unique_raters.each { |rater| csv_headers << make_csv_safe(rater.nil? ? 'Unknown' : rater.name.blank? ? rater.email : rater.name) } + unique_raters.each do |rater| + csv_headers << make_csv_safe(if rater.nil? + 'Unknown' + else + (rater.name.presence || rater.email) + end) + end @csv_array << csv_headers diff --git a/app/models/judgement.rb b/app/models/judgement.rb index 4a662879e..a95d2b373 100644 --- a/app/models/judgement.rb +++ b/app/models/judgement.rb @@ -24,6 +24,7 @@ class Judgement < ApplicationRecord belongs_to :query_doc_pair belongs_to :user, optional: true + validates :user_id, :uniqueness => { :scope => :query_doc_pair_id } validates :rating, presence: true, unless: :unrateable diff --git a/test/models/judgement_test.rb b/test/models/judgement_test.rb index 7099c533b..ba5e99f00 100644 --- a/test/models/judgement_test.rb +++ b/test/models/judgement_test.rb @@ -23,6 +23,23 @@ require 'test_helper' class JudgementTest < ActiveSupport::TestCase + describe 'uniqueness of judgements' do + let(:query_doc_pair) { query_doc_pairs(:one) } + let(:user) { users(:random) } + let(:user2) { users(:doug) } + + test 'Prevent saving two judgements from the same user' do + judgement = Judgement.create(user: user, query_doc_pair: query_doc_pair, rating: 4.4) + assert judgement.save + + duplicate_judgement = Judgement.create(user: user, query_doc_pair: query_doc_pair, rating: 1.0) + assert_not duplicate_judgement.save + assert duplicate_judgement.errors.include?(:user_id) + + judgement2 = Judgement.create(user: user2, query_doc_pair: query_doc_pair, rating: 1.0) + assert judgement2.save + end + end describe 'unrateable attribute behavior' do let(:query_doc_pair) { query_doc_pairs(:one) } From 24ce1e13dded9185de2170631e6fa07bcc992cca Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Fri, 2 Jun 2023 18:19:52 -0400 Subject: [PATCH 02/38] lint --- app/controllers/judgements_controller.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/app/controllers/judgements_controller.rb b/app/controllers/judgements_controller.rb index 2d7328b4a..bc6090898 100644 --- a/app/controllers/judgements_controller.rb +++ b/app/controllers/judgements_controller.rb @@ -43,20 +43,17 @@ def create end def unrateable - @judgement = Judgement.new(query_doc_pair_id: params[:query_doc_pair_id]) - @judgement.user = current_user - - if @judgement.mark_unrateable! - session['previous_judgement_id'] = @judgement.id - redirect_to book_judge_path(@book) - else - render action: :edit - end + @judgement = Judgement.find_or_initialize_by(query_doc_pair_id: params[:query_doc_pair_id], user: current_user) + + @judgement.mark_unrateable! + session['previous_judgement_id'] = @judgement.id + redirect_to book_judge_path(@book) end def update @judgement.update(judgement_params) @judgement.user = current_user + @judgement.unrateable = false if @judgement.save redirect_to book_judge_path(@book) else From 21b6bb5c4a95d9b0214772764c3754d76394a35c Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Fri, 2 Jun 2023 18:20:12 -0400 Subject: [PATCH 03/38] clear existing ratings if you decide that it is not rateable --- app/models/judgement.rb | 1 + test/models/judgement_test.rb | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/app/models/judgement.rb b/app/models/judgement.rb index a95d2b373..e85cc28d5 100644 --- a/app/models/judgement.rb +++ b/app/models/judgement.rb @@ -32,6 +32,7 @@ class Judgement < ApplicationRecord def mark_unrateable self.unrateable = true + self.rating = nil end def mark_unrateable! diff --git a/test/models/judgement_test.rb b/test/models/judgement_test.rb index ba5e99f00..f871868fe 100644 --- a/test/models/judgement_test.rb +++ b/test/models/judgement_test.rb @@ -62,5 +62,11 @@ class JudgementTest < ActiveSupport::TestCase assert judgement.valid? end + + test 'mark a judgement with ratings as unrateble clears exiting rating' do + judgement = Judgement.create(query_doc_pair: query_doc_pair, rating: 4.4) + judgement.mark_unrateable! + assert_nil judgement.rating + end end end From 806676e61fa55a475782f5126b8ecded23b5750d Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Fri, 2 Jun 2023 18:39:39 -0400 Subject: [PATCH 04/38] clean up the export logic to be a LOT faster, way too many database queetries --- app/controllers/api/v1/books_controller.rb | 18 +++++++----------- app/controllers/judgements_controller.rb | 2 +- test/models/judgement_test.rb | 2 +- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/app/controllers/api/v1/books_controller.rb b/app/controllers/api/v1/books_controller.rb index ec5f06bf3..110f11884 100644 --- a/app/controllers/api/v1/books_controller.rb +++ b/app/controllers/api/v1/books_controller.rb @@ -37,16 +37,12 @@ def show @book.query_doc_pairs.each do |qdp| row = [ make_csv_safe(qdp.query_text), qdp.doc_id ] unique_raters.each do |rater| - user_id = rater&.id - judgements = qdp.judgements.where(user_id: user_id) - if judgements.empty? - rating = '' - elsif 1 == judgements.size - judgement = judgements.first - rating = judgement.nil? ? '' : judgement.rating - else - rating = judgements.pluck(:rating).join('|') - end + judgement = qdp.judgements.find { |judgement| judgement.user = rater } + rating = if judgement.nil? + '' + else + judgement.nil? ? '' : judgement.rating + end row.append rating end @csv_array << row @@ -66,7 +62,7 @@ def show private def find_book - @book = current_user.books_involved_with.where(id: params[:id]).first + @book = current_user.books_involved_with.where(id: params[:id]).includes(:query_doc_pairs).preload([ query_doc_pairs: [ :judgements ] ]).first end def check_book diff --git a/app/controllers/judgements_controller.rb b/app/controllers/judgements_controller.rb index bc6090898..1a8c4ad54 100644 --- a/app/controllers/judgements_controller.rb +++ b/app/controllers/judgements_controller.rb @@ -44,7 +44,7 @@ def create def unrateable @judgement = Judgement.find_or_initialize_by(query_doc_pair_id: params[:query_doc_pair_id], user: current_user) - + @judgement.mark_unrateable! session['previous_judgement_id'] = @judgement.id redirect_to book_judge_path(@book) diff --git a/test/models/judgement_test.rb b/test/models/judgement_test.rb index f871868fe..fc5d865f4 100644 --- a/test/models/judgement_test.rb +++ b/test/models/judgement_test.rb @@ -62,7 +62,7 @@ class JudgementTest < ActiveSupport::TestCase assert judgement.valid? end - + test 'mark a judgement with ratings as unrateble clears exiting rating' do judgement = Judgement.create(query_doc_pair: query_doc_pair, rating: 4.4) judgement.mark_unrateable! From dd223e227d981f9853040e4b84a03658372f7410 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Sun, 4 Jun 2023 08:23:35 -0400 Subject: [PATCH 05/38] lets remove a session variable and have the ability to click to previous judgements for any specific judgement so you can go back in time --- app/controllers/api/v1/books_controller.rb | 13 +++++-------- app/controllers/judgements_controller.rb | 8 +++----- app/models/judgement.rb | 13 ++++++++++++- .../judgements/_moar_judgements_needed.html.erb | 4 +--- app/views/judgements/edit.html.erb | 8 +++++++- app/views/judgements/new.html.erb | 8 +++----- ...ndex_to_judgements_on_user_and_query_doc_pair.rb | 5 +++++ db/schema.rb | 3 ++- test/fixtures/judgements.yml | 3 ++- test/models/judgement_test.rb | 3 ++- 10 files changed, 42 insertions(+), 26 deletions(-) create mode 100644 db/migrate/20230604121856_add_index_to_judgements_on_user_and_query_doc_pair.rb diff --git a/app/controllers/api/v1/books_controller.rb b/app/controllers/api/v1/books_controller.rb index 110f11884..28c622205 100644 --- a/app/controllers/api/v1/books_controller.rb +++ b/app/controllers/api/v1/books_controller.rb @@ -12,7 +12,6 @@ class BooksController < Api::ApiController # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity - # rubocop:disable Metrics/BlockLength def show respond_to do |format| format.json @@ -37,12 +36,9 @@ def show @book.query_doc_pairs.each do |qdp| row = [ make_csv_safe(qdp.query_text), qdp.doc_id ] unique_raters.each do |rater| - judgement = qdp.judgements.find { |judgement| judgement.user = rater } - rating = if judgement.nil? - '' - else - judgement.nil? ? '' : judgement.rating - end + judgement = qdp.judgements.find { |j| j.user = rater } + rating = judgement.nil? ? '' : judgement.rating + row.append rating end @csv_array << row @@ -57,13 +53,14 @@ def show # rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/CyclomaticComplexity # rubocop:enable Metrics/PerceivedComplexity - # rubocop:enable Metrics/BlockLength private + # rubocop:disable Layout/LineLength def find_book @book = current_user.books_involved_with.where(id: params[:id]).includes(:query_doc_pairs).preload([ query_doc_pairs: [ :judgements ] ]).first end + # rubocop:enable Layout/LineLength def check_book render json: { message: 'Book not found!' }, status: :not_found unless @book diff --git a/app/controllers/judgements_controller.rb b/app/controllers/judgements_controller.rb index 1a8c4ad54..c53fb1ee9 100644 --- a/app/controllers/judgements_controller.rb +++ b/app/controllers/judgements_controller.rb @@ -14,7 +14,6 @@ def show end def skip_judging - session['previous_judgement_id'] = nil redirect_to book_judge_path(@book) end @@ -24,10 +23,11 @@ def new if @query_doc_pair @query = @current_user.queries.has_information_need.where(query_text: @query_doc_pair.query_text).first end - @judgement = Judgement.new + @judgement = Judgement.new(query_doc_pair: @query_doc_pair, user: @current_user, updated_at: Time.zone.now) end - def edit; end + def edit + end def create @judgement = Judgement.new(judgement_params) @@ -35,7 +35,6 @@ def create @judgement.unrateable = false if @judgement.save - session['previous_judgement_id'] = @judgement.id redirect_to book_judge_path(@book) else render action: :edit @@ -46,7 +45,6 @@ def unrateable @judgement = Judgement.find_or_initialize_by(query_doc_pair_id: params[:query_doc_pair_id], user: current_user) @judgement.mark_unrateable! - session['previous_judgement_id'] = @judgement.id redirect_to book_judge_path(@book) end diff --git a/app/models/judgement.rb b/app/models/judgement.rb index e85cc28d5..7db752a1b 100644 --- a/app/models/judgement.rb +++ b/app/models/judgement.rb @@ -14,7 +14,8 @@ # # Indexes # -# index_judgements_on_query_doc_pair_id (query_doc_pair_id) +# index_judgements_on_query_doc_pair_id (query_doc_pair_id) +# index_judgements_on_user_id_and_query_doc_pair_id (user_id,query_doc_pair_id) UNIQUE # # Foreign Keys # @@ -39,4 +40,14 @@ def mark_unrateable! mark_unrateable save end + + # Based on a judgement, find the previous one made by the + # same user + def previous_judgement_made + # if self.query_doc_pair + query_doc_pair.book.judgements.where(judgements: { user_id: user.id }).where( + 'judgements.updated_at < ?', updated_at + ).order('judgements.updated_at DESC').first + # end + end end diff --git a/app/views/judgements/_moar_judgements_needed.html.erb b/app/views/judgements/_moar_judgements_needed.html.erb index 82c6ad54f..d7d80968d 100644 --- a/app/views/judgements/_moar_judgements_needed.html.erb +++ b/app/views/judgements/_moar_judgements_needed.html.erb @@ -1,6 +1,4 @@ -<% if SelectionStrategy.random_query_doc_based_on_strategy(book, current_user) %> - <% session["previous_judgement_id"] = nil %> - +<% if SelectionStrategy.random_query_doc_based_on_strategy(book, current_user) %> diff --git a/app/views/judgements/edit.html.erb b/app/views/judgements/edit.html.erb index 264bb32ba..ad7faf795 100644 --- a/app/views/judgements/edit.html.erb +++ b/app/views/judgements/edit.html.erb @@ -4,7 +4,13 @@ <%= render 'form', judgement: @judgement, query_doc_pair: @judgement.query_doc_pair, url_for_form: book_query_doc_pair_judgement_path(@book, @judgement.query_doc_pair, @judgement) %>
- +<% + previous_judgement = @judgement.previous_judgement_made +%> +<% if previous_judgement %> + <%= button_to 'Go Back to Previous Query/Doc Pair', edit_book_query_doc_pair_judgement_path(@book, previous_judgement.query_doc_pair, previous_judgement), method: :get %> +
+<% end %> <%= button_to 'Exit', book_judgements_path(@book), method: :get %>
diff --git a/app/views/judgements/new.html.erb b/app/views/judgements/new.html.erb index 1fd7deeef..083379228 100644 --- a/app/views/judgements/new.html.erb +++ b/app/views/judgements/new.html.erb @@ -6,13 +6,11 @@
<% - @previous_judgement = session["previous_judgement_id"].nil? ? nil : Judgement.find_by_id(session["previous_judgement_id"]) + previous_judgement = @judgement.previous_judgement_made %> -<% if @previous_judgement %> - <%= button_to 'Go Back to Previous Query/Doc Pair', edit_book_query_doc_pair_judgement_path(@book, @previous_judgement.query_doc_pair, @previous_judgement.id), method: :get %> +<% if previous_judgement %> + <%= button_to 'Go Back to Previous Query/Doc Pair', edit_book_query_doc_pair_judgement_path(@book, previous_judgement.query_doc_pair, previous_judgement), method: :get %>
<% end %> <%= button_to 'Exit', book_url(@book), method: :get %>
- -
diff --git a/db/migrate/20230604121856_add_index_to_judgements_on_user_and_query_doc_pair.rb b/db/migrate/20230604121856_add_index_to_judgements_on_user_and_query_doc_pair.rb new file mode 100644 index 000000000..856e7df97 --- /dev/null +++ b/db/migrate/20230604121856_add_index_to_judgements_on_user_and_query_doc_pair.rb @@ -0,0 +1,5 @@ +class AddIndexToJudgementsOnUserAndQueryDocPair < ActiveRecord::Migration[7.0] + def change + add_index :judgements, [:user_id, :query_doc_pair_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index bf1d861b7..31df87af0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_05_04_182246) do +ActiveRecord::Schema[7.0].define(version: 2023_06_04_121856) do create_table "annotations", id: :integer, charset: "utf8mb3", force: :cascade do |t| t.text "message" t.string "source" @@ -83,6 +83,7 @@ t.datetime "updated_at", null: false t.boolean "unrateable", default: false t.index ["query_doc_pair_id"], name: "index_judgements_on_query_doc_pair_id" + t.index ["user_id", "query_doc_pair_id"], name: "index_judgements_on_user_id_and_query_doc_pair_id", unique: true end create_table "permissions", id: :integer, charset: "utf8mb3", force: :cascade do |t| diff --git a/test/fixtures/judgements.yml b/test/fixtures/judgements.yml index 905107bb2..ceef8e6b5 100644 --- a/test/fixtures/judgements.yml +++ b/test/fixtures/judgements.yml @@ -12,7 +12,8 @@ # # Indexes # -# index_judgements_on_query_doc_pair_id (query_doc_pair_id) +# index_judgements_on_query_doc_pair_id (query_doc_pair_id) +# index_judgements_on_user_id_and_query_doc_pair_id (user_id,query_doc_pair_id) UNIQUE # # Foreign Keys # diff --git a/test/models/judgement_test.rb b/test/models/judgement_test.rb index fc5d865f4..1dc9ab47e 100644 --- a/test/models/judgement_test.rb +++ b/test/models/judgement_test.rb @@ -14,7 +14,8 @@ # # Indexes # -# index_judgements_on_query_doc_pair_id (query_doc_pair_id) +# index_judgements_on_query_doc_pair_id (query_doc_pair_id) +# index_judgements_on_user_id_and_query_doc_pair_id (user_id,query_doc_pair_id) UNIQUE # # Foreign Keys # From 34e4888e903d41ab229b3f7a30165210363c44ea Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Sun, 4 Jun 2023 13:32:42 -0400 Subject: [PATCH 06/38] fixes #739, Directly inheriting from ActiveRecord::Migration is not supported --- ...0110023326_fix_curator_variables_tries_foreign_key_name.rb | 2 +- db/migrate/20200110192642_fix_camel_case_in_user_table.rb | 2 +- db/migrate/20200110194733_fix_num_logins_in_users.rb | 2 +- db/migrate/20200111143938_rename_search_engine.rb | 2 +- db/migrate/20200113123124_rename_escape_query_in_tries.rb | 2 +- db/migrate/20200113153327_rename_search_url_in_tries.rb | 2 +- db/migrate/20200113154244_rename_search_url_in_cases.rb | 2 +- db/migrate/20200115011043_rename_field_spec_in_tries.rb | 2 +- db/migrate/20200115011742_rename_field_spec_in_cases.rb | 2 +- db/migrate/20200115015024_drop_unused_columns_from_cases.rb | 2 +- db/migrate/20200115020540_drop_display_position_from_cases.rb | 2 +- db/migrate/20200115021550_rename_query_params_in_tries.rb | 2 +- db/migrate/20200115131622_rename_try_no_in_tries.rb | 2 +- db/migrate/20200115152546_fix_case_name.rb | 2 +- db/migrate/20200115205420_fix_last_try_in_case.rb | 2 +- db/migrate/20200128133401_add_email_marketing_to_users.rb | 2 +- ...303190724_update_default_scorers_for_legacy_v1_behavior.rb | 2 +- db/migrate/20200313204611_drop_default_from_default_scorer.rb | 2 +- db/migrate/20200314193226_drop_communal_from_scorers.rb | 4 ++-- db/migrate/20200413184942_rename_users_username_to_email.rb | 2 +- ...20200428150211_update_demo_solr_es_to_non_default_ports.rb | 2 +- db/migrate/20200517162038_migrate_user_default_scorer.rb | 2 +- db/migrate/20200517162819_add_new_default_scorers.rb | 2 +- db/migrate/20200517164239_add_fk_to_scorers.rb | 2 +- db/migrate/20200522215022_remove_scorer_type.rb | 2 +- db/migrate/20200911180507_change_to_zero_to_three_grading.rb | 2 +- .../20200911192850_change_default_scorers_to_at_ten_deep.rb | 2 +- .../20201102142255_redo_communal_scorers_missing_proper_k.rb | 2 +- db/migrate/20201126010021_track_snapshot_type.rb | 2 +- db/migrate/20201203212611_bugfix_ap_ndcg_scorers.rb | 2 +- 30 files changed, 31 insertions(+), 31 deletions(-) diff --git a/db/migrate/20200110023326_fix_curator_variables_tries_foreign_key_name.rb b/db/migrate/20200110023326_fix_curator_variables_tries_foreign_key_name.rb index 91227142a..56145c612 100644 --- a/db/migrate/20200110023326_fix_curator_variables_tries_foreign_key_name.rb +++ b/db/migrate/20200110023326_fix_curator_variables_tries_foreign_key_name.rb @@ -1,4 +1,4 @@ -class FixCuratorVariablesTriesForeignKeyName < ActiveRecord::Migration +class FixCuratorVariablesTriesForeignKeyName < ActiveRecord::Migration[4.2] def change rename_column :curator_variables, :query_param_id, :try_id end diff --git a/db/migrate/20200110192642_fix_camel_case_in_user_table.rb b/db/migrate/20200110192642_fix_camel_case_in_user_table.rb index 91f4c7263..ade781b60 100644 --- a/db/migrate/20200110192642_fix_camel_case_in_user_table.rb +++ b/db/migrate/20200110192642_fix_camel_case_in_user_table.rb @@ -1,4 +1,4 @@ -class FixCamelCaseInUserTable < ActiveRecord::Migration +class FixCamelCaseInUserTable < ActiveRecord::Migration[4.2] def change rename_column :users, :firstLogin, :first_login end diff --git a/db/migrate/20200110194733_fix_num_logins_in_users.rb b/db/migrate/20200110194733_fix_num_logins_in_users.rb index 3fce41af0..e347cd6b9 100644 --- a/db/migrate/20200110194733_fix_num_logins_in_users.rb +++ b/db/migrate/20200110194733_fix_num_logins_in_users.rb @@ -1,4 +1,4 @@ -class FixNumLoginsInUsers < ActiveRecord::Migration +class FixNumLoginsInUsers < ActiveRecord::Migration[4.2] def change rename_column :users, :numLogins, :num_logins end diff --git a/db/migrate/20200111143938_rename_search_engine.rb b/db/migrate/20200111143938_rename_search_engine.rb index 6e109dedc..cb196cd1a 100644 --- a/db/migrate/20200111143938_rename_search_engine.rb +++ b/db/migrate/20200111143938_rename_search_engine.rb @@ -1,4 +1,4 @@ -class RenameSearchEngine < ActiveRecord::Migration +class RenameSearchEngine < ActiveRecord::Migration[4.2] def change rename_column :tries, :searchEngine, :search_engine end diff --git a/db/migrate/20200113123124_rename_escape_query_in_tries.rb b/db/migrate/20200113123124_rename_escape_query_in_tries.rb index f4a51155d..1c0c04694 100644 --- a/db/migrate/20200113123124_rename_escape_query_in_tries.rb +++ b/db/migrate/20200113123124_rename_escape_query_in_tries.rb @@ -1,4 +1,4 @@ -class RenameEscapeQueryInTries < ActiveRecord::Migration +class RenameEscapeQueryInTries < ActiveRecord::Migration[4.2] def change rename_column :tries, :escapeQuery, :escape_query end diff --git a/db/migrate/20200113153327_rename_search_url_in_tries.rb b/db/migrate/20200113153327_rename_search_url_in_tries.rb index fadd105d4..998f46b5f 100644 --- a/db/migrate/20200113153327_rename_search_url_in_tries.rb +++ b/db/migrate/20200113153327_rename_search_url_in_tries.rb @@ -1,4 +1,4 @@ -class RenameSearchUrlInTries < ActiveRecord::Migration +class RenameSearchUrlInTries < ActiveRecord::Migration[4.2] def change rename_column :tries, :searchUrl, :search_url end diff --git a/db/migrate/20200113154244_rename_search_url_in_cases.rb b/db/migrate/20200113154244_rename_search_url_in_cases.rb index 36e22646b..725cf26e8 100644 --- a/db/migrate/20200113154244_rename_search_url_in_cases.rb +++ b/db/migrate/20200113154244_rename_search_url_in_cases.rb @@ -1,4 +1,4 @@ -class RenameSearchUrlInCases < ActiveRecord::Migration +class RenameSearchUrlInCases < ActiveRecord::Migration[4.2] def change rename_column :cases, :searchUrl, :search_url end diff --git a/db/migrate/20200115011043_rename_field_spec_in_tries.rb b/db/migrate/20200115011043_rename_field_spec_in_tries.rb index f77cf0b9c..29c3a2032 100644 --- a/db/migrate/20200115011043_rename_field_spec_in_tries.rb +++ b/db/migrate/20200115011043_rename_field_spec_in_tries.rb @@ -1,4 +1,4 @@ -class RenameFieldSpecInTries < ActiveRecord::Migration +class RenameFieldSpecInTries < ActiveRecord::Migration[4.2] def change rename_column :tries, :fieldSpec, :field_spec end diff --git a/db/migrate/20200115011742_rename_field_spec_in_cases.rb b/db/migrate/20200115011742_rename_field_spec_in_cases.rb index 3b6625b89..20a35b7eb 100644 --- a/db/migrate/20200115011742_rename_field_spec_in_cases.rb +++ b/db/migrate/20200115011742_rename_field_spec_in_cases.rb @@ -1,4 +1,4 @@ -class RenameFieldSpecInCases < ActiveRecord::Migration +class RenameFieldSpecInCases < ActiveRecord::Migration[4.2] def change rename_column :cases, :fieldSpec, :field_spec end diff --git a/db/migrate/20200115015024_drop_unused_columns_from_cases.rb b/db/migrate/20200115015024_drop_unused_columns_from_cases.rb index eb12768b4..5a53e94cb 100644 --- a/db/migrate/20200115015024_drop_unused_columns_from_cases.rb +++ b/db/migrate/20200115015024_drop_unused_columns_from_cases.rb @@ -1,4 +1,4 @@ -class DropUnusedColumnsFromCases < ActiveRecord::Migration +class DropUnusedColumnsFromCases < ActiveRecord::Migration[4.2] def change remove_column :cases, :field_spec remove_column :cases, :search_url diff --git a/db/migrate/20200115020540_drop_display_position_from_cases.rb b/db/migrate/20200115020540_drop_display_position_from_cases.rb index fac737744..917a31529 100644 --- a/db/migrate/20200115020540_drop_display_position_from_cases.rb +++ b/db/migrate/20200115020540_drop_display_position_from_cases.rb @@ -1,4 +1,4 @@ -class DropDisplayPositionFromCases < ActiveRecord::Migration +class DropDisplayPositionFromCases < ActiveRecord::Migration[4.2] def change remove_column :cases, :displayPosition end diff --git a/db/migrate/20200115021550_rename_query_params_in_tries.rb b/db/migrate/20200115021550_rename_query_params_in_tries.rb index d2d9b8acc..aaaa84f70 100644 --- a/db/migrate/20200115021550_rename_query_params_in_tries.rb +++ b/db/migrate/20200115021550_rename_query_params_in_tries.rb @@ -1,4 +1,4 @@ -class RenameQueryParamsInTries < ActiveRecord::Migration +class RenameQueryParamsInTries < ActiveRecord::Migration[4.2] def change # Note, this failed once on werid data in queryParams, but manually running worked: # ALTER TABLE `tries` CHANGE `queryParams` `query_params` varchar(20000) DEFAULT NULL diff --git a/db/migrate/20200115131622_rename_try_no_in_tries.rb b/db/migrate/20200115131622_rename_try_no_in_tries.rb index ce90a3759..67a467070 100644 --- a/db/migrate/20200115131622_rename_try_no_in_tries.rb +++ b/db/migrate/20200115131622_rename_try_no_in_tries.rb @@ -1,4 +1,4 @@ -class RenameTryNoInTries < ActiveRecord::Migration +class RenameTryNoInTries < ActiveRecord::Migration[4.2] def change rename_column :tries, :tryNo, :try_number end diff --git a/db/migrate/20200115152546_fix_case_name.rb b/db/migrate/20200115152546_fix_case_name.rb index 04cf7a6a9..98e99d4a2 100644 --- a/db/migrate/20200115152546_fix_case_name.rb +++ b/db/migrate/20200115152546_fix_case_name.rb @@ -1,4 +1,4 @@ -class FixCaseName < ActiveRecord::Migration +class FixCaseName < ActiveRecord::Migration[4.2] def change rename_column :cases, :caseName, :case_name end diff --git a/db/migrate/20200115205420_fix_last_try_in_case.rb b/db/migrate/20200115205420_fix_last_try_in_case.rb index ca804a682..e26c86259 100644 --- a/db/migrate/20200115205420_fix_last_try_in_case.rb +++ b/db/migrate/20200115205420_fix_last_try_in_case.rb @@ -1,4 +1,4 @@ -class FixLastTryInCase < ActiveRecord::Migration +class FixLastTryInCase < ActiveRecord::Migration[4.2] def change rename_column :cases, :lastTry, :last_try_number end diff --git a/db/migrate/20200128133401_add_email_marketing_to_users.rb b/db/migrate/20200128133401_add_email_marketing_to_users.rb index 6800f90cf..c8f5585b7 100644 --- a/db/migrate/20200128133401_add_email_marketing_to_users.rb +++ b/db/migrate/20200128133401_add_email_marketing_to_users.rb @@ -1,4 +1,4 @@ -class AddEmailMarketingToUsers < ActiveRecord::Migration +class AddEmailMarketingToUsers < ActiveRecord::Migration[4.2] def change add_column :users, :email_marketing, :boolean, null: false, default: false end diff --git a/db/migrate/20200303190724_update_default_scorers_for_legacy_v1_behavior.rb b/db/migrate/20200303190724_update_default_scorers_for_legacy_v1_behavior.rb index ae8cb10af..3c9eb3620 100644 --- a/db/migrate/20200303190724_update_default_scorers_for_legacy_v1_behavior.rb +++ b/db/migrate/20200303190724_update_default_scorers_for_legacy_v1_behavior.rb @@ -1,4 +1,4 @@ -class UpdateDefaultScorersForLegacyV1Behavior < ActiveRecord::Migration +class UpdateDefaultScorersForLegacyV1Behavior < ActiveRecord::Migration[4.2] def up UpdateDefaultScorersForLegacyV1Behavior.connection.execute( "UPDATE default_scorers diff --git a/db/migrate/20200313204611_drop_default_from_default_scorer.rb b/db/migrate/20200313204611_drop_default_from_default_scorer.rb index 543aa5725..a822f717c 100644 --- a/db/migrate/20200313204611_drop_default_from_default_scorer.rb +++ b/db/migrate/20200313204611_drop_default_from_default_scorer.rb @@ -1,4 +1,4 @@ -class DropDefaultFromDefaultScorer < ActiveRecord::Migration +class DropDefaultFromDefaultScorer < ActiveRecord::Migration[4.2] def change remove_column :default_scorers, :default end diff --git a/db/migrate/20200314193226_drop_communal_from_scorers.rb b/db/migrate/20200314193226_drop_communal_from_scorers.rb index 40867e780..e1fe48e87 100644 --- a/db/migrate/20200314193226_drop_communal_from_scorers.rb +++ b/db/migrate/20200314193226_drop_communal_from_scorers.rb @@ -1,5 +1,5 @@ -class DropCommunalFromScorers < ActiveRecord::Migration +class DropCommunalFromScorers < ActiveRecord::Migration[4.2] def change - remove_column :scorers, :communal + remove_column :scorers, :communal end end diff --git a/db/migrate/20200413184942_rename_users_username_to_email.rb b/db/migrate/20200413184942_rename_users_username_to_email.rb index 8fed2465e..7771a5ce9 100644 --- a/db/migrate/20200413184942_rename_users_username_to_email.rb +++ b/db/migrate/20200413184942_rename_users_username_to_email.rb @@ -1,4 +1,4 @@ -class RenameUsersUsernameToEmail < ActiveRecord::Migration +class RenameUsersUsernameToEmail < ActiveRecord::Migration[4.2] def change rename_column :users, :username, :email end diff --git a/db/migrate/20200428150211_update_demo_solr_es_to_non_default_ports.rb b/db/migrate/20200428150211_update_demo_solr_es_to_non_default_ports.rb index 61cd20386..223b60370 100644 --- a/db/migrate/20200428150211_update_demo_solr_es_to_non_default_ports.rb +++ b/db/migrate/20200428150211_update_demo_solr_es_to_non_default_ports.rb @@ -1,4 +1,4 @@ -class UpdateDemoSolrEsToNonDefaultPorts < ActiveRecord::Migration +class UpdateDemoSolrEsToNonDefaultPorts < ActiveRecord::Migration[4.2] # Our demo Solr and ES instances were getting drive by vandalism by running on # default ports on the Internet. So we moved to non defaults. Update existing # cases. https://github.com/o19s/quepid/issues/104 diff --git a/db/migrate/20200517162038_migrate_user_default_scorer.rb b/db/migrate/20200517162038_migrate_user_default_scorer.rb index a3238e096..adad24524 100644 --- a/db/migrate/20200517162038_migrate_user_default_scorer.rb +++ b/db/migrate/20200517162038_migrate_user_default_scorer.rb @@ -1,4 +1,4 @@ -class MigrateUserDefaultScorer < ActiveRecord::Migration +class MigrateUserDefaultScorer < ActiveRecord::Migration[4.2] def change if foreign_keys("users").any?{|k| k[:to_table] == "default_scorers"} diff --git a/db/migrate/20200517162819_add_new_default_scorers.rb b/db/migrate/20200517162819_add_new_default_scorers.rb index c9f73900d..002c7916c 100644 --- a/db/migrate/20200517162819_add_new_default_scorers.rb +++ b/db/migrate/20200517162819_add_new_default_scorers.rb @@ -1,4 +1,4 @@ -class AddNewDefaultScorers < ActiveRecord::Migration +class AddNewDefaultScorers < ActiveRecord::Migration[4.2] def change Scorer.where(name: 'v1').first_or_create( diff --git a/db/migrate/20200517164239_add_fk_to_scorers.rb b/db/migrate/20200517164239_add_fk_to_scorers.rb index 8ea987c5e..cf30bcda1 100644 --- a/db/migrate/20200517164239_add_fk_to_scorers.rb +++ b/db/migrate/20200517164239_add_fk_to_scorers.rb @@ -1,4 +1,4 @@ -class AddFkToScorers < ActiveRecord::Migration +class AddFkToScorers < ActiveRecord::Migration[4.2] def change remove_column :users, :scorer_id diff --git a/db/migrate/20200522215022_remove_scorer_type.rb b/db/migrate/20200522215022_remove_scorer_type.rb index 20649a499..5e82d3551 100644 --- a/db/migrate/20200522215022_remove_scorer_type.rb +++ b/db/migrate/20200522215022_remove_scorer_type.rb @@ -1,4 +1,4 @@ -class RemoveScorerType < ActiveRecord::Migration +class RemoveScorerType < ActiveRecord::Migration[4.2] # Note: we are leaving the default_scorer table alone for now in case # we need to bring it back, we don't want to lose the data. Just in case.. def change diff --git a/db/migrate/20200911180507_change_to_zero_to_three_grading.rb b/db/migrate/20200911180507_change_to_zero_to_three_grading.rb index 64446e7df..bb56fe072 100644 --- a/db/migrate/20200911180507_change_to_zero_to_three_grading.rb +++ b/db/migrate/20200911180507_change_to_zero_to_three_grading.rb @@ -1,4 +1,4 @@ -class ChangeToZeroToThreeGrading < ActiveRecord::Migration +class ChangeToZeroToThreeGrading < ActiveRecord::Migration[4.2] def change scale_with_labels = {"0":"Poor","1":"Fair","2":"Good","3":"Perfect"} diff --git a/db/migrate/20200911192850_change_default_scorers_to_at_ten_deep.rb b/db/migrate/20200911192850_change_default_scorers_to_at_ten_deep.rb index 962c117df..796c8ac8e 100644 --- a/db/migrate/20200911192850_change_default_scorers_to_at_ten_deep.rb +++ b/db/migrate/20200911192850_change_default_scorers_to_at_ten_deep.rb @@ -1,4 +1,4 @@ -class ChangeDefaultScorersToAtTenDeep < ActiveRecord::Migration +class ChangeDefaultScorersToAtTenDeep < ActiveRecord::Migration[4.2] def change scorers_to_update = ['P@5', 'AP@5','nDCG@5', 'DCG@5', 'CG@5'] diff --git a/db/migrate/20201102142255_redo_communal_scorers_missing_proper_k.rb b/db/migrate/20201102142255_redo_communal_scorers_missing_proper_k.rb index 63be2b396..da417bbc9 100644 --- a/db/migrate/20201102142255_redo_communal_scorers_missing_proper_k.rb +++ b/db/migrate/20201102142255_redo_communal_scorers_missing_proper_k.rb @@ -1,6 +1,6 @@ # It appears that we still have k=5, even through the scores are labeled as @10, so fix this. # caused by the previous scorer had an errant "," after the scorer.code line -class RedoCommunalScorersMissingProperK < ActiveRecord::Migration +class RedoCommunalScorersMissingProperK < ActiveRecord::Migration[4.2] def change scorers_to_update = ['P@10', 'AP@10','nDCG@10', 'DCG@10', 'CG@10'] diff --git a/db/migrate/20201126010021_track_snapshot_type.rb b/db/migrate/20201126010021_track_snapshot_type.rb index a9e2b0820..b26abac7d 100644 --- a/db/migrate/20201126010021_track_snapshot_type.rb +++ b/db/migrate/20201126010021_track_snapshot_type.rb @@ -2,7 +2,7 @@ # have so much data that the migration fails. For those, we contact tech support # and have them run the migration, and then just do a # INSERT INTO schema_migrations VALUES ('20201126010021') to fake out Rails -class TrackSnapshotType < ActiveRecord::Migration +class TrackSnapshotType < ActiveRecord::Migration[4.2] def change add_column :snapshot_docs, :rated_only, :boolean, default: false end diff --git a/db/migrate/20201203212611_bugfix_ap_ndcg_scorers.rb b/db/migrate/20201203212611_bugfix_ap_ndcg_scorers.rb index 5b3a7a84c..84cb36c32 100644 --- a/db/migrate/20201203212611_bugfix_ap_ndcg_scorers.rb +++ b/db/migrate/20201203212611_bugfix_ap_ndcg_scorers.rb @@ -1,5 +1,5 @@ # See https://github.com/o19s/quepid/issues/225 -class BugfixApNdcgScorers < ActiveRecord::Migration +class BugfixApNdcgScorers < ActiveRecord::Migration[4.2] def change scorers_to_update = ['AP@10','nDCG@10'] scorers_to_update.each do |scorer_name| From 9eb017f125e37ec3771e6ae56780ffc057d62131 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Wed, 7 Jun 2023 13:26:26 -0400 Subject: [PATCH 07/38] be able to force error messages to the surface (#748) --- config/environments/production.rb | 4 +++- docs/operating_documentation.md | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/config/environments/production.rb b/config/environments/production.rb index 6f276d4d6..cb9ef57de 100755 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -15,7 +15,9 @@ config.eager_load = true # Full error reports are disabled and caching is turned on. - config.consider_all_requests_local = false + # Often Quepid production setup is deployed in a customer site as a "dev", and then promoted to "prod" + # Let's be able to see error messages. + config.consider_all_requests_local = ENV['QUEPID_CONSIDER_ALL_REQUESTS_LOCAL'].present? config.action_controller.perform_caching = true # Ensures that a master key has been made available in either ENV["RAILS_MASTER_KEY"] diff --git a/docs/operating_documentation.md b/docs/operating_documentation.md index b077dc451..a2d82ee1d 100644 --- a/docs/operating_documentation.md +++ b/docs/operating_documentation.md @@ -9,6 +9,7 @@ This document explains how Quepid can be operated and configured. - [Legal Pages & GDPR](#legal-pages-&-gdpr) - [User Tracking](#user-tracking) - [Heathcheck Endpoint](#healthcheck) +- [Troubleshoot Your Deploy](#troubleshoot-your-deploy) - [Database Management](#database-management) - [Jupyterlite Notebooks](#jupyterlite-notebooks) ## Running behind a load balancer @@ -172,6 +173,17 @@ the file `Procfile` Want to monitor if Quepid is behaving? Just monitor `/healthcheck`, and you will get 200 status codes from a healthy Quepid, and 503 if not. The JSON output is `{"code":200,"status":{"database":"OK","migrations":"OK"}}`. +## Troubleshoot Your Deploy + +When errors occur, Quepid logs them and shows a generic page. +However sometimes getting to those logs is difficult, and you just want the message immediately. + +You can enable this behavior by setting the follow `ENV` var: + +``` +QUEPID_CONSIDER_ALL_REQUESTS_LOCAL=true +``` + ## Database Management See the details in [](./database.md). From 5027936e3490cc44306417f99a891b6175941262 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Wed, 7 Jun 2023 13:35:52 -0400 Subject: [PATCH 08/38] Be able to see errors (#750) * be able to force error messages to the surface * Slim down production image by not including notebook dir twice, once as files and again as a tar.gz file. * finalize this troubleshooting tool * lint --- Dockerfile.prod | 8 +++++++- app/controllers/api/api_controller.rb | 7 ++++++- config/routes.rb | 1 + docs/operating_documentation.md | 2 ++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/Dockerfile.prod b/Dockerfile.prod index 0de65fbc4..a225f7362 100644 --- a/Dockerfile.prod +++ b/Dockerfile.prod @@ -57,7 +57,13 @@ COPY ./LICENSE ./LICENSE COPY ./package.json ./package.json COPY ./postcss.config.js ./postcss.config.js COPY ./Procfile ./Procfile -COPY ./public ./public + +# Exclude the /public/notebook directory +COPY ./public/images ./public/images +COPY ./public/javascripts ./public/javascripts +COPY ./public/*.html ./public +COPY ./public/robots.txt ./public + COPY ./Rakefile ./Rakefile COPY ./README.md ./README.md COPY ./vendor ./vendor diff --git a/app/controllers/api/api_controller.rb b/app/controllers/api/api_controller.rb index bafffbcee..57173fa4f 100644 --- a/app/controllers/api/api_controller.rb +++ b/app/controllers/api/api_controller.rb @@ -17,7 +17,7 @@ class ApiController < ActionController::Base before_action :set_default_response_format before_action :set_current_user before_action :check_current_user_locked! - before_action :authenticate_api! + before_action :authenticate_api!, except: [ :test, :test_exception ] protect_from_forgery with: :null_session @@ -28,6 +28,11 @@ def test render json: { message: 'Success!' }, status: :ok end + # Use to test that exception are rendered properly. + def test_exception + raise 'boom' + end + def signup_enabled? Rails.application.config.signup_enabled end diff --git a/config/routes.rb b/config/routes.rb index 54abcae19..0a8a0764b 100755 --- a/config/routes.rb +++ b/config/routes.rb @@ -74,6 +74,7 @@ namespace :api, defaults: { format: :json } do get 'test' => 'api#test' + get 'test_exception' => 'api#test_exception' scope module: :v1, constraints: ApiConstraint.new(version: 1, default: true) do resources :users, only: [ :index, :show, :update ] do diff --git a/docs/operating_documentation.md b/docs/operating_documentation.md index a2d82ee1d..3f2741960 100644 --- a/docs/operating_documentation.md +++ b/docs/operating_documentation.md @@ -184,6 +184,8 @@ You can enable this behavior by setting the follow `ENV` var: QUEPID_CONSIDER_ALL_REQUESTS_LOCAL=true ``` +Confirm the setup by visiting `/api/test_exception` which raises an error and will give you the debugging page "RuntimeError in Api::ApiController#test_exception". + ## Database Management See the details in [](./database.md). From 7b5d410d7a249c2f44e2fce9e5cdd9c773f35c30 Mon Sep 17 00:00:00 2001 From: Chris Morley Date: Thu, 8 Jun 2023 09:08:45 -0400 Subject: [PATCH 09/38] add keyboard shortcuts for a 0,1,2 custom scorer (#752) --- app/views/judgements/_form.html.erb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/views/judgements/_form.html.erb b/app/views/judgements/_form.html.erb index 49d3a2c9c..09a841c8e 100644 --- a/app/views/judgements/_form.html.erb +++ b/app/views/judgements/_form.html.erb @@ -205,6 +205,17 @@ return {"name": "f", "display_name": "f" } end end + + if max_value == 2 + if value == 0 + return {"name": "s", "display_name": "s" } + elsif value == 1 + return {"name": "d", "display_name": "d" } + else + return {"name": "f", "display_name": "f" } + end + end + if max_value == 3 if value == 0 return {"name": "a", "display_name": "a" } From fdf29d0bfb302ee6f63fcb0dcc37c105d1f017e0 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Thu, 8 Jun 2023 09:10:00 -0400 Subject: [PATCH 10/38] Embrace new template capablities (#751) * fix test that assumes /test is under auth * this pattern doesnt wipe out all the other files * Now checking existence of template off of the queryParams. * lint * slap in templates rendering * make sure we the $log * bump to new splainer-search * lint --- .../components/query_explain/_modal.html | 19 +++++++++--- ...query_explain_modal_instance_controller.js | 29 +++++++++++++++---- .../javascripts/controllers/queryParams.js | 9 ++++-- app/controllers/api/api_controller.rb | 2 +- docker-compose.override.yml.example | 3 +- package.json | 2 +- yarn.lock | 8 ++--- 7 files changed, 53 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/components/query_explain/_modal.html b/app/assets/javascripts/components/query_explain/_modal.html index fbb3c7890..73e577244 100644 --- a/app/assets/javascripts/components/query_explain/_modal.html +++ b/app/assets/javascripts/components/query_explain/_modal.html @@ -7,7 +7,7 @@

Explain Query Parsing

These are details returned from the search engine on how it processed the input query

- +

{{ctrl.queryDetailsMessage}} @@ -20,18 +20,29 @@

Explain Query Parsing

 - +

This is how the search engine parsed the query.

 + +

This is not a templated query.

+ +

This is what the populated query template looks like

+ + + +
- - + + +
diff --git a/app/assets/javascripts/components/query_explain/query_explain_modal_instance_controller.js b/app/assets/javascripts/components/query_explain/query_explain_modal_instance_controller.js index 3340a7f4e..851b9169f 100644 --- a/app/assets/javascripts/components/query_explain/query_explain_modal_instance_controller.js +++ b/app/assets/javascripts/components/query_explain/query_explain_modal_instance_controller.js @@ -3,21 +3,28 @@ angular.module('QuepidApp') .controller('QueryExplainModalInstanceCtrl', [ '$uibModalInstance', + '$log', 'query', function ( $uibModalInstance, + $log, query ) { - var ctrl = this; + let ctrl = this; - // default to showing the params toggle. - ctrl.toggleParams = true; + // default to showing the params panels. + ctrl.toggledPanel = 'queryDetails'; + + ctrl.togglePanel = function(panel) { + ctrl.toggledPanel = panel; + }; ctrl.query = query; + ctrl.isTemplatedQuery = false; ctrl.sortJsonByKeys = function (obj) { - var sortedJsonKeys = Object.keys(obj).sort(); - var tempObj = {}; + let sortedJsonKeys = Object.keys(obj).sort(); + let tempObj = {}; sortedJsonKeys.map(key => tempObj[key] = obj[key]); return angular.toJson(tempObj, true); }; @@ -37,7 +44,17 @@ angular.module('QuepidApp') else { ctrl.queryDetailsMessage = 'Query parameters are not returned by the current Search Engine.'; } - + + ctrl.renderQueryTemplate = function(){ + ctrl.isTemplatedQuery = query.searcher.isTemplateCall(query.searcher.args); + + query.searcher.renderTemplate().then(function() { + ctrl.renderedQueryTemplate = query.searcher.renderedTemplateJson; + ctrl.togglePanel('renderedQueryTemplate'); + }, function(response) { + $log.debug(response.data); + }); + }; ctrl.cancel = function () { $uibModalInstance.dismiss('cancel'); }; diff --git a/app/assets/javascripts/controllers/queryParams.js b/app/assets/javascripts/controllers/queryParams.js index c833d561c..cb3b6c683 100644 --- a/app/assets/javascripts/controllers/queryParams.js +++ b/app/assets/javascripts/controllers/queryParams.js @@ -25,8 +25,13 @@ angular.module('QuepidApp') $scope.validateSearchEngineUrl = function() { if (!angular.isUndefined($scope.settings.searchUrl)){ if ($scope.settings.searchEngine === 'es' || $scope.settings.searchEngine === 'os'){ - var uri = esUrlSvc.parseUrl($scope.settings.searchUrl); - $scope.showESTemplateWarning = esUrlSvc.isTemplateCall(uri); + try { + const args = JSON.parse($scope.settings.queryParams); + $scope.showESTemplateWarning = esUrlSvc.isTemplateCall(args); + } + catch (error){ + // Ignore if we don't have valid JSON in queryParams. + } } if ($scope.settings.searchEngine !== '' && !angular.isUndefined($scope.settings.searchUrl)){ diff --git a/app/controllers/api/api_controller.rb b/app/controllers/api/api_controller.rb index 57173fa4f..b73b13a2c 100644 --- a/app/controllers/api/api_controller.rb +++ b/app/controllers/api/api_controller.rb @@ -17,7 +17,7 @@ class ApiController < ActionController::Base before_action :set_default_response_format before_action :set_current_user before_action :check_current_user_locked! - before_action :authenticate_api!, except: [ :test, :test_exception ] + before_action :authenticate_api!, except: [ :test_exception ] protect_from_forgery with: :null_session diff --git a/docker-compose.override.yml.example b/docker-compose.override.yml.example index 35c2f5823..2133b8809 100644 --- a/docker-compose.override.yml.example +++ b/docker-compose.override.yml.example @@ -5,4 +5,5 @@ version: '3' services: app: volumes: - - /Users/epugh/Documents/projects/splainer-search/:/srv/app/node_modules/splainer-search + - /Users/epugh/Documents/projects/splainer-search/package.json:/srv/app/node_modules/splainer-search/package.json + - /Users/epugh/Documents/projects/splainer-search/splainer-search.js:/srv/app/node_modules/splainer-search/splainer-search.js diff --git a/package.json b/package.json index 98971689b..3509b0f49 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "ng-tags-input": "3.2.0", "ngclipboard": "^2.0.0", "popper.js": "^1.16.1", - "splainer-search": "2.21.0", + "splainer-search": "2.22.0", "tether-shepherd": "latest", "turbolinks": "^5.2.0", "vega": "5.24.0" diff --git a/yarn.lock b/yarn.lock index ad02c60e5..cf3b49ab3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1857,10 +1857,10 @@ spark-md5@^3.0.1: resolved "https://registry.yarnpkg.com/spark-md5/-/spark-md5-3.0.2.tgz#7952c4a30784347abcee73268e473b9c0167e3fc" integrity sha512-wcFzz9cDfbuqe0FZzfi2or1sgyIrsDwmPwfZC4hiNidPdPINjeUwNfv5kldczoEAcjl9Y1L3SM7Uz2PUEQzxQw== -splainer-search@2.21.0: - version "2.21.0" - resolved "https://registry.yarnpkg.com/splainer-search/-/splainer-search-2.21.0.tgz#9767ae4c061255d3db76c2c9dab0bd53f23455e3" - integrity sha512-Zu1IHCSZRVnvvj8n5RQaEzOBTVOwmXH0InQVPSstrczXrBYHmHdJAlzh8lq5DCwnAoMD+I52Sovihzy6R1fACg== +splainer-search@2.22.0: + version "2.22.0" + resolved "https://registry.yarnpkg.com/splainer-search/-/splainer-search-2.22.0.tgz#f1f425961ba7069b6fc9cfc9316150c7e53900ec" + integrity sha512-3KPzjH7NQHrcY3FXIfIYNoWd5pakiw7cBGH0OVqTXzxNzoZu8R0aRKmyYryO1keokaGDuOnzd+Is8RgcQ+pNmA== dependencies: angular "1.8.3" urijs "^1.19.7" From 48f0e2f33b9ef846bfe75621e29a22480aa06c26 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Fri, 9 Jun 2023 14:22:23 -0400 Subject: [PATCH 11/38] Fix missing docs dialogue with templates (#753) The explain logic doesn't appear to work with ES/OS templates, at least, wihtout extracting the query FROM the template and then submitting it! --- .../components/query_explain/_modal.html | 14 +++++++----- ...query_explain_modal_instance_controller.js | 11 +++++----- .../javascripts/controllers/docFinder.js | 22 +++++++++++++++---- app/controllers/judgements_controller.rb | 2 +- package.json | 2 +- yarn.lock | 8 +++---- 6 files changed, 39 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/components/query_explain/_modal.html b/app/assets/javascripts/components/query_explain/_modal.html index 73e577244..0a0c45235 100644 --- a/app/assets/javascripts/components/query_explain/_modal.html +++ b/app/assets/javascripts/components/query_explain/_modal.html @@ -31,12 +31,16 @@

Explain Query Parsing

This is not a templated query.

This is what the populated query template looks like

- - +
+
- + diff --git a/app/assets/javascripts/components/query_explain/query_explain_modal_instance_controller.js b/app/assets/javascripts/components/query_explain/query_explain_modal_instance_controller.js index 851b9169f..f240fd76d 100644 --- a/app/assets/javascripts/components/query_explain/query_explain_modal_instance_controller.js +++ b/app/assets/javascripts/components/query_explain/query_explain_modal_instance_controller.js @@ -14,7 +14,7 @@ angular.module('QuepidApp') // default to showing the params panels. ctrl.toggledPanel = 'queryDetails'; - + ctrl.togglePanel = function(panel) { ctrl.toggledPanel = panel; }; @@ -44,17 +44,18 @@ angular.module('QuepidApp') else { ctrl.queryDetailsMessage = 'Query parameters are not returned by the current Search Engine.'; } - + ctrl.renderQueryTemplate = function(){ - ctrl.isTemplatedQuery = query.searcher.isTemplateCall(query.searcher.args); - + ctrl.isTemplatedQuery = query.searcher.isTemplateCall(query.searcher.args); + query.searcher.renderTemplate().then(function() { - ctrl.renderedQueryTemplate = query.searcher.renderedTemplateJson; + ctrl.renderedQueryTemplate = angular.toJson(query.searcher.renderedTemplateJson.template_output,true); ctrl.togglePanel('renderedQueryTemplate'); }, function(response) { $log.debug(response.data); }); }; + ctrl.cancel = function () { $uibModalInstance.dismiss('cancel'); }; diff --git a/app/assets/javascripts/controllers/docFinder.js b/app/assets/javascripts/controllers/docFinder.js index c34f3e76c..277117493 100644 --- a/app/assets/javascripts/controllers/docFinder.js +++ b/app/assets/javascripts/controllers/docFinder.js @@ -192,14 +192,28 @@ angular.module('QuepidApp') var filter = { 'query': $scope.query.filterToRatings(currSettings, $scope.docFinder.docs.length) }; - $scope.docFinder.searcher.explainOther( - filter, fieldSpec) - .then(function() { + if($scope.docFinder.searcher.isTemplateCall($scope.docFinder.searcher.args)){ + // Do a normal search if it's a templated call as we can't get the explain. + delete $scope.docFinder.searcher.args.id; + delete $scope.docFinder.searcher.args.params; + $scope.docFinder.searcher.queryDsl = filter; // is this terrible? + $scope.docFinder.searcher.search(filter).then(function(){ var normed = queriesSvc.normalizeDocExplains($scope.query, $scope.docFinder.searcher, fieldSpec); $scope.docFinder.docs = normed; $scope.defaultList = true; - }); + }); + } + else { + $scope.docFinder.searcher.explainOther( + filter, fieldSpec) + .then(function() { + let normed = queriesSvc.normalizeDocExplains($scope.query, $scope.docFinder.searcher, fieldSpec); + $scope.docFinder.docs = normed; + + $scope.defaultList = true; + }); + } } else if ($scope.docFinder.searcher.type === 'solr') { $scope.docFinder.searcher.explainOther( diff --git a/app/controllers/judgements_controller.rb b/app/controllers/judgements_controller.rb index c53fb1ee9..7a124b344 100644 --- a/app/controllers/judgements_controller.rb +++ b/app/controllers/judgements_controller.rb @@ -37,7 +37,7 @@ def create if @judgement.save redirect_to book_judge_path(@book) else - render action: :edit + render action: :new end end diff --git a/package.json b/package.json index 3509b0f49..3adc494b9 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "ng-tags-input": "3.2.0", "ngclipboard": "^2.0.0", "popper.js": "^1.16.1", - "splainer-search": "2.22.0", + "splainer-search": "2.22.2", "tether-shepherd": "latest", "turbolinks": "^5.2.0", "vega": "5.24.0" diff --git a/yarn.lock b/yarn.lock index cf3b49ab3..1c4a4ecb7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1857,10 +1857,10 @@ spark-md5@^3.0.1: resolved "https://registry.yarnpkg.com/spark-md5/-/spark-md5-3.0.2.tgz#7952c4a30784347abcee73268e473b9c0167e3fc" integrity sha512-wcFzz9cDfbuqe0FZzfi2or1sgyIrsDwmPwfZC4hiNidPdPINjeUwNfv5kldczoEAcjl9Y1L3SM7Uz2PUEQzxQw== -splainer-search@2.22.0: - version "2.22.0" - resolved "https://registry.yarnpkg.com/splainer-search/-/splainer-search-2.22.0.tgz#f1f425961ba7069b6fc9cfc9316150c7e53900ec" - integrity sha512-3KPzjH7NQHrcY3FXIfIYNoWd5pakiw7cBGH0OVqTXzxNzoZu8R0aRKmyYryO1keokaGDuOnzd+Is8RgcQ+pNmA== +splainer-search@2.22.2: + version "2.22.2" + resolved "https://registry.yarnpkg.com/splainer-search/-/splainer-search-2.22.2.tgz#ef790d36c2bfd2e1b3dbf06f2527fffc76631f4a" + integrity sha512-ix+dT5arjN03jTVGG9EjvaOsNDzM2xGHWWoM2CFR9q0wNgrOUWgmAUcrWgvynA39Hcl8eEvifT66XfsqCba3Dg== dependencies: angular "1.8.3" urijs "^1.19.7" From b0ed203bd8339ded162df619f832adf09312fc47 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Fri, 9 Jun 2023 14:35:05 -0400 Subject: [PATCH 12/38] use relative paths so when quepid behind proxies etc... (#754) --- app/views/analytics/tries_visualization/_tree.json.jbuilder | 2 +- app/views/analytics/tries_visualization/show.html.erb | 2 +- .../analytics/tries_visualization/vega_specification.json.erb | 2 +- app/views/books/show.html.erb | 4 ++-- app/views/judgements/edit.html.erb | 2 +- app/views/judgements/new.html.erb | 2 +- app/views/judgements/show.html.erb | 2 +- app/views/layouts/analytics.html.erb | 2 +- app/views/query_doc_pairs/edit.html.erb | 4 ++-- app/views/query_doc_pairs/index.html.erb | 2 +- app/views/query_doc_pairs/new.html.erb | 2 +- app/views/query_doc_pairs/show.html.erb | 2 +- 12 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/views/analytics/tries_visualization/_tree.json.jbuilder b/app/views/analytics/tries_visualization/_tree.json.jbuilder index 05f0fcb50..cd7b12659 100644 --- a/app/views/analytics/tries_visualization/_tree.json.jbuilder +++ b/app/views/analytics/tries_visualization/_tree.json.jbuilder @@ -4,5 +4,5 @@ json.id try.id json.name try.name json.parent try.parent_id if try.parent json.size 10 -json.url case_core_url(id: @case.id, try_number: try.try_number) +json.url case_core_path(id: @case.id, try_number: try.try_number) json.query_params try.query_params diff --git a/app/views/analytics/tries_visualization/show.html.erb b/app/views/analytics/tries_visualization/show.html.erb index f0c3a8e71..2f6eeab76 100644 --- a/app/views/analytics/tries_visualization/show.html.erb +++ b/app/views/analytics/tries_visualization/show.html.erb @@ -4,7 +4,7 @@