Fix erroneous media search (#593)

* Warn about too long GitHub commit messages (#586)

* Fix comment status (#585)

* Reapply first fix for Reader/Media

See discussion on #574 for further details.
Previous PR for this was #576, closed in favor of this one
as this directly branches off the new "dev" branch.

* Correctly show latest post (might be current_user's comment)

* Fix update of unread comments logic in comments controller

* Fix update icon logic and latest post comment

* Simplify latest comment logic

* Improve code comments

* Further improve comments

* Fix wording in comment

* Fix construction of media array & use `.blank?` instead of `.empty?`

* Migrate `unread_comments` flag (fix inconsistencies) (#587)

* Add dummy migration

* Implement migration for unread comment flag

* Remove unnecessary comment

* Declare migration as not idempotent

* Use array.length instead of counting

* Throw error to prevent revert of migration

* Fix severe flaws in unread comments migration

* Simplify Reader retrieval

* Use the more explicit `.nil?` method

* Update migration date

* Fix annoying bug: don't use `.select!` but `.select`

* Polish migration

e.g. update comment, more suitable name for the method etc.

* Rename method according to #585

* Use `warn` log level for migration (#588)

* Rename get_statistics.coffee file to statistics.coffee (#591)

This is to reflect the corresponding renaming of the route
due to rubocop.

* fix behaviour of media search

* Disable OR/AND buttons if "all" tags is enabled

(inside the media search, right now only frontend change)

* Use ids autogenerated by Rails

This is such that we can still click on the label to select
the respective radio button, instead of only being able
to click on the radio button itself.

* Disable AND/OR initially (until user deselects "all tags")

* Don't restrict media search if "all" tags is enabled.

If the "all tags" option is enabled in the search, we should allow the
results to include *any* tag. This is automatically the case if we just
don't add any restriction regarding the tag ids in the SOLR search.

This change accompanies the frontend change to disable the AND/OR
buttons if the "all" button is selected meaning the user wants to search
for all tags. See #593 for more details.

---------

Co-authored-by: Splines <[email protected]>
Co-authored-by: Splines <[email protected]>

app/assets/javascripts/application.js

@@ -57,3 +57,4 @@
 //= require vertices
 //= require watchlists
 //= require turbolinks
+//= require search_tags

app/assets/javascripts/search_tags.js

@@ -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);
+}

app/controllers/media_controller.rb

@@ -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
-
     if filter_media
       search_arel = Medium.where(id: results.pluck(:id))
       visible_search_results = current_user.filter_visible_media(search_arel)

app/models/medium.rb

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

app/views/main/start/_media_search.html.erb

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

app/views/media/catalog/_search_form.html.erb

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