Move topics to a dedicated controller

I prefer to stick to only the default resources inside of controller actions when possible. A pattern I use in other places is to use show coupled with params[:id] to represent differentiated resources. In this process I removed the API handler (as I don't think it's needed here). I also took out the logic for showing repo subscriptions. I wanted to get a barebones but solid page up first and make sure the performance is okay before adding in extra functionality. I also moved from the `includes(:repo_labels)` to a `joins(:repo_labels)` as the first will force all repo labels into memory and it can have massive memory side effects.

Author: schneems

app/controllers/pages_controller.rb

@@ -51,46 +51,7 @@ def index
  end
  end
 
- def topic
- topic_name = params['topic']
-
- if not ['hacktoberfest'].include?(topic_name)
- redirect_back fallback_location: root_path
- return
- end
-
- set_title("Open Source Project Topic: #{topic_name}")
- set_description(description)
-
- label = Label.find_by(name: topic_name)
-
- @repos = Repo.with_some_issues
- .includes(:repo_labels)
- .where(repo_labels: { label_id: label.id })
- .select(:id, :updated_at, :issues_count, :language, :full_name, :name, :description)
-
- topic_repo_ids = @repos.map { |repo| repo.id }
-
- @repos = @repos.without_user_subscriptions(current_user.id) if user_signed_in?
- @repos = @repos.order_by_issue_count.page(valid_params[:page]).per_page(valid_params[:per_page] || 50)
-
- if user_signed_in?
- @repos_subs = current_user.repo_subscriptions.page(valid_params[:page]).per_page(valid_params[:per_page] || 50).includes(:repo)
- @repos_subs = @repos_subs.where(repo_id: topic_repo_ids)
- end
-
- respond_to do |format|
- format.html {}
- format.json do
- htmlForPage = render_to_string(partial: "repos_with_pagination", locals: { repos: @repos }, formats: ['html'])
- render json: { html: htmlForPage }.to_json
- end
- end
- end
-
- private
-
- def description
+ private def description
  Rails.cache.fetch("pages#index/description", expires_in: 1.hour) do
  "Discover the easiest way to get started contributing to open source. " \
  "Over #{number_with_delimiter(User.count, delimiter: ',')} devs are " \
@@ -99,11 +60,11 @@ def description
  end
  end
 
- def valid_params
+ private def valid_params
  params.permit(:language, :per_page, :page)
  end
 
- def set_cache_headers
+ private def set_cache_headers
  response.headers["Cache-Control"] = "no-cache, no-store"
  response.headers["Pragma"] = "no-cache"
  response.headers["Expires"] = "Fri, 01 Jan 1990 00:00:00 GMT"

app/controllers/topics_controller.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class TopicsController < ApplicationController
+ TOPICS = ["hacktoberfest".html_safe].freeze
+ before_action :set_topic
+
+ def show
+ set_title("Open Source Project Topic: #{@topic}")
+ set_description("Open source repos labeled #{@topic}")
+
+ label = Label.find_by(name: @topic)
+ @repos = Repo.with_some_issues
+ .joins(:repo_labels)
+ .where(repo_labels: { label_id: label.id })
+ .select(:id, :issues_count, :language, :full_name, :name, :description)
+
+ @repos = @repos.order_by_issue_count.page(valid_params[:page]).per_page(valid_params[:per_page] || 50)
+ end
+
+ private def set_topic
+ topic_name = valid_params['id'].downcase
+ @topic = TOPICS.detect { |t| t == topic_name }
+
+ if @topic.blank?
+ redirect_back fallback_location: root_path, notice: "Invalid topic #{topic_name.inspect} not found in #{TOPICS.map(&:inspect).join(", ")}"
+ end
+ end
+
+ private def valid_params
+ params.permit(:id, :per_page, :page)
+ end
+end

app/views/application/_nav.html.slim

@@ -12,21 +12,13 @@ nav.application-navigation
  li.application-navigation-list-item= image_tag current_user.avatar_url, class: 'user-profile-image', alt: 'user avatar'
  li.application-navigation-list-item= link_to current_user.github, current_user
  li.application-navigation-list-item= link_to 'University', university_index_path
- li.application-navigation-list-item
- div.dropdown
- = content_tag 'a', 'Topics'
- div.dropdown-content
- =link_to 'Hacktoberfest', '/topics/hacktoberfest'
+ li.application-navigation-list-item= link_to 'Hacktoberfest', topic_path(id: "hacktoberfest")
  li.application-navigation-list-item= link_to "Submit Repo", new_repo_path
  li.application-navigation-list-item= link_to "Sign Out", destroy_user_session_path, method: :delete
  - else
  li.application-navigation-list-item= link_to 'About', what_path
  li.application-navigation-list-item= link_to 'University', university_index_path
- li.application-navigation-list-item
- div.dropdown
- = content_tag 'a', 'Topics'
- div.dropdown-content
- =link_to 'Hacktoberfest', '/topics/hacktoberfest'
+ li.application-navigation-list-item= link_to 'Hacktoberfest', topic_path(id: "hacktoberfest")
  li.application-navigation-list-item= link_to 'Log in', user_github_omniauth_authorize_path, method: :post
  li.application-navigation-list-item= link_to 'Sign Up', user_github_omniauth_authorize_path, method: :post
 
@@ -35,7 +27,6 @@ div.application-logo
  span.application-logo-image
  span.hide-visually Code Triage Home
 
-
 scss class="class":
 
  .dropdown:hover .dropdown-content {

app/views/pages/topic.html.slim

@@ -0,0 +1,14 @@
+.hero
+ h1.hero-title-primary Repos with label: #{@topic}
+
+ - unless user_signed_in?
+ = link_to user_github_omniauth_authorize_path, class: 'button', method: :post
+ | Sign up with GitHub
+
+hr.section-break
+
+.repos-list-wrapper
+ h2.repos-list-title All repos with with label #{@topic}.
+
+ section.repo-list-with-pagination
+ = render "pages/repos_with_pagination", repos: @repos

app/views/topics/show.html.slim

@@ -27,8 +27,7 @@
  end
 
  root to: "pages#index"
-
- get "/topics/:topic", to: "pages#topic"
+ resources :topics, only: [:show]
 
  get 'what' => "pages#what"
  get 'privacy' => "pages#privacy"