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 sort and search issues fixes #21392 #21407

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion app/controllers/articles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ class ArticlesController < ApplicationController
include ApplicationHelper

# NOTE: It seems quite odd to not authenticate the user for the :new action.
before_action :authenticate_user!, except: %i[feed new]
# before_action :authenticate_user!, except: %i[feed new]
before_action :authenticate_user!, except: %i[feed new search]
before_action :set_article, only: %i[edit manage update destroy stats admin_unpublish admin_featured_toggle]
# NOTE: Consider pushing this check into the associated Policy. We could choose to raise a
# different error which we could then rescue as part of our exception handling.
Expand Down Expand Up @@ -65,6 +66,20 @@ def feed
}
end

def search
skip_authorization

query = params[:query].to_s.strip
sort_option = params[:sort] || "relevance" # Default to relevance if no sort option is provided

@articles = Article.search_with_sort(query: query, sort_option: sort_option)

respond_to do |format|
format.html { render :index }
format.json { render json: @articles }
end
end

# @note The /new path is a unique creature. We want to ensure that folks coming to the /new with
# a prefill of information are first prompted to sign-in, and then given a form that
# prepopulates with that pre-fill information. This is a feature that StackOverflow and
Expand Down
60 changes: 40 additions & 20 deletions app/models/article.rb
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ def self.unique_url_error
}
},
ignoring: :accents
ranked_by: ":tsearch + (1.0 / (NOW() - published_at))"

# [@jgaskins] We use an index on `published`, but since it's a boolean value
# the Postgres query planner often skips it due to lack of diversity of the
Expand Down Expand Up @@ -391,29 +392,48 @@ def self.unique_url_error
:body_markdown, :email_digest_eligible, :processed_html, :co_author_ids, :score, :type_of)
}

scope :search_and_sort, lambda { |query, sort_option|
articles = search_articles(query).published
case sort_option
when "newest"
articles.order(published_at: :desc)
when "oldest"
articles.order(published_at: :asc)
when "relevance"
articles
else
articles.order(created_at: :desc)
end
}

scope :sorting, lambda { |value|
value ||= "creation-desc"
kind, dir = value.split("-")

dir = "desc" unless %w[asc desc].include?(dir)

case kind
when "creation"
order(created_at: dir)
when "views"
order(page_views_count: dir)
when "reactions"
order(public_reactions_count: dir)
when "comments"
order(comments_count: dir)
when "published"
# NOTE: For recently published, we further filter to only published posts
order(published_at: dir).published
else
order(created_at: dir)
end
value ||= "creation-desc"
kind, dir = value.split("-")

dir = "desc" unless %w[asc desc].include?(dir)

case kind
when "creation"
order(created_at: dir)
when "views"
order(page_views_count: dir)
when "reactions"
order(public_reactions_count: dir)
when "comments"
order(comments_count: dir)
when "published"
order(published_at: dir).published
when "search_newest"
order(published_at: :desc).search_articles
else
order(created_at: dir)
end
}

def self.search_with_sort(query:, sort_option:)
search_and_sort(query, sort_option).limit(DEFAULT_FEED_PAGINATION_WINDOW_SIZE)
end

# @note This includes the `featured` scope, which may or may not be
# something we expose going forward. However, it was
# something used in two of the three queries we had that
Expand Down
12 changes: 12 additions & 0 deletions app/models/articles/feeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ def self.oldest_published_at_to_consider_for(user:, days_since_published: DEFAUL
def self.feed_for(controller:, user:, number_of_articles:, page:, tag:, type_of: "discover")
variant = AbExperiment.get_feed_variant_for(controller: controller, user: user)

# Ensure relevancy and recency sorting for "Newest"
if type_of == "search_sort_newest"
return VariantQuery.build_for(
variant: :relevancy_score_and_publication_date,
user: user,
number_of_articles: number_of_articles,
page: page,
tag: tag,
type_of: type_of
)
end

VariantQuery.build_for(
variant: variant,
user: user,
Expand Down
17 changes: 15 additions & 2 deletions app/views/admin/users/index/_applied_filters.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@
<%= crayons_icon_tag("x.svg", class: "c-pill__action-icon", aria_hidden: true) %>
</button>
<% end %>

<% if params[:search].present? %>
<button
data-filter-key="search"
data-filter-value="<%= params[:search] %>"
class="c-pill c-pill--action-icon c-pill--action-icon--destructive"
type="button">
<%= params[:search] %>
<%= crayons_icon_tag("x.svg", class: "c-pill__action-icon", aria_hidden: true) %>
</button>
<% end %>


<% params[:organizations]&.each do |org_id| %>
<% org = @organizations.find_by(id: org_id) %>
<button
Expand All @@ -46,7 +59,7 @@
<%= crayons_icon_tag("x.svg", class: "c-pill__action-icon", aria_hidden: true) %>
</button>
<% end %>
<% if params[:roles] || params[:organizations] || params[:joining_start].present? || params[:joining_end].present? || params[:statuses] %>
<button class="js-clear-filters-btn c-btn" aria-label="<%= t("views.admin.users.filters.aria_clear") %>"><%= t("views.admin.users.filters.clear") %></button>
<% if params[:roles] || params[:organizations] || params[:joining_start].present? || params[:joining_end].present? || params[:statuses] || params[:search].present? %>
<button class="js-clear-filters-btn c-btn" aria-label="<%= t("views.admin.users.filters.aria_clear") %>"><%= t("views.admin.users.filters.clear") %></button>
<% end %>
</div>
1 change: 1 addition & 0 deletions app/views/admin/users/index/_controls.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<div class="m:hidden">
<%= render "admin/users/controls/expand_search_button" %>
</div>
<%= f.text_field :search, placeholder: t("views.admin.users.search.placeholder"), class: "crayons-textfield crayons-textfield--sm ml-2" if params[:search].present? %>
<button type="button" aria-label="<%= t("views.admin.users.filters.aria_label") %>" class="js-open-filter-modal-btn c-btn c-btn--icon-alone"><%= crayons_icon_tag("filter", aria_hidden: true) %></button>
<%= render "admin/users/controls/export" %>
</div>
Expand Down
22 changes: 22 additions & 0 deletions app/views/admin/users/index/_filters_modal.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@
</div>
<p id="filters-description" class="mt-4 crayons-field__description"><%= t("views.admin.users.filters.desc") %></p>
<div class="py-4">
<!-- Added Keyword Filter Section -->
<details data-section="keyword" class="admin-details">
<summary class="py-4 flex justify-between text-uppercase color-base-60 fs-s">
<span class="flex">
<%= t("views.admin.users.filters.summary.keyword") %>
<span class="js-filtered-indicator-keyword relative ml-2 <%= params[:search].blank? ? "hidden" : "" %>">
<span class="block absolute c-indicator c-indicator--info"></span>
</span>
</span>
<%= crayons_icon_tag("chevron-down", aria_hidden: true, class: "summary-icon") %>
</summary>
<fieldset class="js-keyword-fieldset pb-4 flex flex-col items-start">
<legend class="screen-reader-only"><%= t("views.admin.users.filters.legend.keyword") %></legend>
<div class="crayons-field crayons-field--text">
<%= f.text_field :search, placeholder: t("views.admin.users.filters.keyword.placeholder"), class: "crayons-textfield", value: params[:search] %>
</div>
<button type="button" class="js-clear-filter-btn c-btn <%= params[:search].blank? ? "hidden" : "" %>" data-textfield-selector=".js-keyword-fieldset" data-filter-indicator-selector=".js-filtered-indicator-keyword"
aria-label="<%= t("views.admin.users.filters.clear_filter.aria_keyword") %>">
<%= t("views.admin.users.filters.clear_filter.text") %>
</button>
</fieldset>
</details>
<details data-section="roles" class="admin-details">
<summary class="py-4 flex justify-between text-uppercase color-base-60 fs-s">
<span class="flex">
Expand Down
Loading