Skip to content

Commit

Permalink
Merge pull request #380 from rubytoolbox/co-filter-bugfix-forks-from-…
Browse files Browse the repository at this point in the history
…search-results

Filter bugfix forks from search results by default & allow to toggle display
  • Loading branch information
colszowka committed Jan 15, 2019
2 parents 1f97439 + 1183d66 commit 8aa4a23
Show file tree
Hide file tree
Showing 17 changed files with 286 additions and 39 deletions.
24 changes: 24 additions & 0 deletions app/assets/javascripts/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,28 @@ document.addEventListener("turbolinks:load", function () {
});
})
});

// When the bugfix forks toggle is clicked on the search the HTTP response takes
// a tad too long to process without visual feedback to the user that something has happened,
// therefore we put it into the loading state (and when going back in history ensure to revert
// that state)
document.querySelectorAll("a.bugfix-forks-toggle").forEach(function(button) {
button.classList.remove("is-loading");
button.addEventListener("click", function() {
button.classList.add("is-loading");
});
});

// See above, just for the custom project order dropdown
document.querySelectorAll(".project-order-dropdown .dropdown-content a").forEach(function(button) {
document.querySelectorAll(".project-order-dropdown button").forEach(function(dropdown) {
dropdown.classList.remove("is-loading");
});
button.addEventListener("click", function() {
document.querySelectorAll(".project-order-dropdown button").forEach(function(dropdown) {
dropdown.classList.add("is-loading");
});
});
});
});

3 changes: 3 additions & 0 deletions app/assets/stylesheets/components/project_health_tag.sass
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@
&.level-red
.tag
@extend .is-danger

a.tag:hover
text-decoration: none
24 changes: 23 additions & 1 deletion app/controllers/searches_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,34 @@ def show
@query = params[:q].presence
return unless @query

@search = Search.new(@query, order: current_order)
@search = Search.new(@query, order: current_order, show_forks: show_forks?)
@projects = @search.projects.page(params[:page])

redirect_to_search_with_forks_included if should_redirect_to_included_forks?
end

private

# If a user searches for some query but that search does not
# yield any project results we automatically redirect to the
# search with bugfix forks included. However this must only
# happen if the show forks param is not set at all, otherwise
# it becomes impossible to reduce the query back to "without forks" :)
def should_redirect_to_included_forks?
!show_forks? && !params.key?(:show_forks) && @projects.empty?
end

def redirect_to_search_with_forks_included
redirect_to action: :show,
q: @search.query,
order: current_order.ordered_by,
show_forks: true
end

def show_forks?
params[:show_forks].present? && params[:show_forks] == "true"
end

def current_order
@current_order ||= Project::Order.new order: params[:order], directions: Project::Order::SEARCH_DIRECTIONS
end
Expand Down
4 changes: 4 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ def category_card(category, compact: false, inline: false)
render partial: "components/category_card", locals: locals
end

def project_health_tags(project)
render "components/project_health_tags", project: project
end

def project_health_tag(health_status)
render "components/project_health_tag", status: health_status
end
Expand Down
3 changes: 2 additions & 1 deletion app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ def self.with_bugfix_forks(include_forks)
},
ranked_by: ":tsearch * (#{table_name}.score + 1) * (#{table_name}.score + 1)"

def self.search(query, order: Project::Order.new(directions: Project::Order::SEARCH_DIRECTIONS))
def self.search(query, order: Project::Order.new(directions: Project::Order::SEARCH_DIRECTIONS), show_forks: false)
with_score
.with_bugfix_forks(show_forks)
.search_scope(query)
.reorder("")
.includes_associations
Expand Down
9 changes: 5 additions & 4 deletions app/models/search.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
# frozen_string_literal: true

class Search
attr_accessor :query, :order
private :query=, :order=
attr_accessor :query, :order, :show_forks
private :query=, :order=, :show_forks=

def initialize(query, order: Project::Order.new(directions: Project::Order::SEARCH_DIRECTIONS))
def initialize(query, order: Project::Order.new(directions: Project::Order::SEARCH_DIRECTIONS), show_forks: false)
self.query = query.presence&.strip
self.order = order
self.show_forks = !!show_forks
end

def query?
query.present?
end

def projects
@projects ||= Project.search(query, order: order)
@projects ||= Project.search(query, order: order, show_forks: show_forks)
end

def categories
Expand Down
11 changes: 11 additions & 0 deletions app/views/components/_project_health_tags.html.slim
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.project-health-tags
- if project.bugfix_fork_of
.project-health-tag.tags.has-addons
a.tag.is-dark href=page_path("docs/features/bugfix_forks")
span.icon: i.fa.fa-code-fork
span Bugfix fork of
a.tag.is-primary href=project_path(project.bugfix_fork_of)
= project.bugfix_fork_of

- Project::Health.new(project).status.each do |status|
= project_health_tag status
2 changes: 1 addition & 1 deletion app/views/components/_project_order_dropdown.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@
strong= t(:label, scope: "labels.#{group}")

- directions.map(&:key).each do |direction|
a href="#{request.path}?order=#{direction}&q=#{@search&.query}" class=(order.is?(direction) ? "is-active" : "")
a href="#{request.path}?order=#{direction}&q=#{@search&.query}&show_forks=#{@search&.show_forks}" class=(order.is?(direction) ? "is-active" : "")
span.icon: i.fa class=metric_icon(direction)
= metric_label direction
16 changes: 16 additions & 0 deletions app/views/pages/components/project_health_tag.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,19 @@
= project_health_tag Project::Health::HEALTHY_STATUS
- Project::Health::Checks::ALL.each do |status|
= project_health_tag status

= component_example "Tag list shortcut helper" do
.content
markdown:
Use `project_health_tags(project)` to render the full list of health tags without iterating over individual items

= project_health_tags Project.new

= component_example "Reference to bugfix forks" do
.content
markdown:
Projects that are a [bugfix fork](/pages/docs/features/bugfix_forks)
get an additional badge linking the documentation and the
suspected originating gem.

= project_health_tags Project.new(bugfix_fork_of: "rails")
40 changes: 40 additions & 0 deletions app/views/pages/docs/features/bugfix_forks.html.slim
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
.hero
section.section: .container.has-text-centered
h2 Bugfix Forks


section.section: .container: .columns
article.blog-post
markdown:
Nowadays it's easy to **include a patched gem version from a git repository**
by adding `gem "xyz", git: "some git source"` to your application's `Gemfile`, but **before
[bundler] existed** this was a very **cumbersome task**.

If you **ran into an issue with a gem** you were using and you or somebody else
created a bugfix for the problem **you had to wait for the library maintainers to publish
a new release** to Rubygems **before you could patch your application** (or use [Github's discontinued Rubygems server][github-gems], which also had namespacing).

To **get around this issue** many developers adopted a pattern of **publishing a namespaced fork
including a bugfix** to Rubygems **until the fix was merged and released upstream**.

Unfortunately the widespread adoption of this practice led to a **huge number of orphaned gems**
that usually have only a single or at most a handful of releases. Additionally these **forked gems
often retained** their upstream **github source code url**, so on the Ruby Toolbox **these forks
receive a high popularity ranking**.

Because of this, the Ruby Toolbox tries to identify these projects based on common patterns.

If a **project is flagged as a bugfix fork** by default it **does not show up in search results**.
However, you can **re-enable inclusion of bugfix forks** from within the **search navigation** if
you are missing a library.

If you spot a **library that is flagged as a bugfix fork wrongly** please
[report this as an issue on the Ruby Toolbox issue tracker](https://github.com/rubytoolbox/rubytoolbox/issues).

#### See also:

* [rubytoolbox#377](https://github.com/rubytoolbox/rubytoolbox/pull/377)
* [rubytoolbox#380](https://github.com/rubytoolbox/rubytoolbox/pull/380)

[bundler]: /projects/bundler
[github-gems]: https://blog.github.com/2008-04-25-github-s-rubygem-server/
5 changes: 1 addition & 4 deletions app/views/projects/_project.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@
i.fa class=metric_icon(:score)
span= project.score

.columns: .column
.project-health-tags
- Project::Health.new(project).status.each do |status|
= project_health_tag status
.columns: .column= project_health_tags project

- if local_assigns[:show_categories] and project.categories.any?
.columns.is-hidden-desktop: .column
Expand Down
6 changes: 2 additions & 4 deletions app/views/projects/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@
- @project.categories.each do |category|
= category_card category, compact: true, inline: true

.columns: .column
.project-health-tags
- Project::Health.new(@project).status.each do |status|
= project_health_tag status
.columns: .column= project_health_tags @project


.columns: .links.column
= render partial: "projects/links", locals: { project: @project }
Expand Down
24 changes: 21 additions & 3 deletions app/views/searches/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,29 @@
span Category results are hidden when using a custom project result order

.columns: .column.projects
.level
.level.project-search-nav
.level-left: .level-item
h2.subtitle.is-4 Projects
.level-right: .level-item
= project_order_dropdown current_order
.level-right
.level-item
.field.has-addons
.control
- if @search.show_forks
a.button.bugfix-forks-toggle href=search_path(q: @search.query, order: current_order.ordered_by, show_forks: false)
span.icon: i.fa.fa-check-square
span Bugfix forks are <strong>shown</strong>

- else
a.button.bugfix-forks-toggle href=search_path(q: @search.query, order: current_order.ordered_by, show_forks: true)
span.icon: i.fa.fa-square-o
span Bugfix forks are <strong>hidden</strong>

.control
a.button.bugfix-forks-help href=page_path("docs/features/bugfix_forks")
span.icon: i.fa.fa-question-circle

.level-item
= project_order_dropdown current_order

- if @projects.any?
.columns: .column= paginate @projects
Expand Down
40 changes: 34 additions & 6 deletions spec/controllers/searches_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,20 @@

RSpec.describe SearchesController, type: :controller do
describe "GET #show" do
def do_request(query: nil, order: nil)
get :show, params: { q: query, order: order }
def do_request(query: nil, order: nil, show_forks: nil)
get :show, params: { q: query, order: order, show_forks: show_forks }
end

it "returns http success" do
do_request
expect(response).to be_successful
Factories.project "foobar"
do_request query: "foobar"
expect(response).to have_http_status(:success)
end

it "renders show template" do
Factories.project "foobar"
do_request query: "foobar"
expect(response).to render_template(:show)
end

it "assigns nothing to search when there is no query" do
Expand All @@ -30,13 +37,34 @@ def do_request(query: nil, order: nil)
.and respond_to(:total_pages)
end

it "passes query and a project order instance to Search.new" do
it "passes query, a project order instance and show_forks status to Search.new" do
order = Project::Order.new(order: "rubygem_downloads")
allow(Project::Order).to receive(:new)
.with(order: "rubygem_downloads", directions: Project::Order::SEARCH_DIRECTIONS)
.and_return(order)
expect(Search).to receive(:new).with("hello world", order: order).and_call_original
expect(Search).to receive(:new).with("hello world", order: order, show_forks: false).and_call_original
do_request query: "hello world", order: "rubygem_downloads"
end

it "passes show_forks true to search when set in params" do
expect(Search).to receive(:new)
.with(
kind_of(String),
order: kind_of(Project::Order),
show_forks: true
)
.and_call_original
do_request query: "hello world", order: "rubygem_downloads", show_forks: true
end

it "redirects to results including forks when project search has no results" do
get :show, params: { q: "my query" }
expect(response).to redirect_to(search_path(q: "my query", order: "rank", show_forks: true))
end

it "does not redirect when project search has no results but explicit show forks is given" do
get :show, params: { q: "my query", show_forks: true }
expect(response).to have_http_status(:success)
end
end
end
Loading

0 comments on commit 8aa4a23

Please sign in to comment.