Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix erroneous media search #593

Merged
merged 11 commits into from
Feb 16, 2024
1 change: 1 addition & 0 deletions app/assets/javascripts/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,4 @@
//= require vertices
//= require watchlists
//= require turbolinks
//= require search_tags
19 changes: 19 additions & 0 deletions app/assets/javascripts/search_tags.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
$(document).on("turbolinks:load", function () {
$("#search_all_tags").change(evt => toggleSearchAllTags(evt));
});

/**
* Dynamically enable/disable the OR/AND buttons in the media search form.
* If the user has decided to search for media regardless of the tags,
* i.e. they enable the "all" (tags) button, we disable the "OR/AND" buttons
* as it is pointless to search for media that references *all* available tags
* at once.
*/
function toggleSearchAllTags(evt) {
const searchAllTags = evt.target.checked;
if (searchAllTags) {
$("#search_tag_operator_or").prop("checked", true);
}
$("#search_tag_operator_or").prop("disabled", searchAllTags);
$("#search_tag_operator_and").prop("disabled", searchAllTags);
}
15 changes: 0 additions & 15 deletions app/controllers/media_controller.rb
Splines marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -228,21 +228,6 @@ def search
results = search.results
@total = search.total

# in the case of a search with tag_operator 'or', we
# execute two searches and merge the results, where media
# with the selected tags are now shown at the front of the list
if (search_params[:tag_operator] == "or") \
&& (search_params[:all_tags] == "0") \
&& (search_params[:fulltext].size >= 2)
params["search"]["all_tags"] = "1"
search_no_tags = Medium.search_by(search_params, params[:page])
search_no_tags.execute
results_no_tags = search_no_tags.results
results = (results + results_no_tags).uniq
@total = results.size
params["search"]["all_tags"] = "0"
end
Splines marked this conversation as resolved.
Show resolved Hide resolved

if filter_media
search_arel = Medium.where(id: results.pluck(:id))
visible_search_results = current_user.filter_visible_media(search_arel)
Expand Down
16 changes: 5 additions & 11 deletions app/models/medium.rb
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,6 @@ def self.search_by(search_params, _page)
search_params[:all_terms] = "1" if search_params[:all_terms].blank?
search_params[:all_teachers] = "1" if search_params[:all_teachers].blank?
search_params[:term_ids].push("0") if search_params[:term_ids].present?
if search_params[:all_tags] == "1" && search_params[:tag_operator] == "and"
search_params[:tag_ids] = Tag.pluck(:id)
end
user = User.find_by(id: search_params[:user_id])
search = Sunspot.new_search(Medium)
search.build do
Expand Down Expand Up @@ -336,15 +333,12 @@ def self.search_by(search_params, _page)
with(:release_state, search_params[:access])
end
end
if !search_params[:all_tags] == "1" &&
!search_params[:tag_operator] == "or" && (search_params[:tag_ids])
if search_params[:tag_operator] == "or" || search_params[:all_tags] == "1"
fosterfarrell9 marked this conversation as resolved.
Show resolved Hide resolved
search.build do
with(:tag_ids).any_of(search_params[:tag_ids])
end
else
search.build do
if search_params[:all_tags] == "0" && search_params[:tag_ids].any?
search.build do
if search_params[:tag_operator] == "and"
with(:tag_ids).all_of(search_params[:tag_ids])
else
with(:tag_ids).any_of(search_params[:tag_ids])
end
end
end
Expand Down
6 changes: 4 additions & 2 deletions app/views/main/start/_media_search.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@
<%= f.radio_button :tag_operator,
'or',
checked: true,
class: 'form-check-input' %>
class: 'form-check-input',
disabled: true %>
<%= f.label :tag_operator,
t('basics.OR'),
value: 'or',
Expand All @@ -71,7 +72,8 @@
<div class="form-check form-check-inline">
<%= f.radio_button :tag_operator,
'and',
class: 'form-check-input' %>
class: 'form-check-input',
disabled: true %>
<%= f.label :tag_operator,
t('basics.AND'),
value: 'and',
Expand Down
6 changes: 4 additions & 2 deletions app/views/media/catalog/_search_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@
<%= f.radio_button :tag_operator,
'or',
checked: true,
class: 'form-check-input' %>
class: 'form-check-input',
disabled: true %>
<%= f.label :tag_operator,
t('basics.OR'),
value: 'or',
Expand All @@ -114,7 +115,8 @@
<div class="form-check form-check-inline">
<%= f.radio_button :tag_operator,
'and',
class: 'form-check-input' %>
class: 'form-check-input',
disabled: true %>
<%= f.label :tag_operator,
t('basics.AND'),
value: 'and',
Expand Down