From 7e67d3dd9c9437ce05310a9b42afc35dcf15153c Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 7 Feb 2020 18:08:01 -0600 Subject: [PATCH] views: replace .category-N css classes with .tag-type-N * Replace the .category-N CSS classes on tags with .tag-type-N. Before we were inconsistent about whether tag colors were indicated with .category-N or .tag-type-N. Now it's always .tag-type-N. * Fix various places to not use Tag.category_for. Tag.category_for does one Redis call per tag lookup, which leads to N Redis calls on many pages. This was inefficient because usually we either already had the tags from the database, or we could fetch them easily. --- app/controllers/dtext_links_controller.rb | 2 +- app/helpers/tags_helper.rb | 6 ++++++ app/javascript/src/styles/common/tags.scss.erb | 4 ++-- app/models/artist.rb | 4 ---- app/models/dtext_link.rb | 2 ++ app/models/wiki_page.rb | 6 +----- app/models/wiki_page_version.rb | 5 +---- app/presenters/tag_set_presenter.rb | 2 +- app/views/artists/_show.html.erb | 2 +- app/views/dtext_links/index.html.erb | 4 ++-- app/views/explore/posts/missed_searches.html.erb | 2 +- app/views/explore/posts/searches.html.erb | 2 +- app/views/related_tags/show.html.erb | 4 ++-- app/views/sources/_info.html.erb | 2 +- app/views/tag_aliases/_listing.html.erb | 6 ++++-- app/views/tag_implications/_listing.html.erb | 6 ++++-- app/views/tags/index.html.erb | 4 ++-- app/views/wiki_page_versions/_listing.html.erb | 4 ++-- app/views/wiki_pages/_recent_changes.html.erb | 4 ++-- app/views/wiki_pages/index.html.erb | 2 +- app/views/wiki_pages/show.html.erb | 2 +- 21 files changed, 38 insertions(+), 37 deletions(-) create mode 100644 app/helpers/tags_helper.rb diff --git a/app/controllers/dtext_links_controller.rb b/app/controllers/dtext_links_controller.rb index 2d7d42489..23a599f7b 100644 --- a/app/controllers/dtext_links_controller.rb +++ b/app/controllers/dtext_links_controller.rb @@ -3,7 +3,7 @@ class DtextLinksController < ApplicationController def index @dtext_links = DtextLink.paginated_search(params) - @dtext_links = @dtext_links.includes(:model) if request.format.html? + @dtext_links = @dtext_links.includes(linked_wiki: :tag, model: :tag) if request.format.html? respond_with(@dtext_links) end diff --git a/app/helpers/tags_helper.rb b/app/helpers/tags_helper.rb new file mode 100644 index 000000000..d6e8a9728 --- /dev/null +++ b/app/helpers/tags_helper.rb @@ -0,0 +1,6 @@ +module TagsHelper + def tag_class(tag) + return nil if tag.blank? + "tag-type-#{tag.category}" + end +end diff --git a/app/javascript/src/styles/common/tags.scss.erb b/app/javascript/src/styles/common/tags.scss.erb index 406d3819b..3d476464b 100644 --- a/app/javascript/src/styles/common/tags.scss.erb +++ b/app/javascript/src/styles/common/tags.scss.erb @@ -1,7 +1,7 @@ <% TagCategory.css_mapping.each do |category,cssmap| %> - .category-<%= category %> a, + .tag-type-<%= category %> a, a.tag-type-<%= category %>, - .ui-widget-content .category-<%= category %> a, + .ui-widget-content .tag-type-<%= category %> a, .ui-widget-content a.tag-type-<%= category %> { color: <%= cssmap["color"] %>; diff --git a/app/models/artist.rb b/app/models/artist.rb index 524a2be75..3b924d46f 100644 --- a/app/models/artist.rb +++ b/app/models/artist.rb @@ -354,10 +354,6 @@ class Artist < ApplicationRecord end module TagMethods - def category_name - Tag.category_for(name) - end - def validate_tag_category return unless is_active? && name_changed? diff --git a/app/models/dtext_link.rb b/app/models/dtext_link.rb index c922aae8b..08df3bc23 100644 --- a/app/models/dtext_link.rb +++ b/app/models/dtext_link.rb @@ -1,5 +1,7 @@ class DtextLink < ApplicationRecord belongs_to :model, polymorphic: true + belongs_to :linked_wiki, primary_key: :title, foreign_key: :link_target, class_name: "WikiPage", optional: true + enum link_type: [:wiki_link, :external_link] before_validation :normalize_link_target diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index abb69fb94..4f8e25b44 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -38,10 +38,6 @@ class WikiPage < ApplicationRecord where("is_deleted = false") end - def recent - order("updated_at DESC").limit(25) - end - def other_names_include(name) name = normalize_other_name(name) subquery = WikiPage.from("unnest(other_names) AS other_name").where_iequals("other_name", name) @@ -177,7 +173,7 @@ class WikiPage < ApplicationRecord end def category_name - Tag.category_for(title) + tag&.category end def pretty_title diff --git a/app/models/wiki_page_version.rb b/app/models/wiki_page_version.rb index c5f6302f8..f80522ebe 100644 --- a/app/models/wiki_page_version.rb +++ b/app/models/wiki_page_version.rb @@ -3,6 +3,7 @@ class WikiPageVersion < ApplicationRecord belongs_to :wiki_page belongs_to_updater belongs_to :artist, optional: true + belongs_to :tag, primary_key: :name, foreign_key: :title, optional: true module SearchMethods def search(params) @@ -51,10 +52,6 @@ class WikiPageVersion < ApplicationRecord !is_deleted && previous.is_deleted end - def category_name - Tag.category_for(title) - end - def self.available_includes [:updater, :wiki_page, :artist] end diff --git a/app/presenters/tag_set_presenter.rb b/app/presenters/tag_set_presenter.rb index 3e50dedd7..d79b8f704 100644 --- a/app/presenters/tag_set_presenter.rb +++ b/app/presenters/tag_set_presenter.rb @@ -107,7 +107,7 @@ class TagSetPresenter < Presenter count = tag.post_count category = tag.category - html = %{
  • } + html = %{
  • } unless name_only if category == Tag.categories.artist diff --git a/app/views/artists/_show.html.erb b/app/views/artists/_show.html.erb index 2dc98e1ab..1a9cc9590 100644 --- a/app/views/artists/_show.html.erb +++ b/app/views/artists/_show.html.erb @@ -3,7 +3,7 @@
    -

    Artist: <%= link_to @artist.pretty_name, posts_path(:tags => @artist.name), :class => "tag-type-#{@artist.category_name}" %>

    +

    Artist: <%= link_to @artist.pretty_name, posts_path(tags: @artist.name), class: tag_class(@artist.tag) %>

    <% if @artist.notes.present? %>
    diff --git a/app/views/dtext_links/index.html.erb b/app/views/dtext_links/index.html.erb index 3a393e08a..8cc2957fd 100644 --- a/app/views/dtext_links/index.html.erb +++ b/app/views/dtext_links/index.html.erb @@ -12,7 +12,7 @@ <%= table_for @dtext_links, class: "striped autofit" do |t| %> <% t.column "Page" do |dtext_link| %> <% if dtext_link.model_type == "WikiPage" %> - + <%= link_to(dtext_link.model.title, dtext_link.model) %> <%= link_to("»", dtext_links_path(search: { model_type: "WikiPage", model: { title: dtext_link.model.title }})) %> @@ -25,7 +25,7 @@ <% if dtext_link.external_link? %> <%= external_link_to(dtext_link.link_target) %> <% elsif dtext_link.wiki_link? %> - <%= link_to_wiki dtext_link.link_target, class: "tag-type-#{Tag.category_for(dtext_link.link_target)}" %> + <%= link_to_wiki dtext_link.link_target, class: tag_class(dtext_link.linked_wiki&.tag) %> <% end %> <%= link_to("»", dtext_links_path(search: { link_target: dtext_link.link_target })) %> diff --git a/app/views/explore/posts/missed_searches.html.erb b/app/views/explore/posts/missed_searches.html.erb index 2aa207094..2575d1694 100644 --- a/app/views/explore/posts/missed_searches.html.erb +++ b/app/views/explore/posts/missed_searches.html.erb @@ -16,7 +16,7 @@ <% @search_service.each_search do |tags, count| %> - + <%= link_to tags, posts_path(:tags => tags) %> <% unless WikiPage.titled(tags).exists? %> diff --git a/app/views/explore/posts/searches.html.erb b/app/views/explore/posts/searches.html.erb index f52a2c410..42770ad5a 100644 --- a/app/views/explore/posts/searches.html.erb +++ b/app/views/explore/posts/searches.html.erb @@ -14,7 +14,7 @@ <% @search_service.each_search do |tags, count| %> - + <%= link_to tags, posts_path(:tags => tags) %> <%= count.to_i %> diff --git a/app/views/related_tags/show.html.erb b/app/views/related_tags/show.html.erb index 90b3b6e0e..e292492a7 100644 --- a/app/views/related_tags/show.html.erb +++ b/app/views/related_tags/show.html.erb @@ -14,8 +14,8 @@ <% if params.dig(:search, :query).present? %> <%= table_for @query.tags do |t| %> <% t.column "Name" do |tag| %> - <%= link_to_wiki "?", tag.name, class: "tag-type-#{tag.category}" %> - <%= link_to tag.name, posts_path(tags: tag.name), class: "tag-type-#{tag.category}" %> + <%= link_to_wiki "?", tag.name, class: tag_class(tag) %> + <%= link_to tag.name, posts_path(tags: tag.name), class: tag_class(tag) %> <% end %> <% end %> <% end %> diff --git a/app/views/sources/_info.html.erb b/app/views/sources/_info.html.erb index 569bc8a73..9326d9b6b 100644 --- a/app/views/sources/_info.html.erb +++ b/app/views/sources/_info.html.erb @@ -19,7 +19,7 @@ <% else %> (
      <% @source.artists.each do |artist| %> -
    • <%= link_to artist.name, artist_path(artist), class: "tag-type-#{artist.category_name}" %>
      • +
    • <%= link_to artist.name, artist_path(artist), class: tag_class(artist.tag) %>
      • <% end %>
    ) <% end %> diff --git a/app/views/tag_aliases/_listing.html.erb b/app/views/tag_aliases/_listing.html.erb index bcef4a136..9fdafc433 100644 --- a/app/views/tag_aliases/_listing.html.erb +++ b/app/views/tag_aliases/_listing.html.erb @@ -1,9 +1,11 @@ <%= table_for tag_aliases, width: "100%" do |t| %> <% t.column "From", width: "25%" do |tag_alias| %> - <%= link_to tag_alias.antecedent_name, posts_path(:tags => tag_alias.antecedent_name) %> <%= tag_alias.antecedent_tag.post_count rescue 0 %> + <%= link_to tag_alias.antecedent_name, posts_path(tags: tag_alias.antecedent_name), class: tag_class(tag_alias.antecedent_tag) %> + <%= tag_alias.antecedent_tag&.post_count.to_i %> <% end %> <% t.column "To", width: "25%" do |tag_alias| %> - <%= link_to tag_alias.consequent_name, posts_path(:tags => tag_alias.consequent_name) %> <%= tag_alias.consequent_tag.post_count rescue 0 %> + <%= link_to tag_alias.consequent_name, posts_path(tags: tag_alias.consequent_name), class: tag_class(tag_alias.consequent_tag) %> + <%= tag_alias.consequent_tag&.post_count.to_i %> <% end %> <% t.column "Reference", width: "10%" do |tag_alias| %> <% if tag_alias.forum_topic_id %> diff --git a/app/views/tag_implications/_listing.html.erb b/app/views/tag_implications/_listing.html.erb index 441bedae7..3d633bc31 100644 --- a/app/views/tag_implications/_listing.html.erb +++ b/app/views/tag_implications/_listing.html.erb @@ -1,9 +1,11 @@ <%= table_for tag_implications, width: "100%" do |t| %> <% t.column "From", width: "25%" do |tag_implication| %> - <%= link_to tag_implication.antecedent_name, posts_path(:tags => tag_implication.antecedent_name) %> <%= tag_implication.antecedent_tag.post_count rescue 0 %> + <%= link_to tag_implication.antecedent_name, posts_path(tags: tag_implication.antecedent_name), class: tag_class(tag_implication.antecedent_tag) %> + <%= tag_implication.antecedent_tag&.post_count.to_i %> <% end %> <% t.column "To", width: "25%" do |tag_implication| %> - <%= link_to tag_implication.consequent_name, posts_path(:tags => tag_implication.consequent_name) %> <%= tag_implication.consequent_tag.post_count rescue 0 %> + <%= link_to tag_implication.consequent_name, posts_path(tags: tag_implication.consequent_name), class: tag_class(tag_implication.consequent_tag) %> + <%= tag_implication.consequent_tag&.post_count.to_i %> <% end %> <% t.column "Reference", width: "10%" do |tag_implication| %> <% if tag_implication.forum_topic_id %> diff --git a/app/views/tags/index.html.erb b/app/views/tags/index.html.erb index c9d800545..20cd6286c 100644 --- a/app/views/tags/index.html.erb +++ b/app/views/tags/index.html.erb @@ -7,8 +7,8 @@ <%= table_for @tags, {class: "striped autofit"} do |t| %> <% t.column :post_count, name: "Count" %> <% t.column "Name", td: {class: "col-expand"} do |tag| %> - <%= link_to_wiki "?", tag.name, class: "tag-type-#{tag.category}" %> - <%= link_to tag.name, posts_path(tags: tag.name), class: "tag-type-#{tag.category}" %> + <%= link_to_wiki "?", tag.name, class: tag_class(tag) %> + <%= link_to tag.name, posts_path(tags: tag.name), class: tag_class(tag) %> <% end %> <% t.column column: "control" do |tag| %> <%= link_to_if tag.editable_by?(CurrentUser.user), "Edit", edit_tag_path(tag) %> | diff --git a/app/views/wiki_page_versions/_listing.html.erb b/app/views/wiki_page_versions/_listing.html.erb index 3edd37d2a..21998b425 100644 --- a/app/views/wiki_page_versions/_listing.html.erb +++ b/app/views/wiki_page_versions/_listing.html.erb @@ -1,6 +1,6 @@
    <%= form_tag(diff_wiki_page_versions_path, :method => :get) do %> - <%= table_for @wiki_page_versions, width: "100%" do |t| %> + <%= table_for @wiki_page_versions.includes(:updater, :tag), width: "100%" do |t| %> <% t.column column: "diff", width: "3%" do |wiki_page_version, i| %> <%= link_to_if wiki_page_version.previous.present?, "diff", diff_wiki_page_versions_path(otherpage: wiki_page_version.previous.try(:id), thispage: wiki_page_version.id) %> <% end %> @@ -15,7 +15,7 @@ <% end %> <% t.column "Title" do |wiki_page_version| %> - + <%= link_to "?", wiki_page_path(wiki_page_version.wiki_page_id) %> <%= link_to wiki_page_version.title, wiki_page_version %> <%= link_to "»", wiki_page_versions_path(search: { wiki_page_id: wiki_page_version.wiki_page_id }) %> diff --git a/app/views/wiki_pages/_recent_changes.html.erb b/app/views/wiki_pages/_recent_changes.html.erb index 74312bcac..20f5eb2cc 100644 --- a/app/views/wiki_pages/_recent_changes.html.erb +++ b/app/views/wiki_pages/_recent_changes.html.erb @@ -1,8 +1,8 @@

    Recent Changes (<%= link_to "all", wiki_page_versions_path %>)

      - <% WikiPage.recent.each do |page| %> -
    • <%= link_to page.pretty_title, wiki_page_path(page) %>
      • + <% WikiPage.order(updated_at: :desc).includes(:tag).limit(25).each do |page| %> +
    • <%= link_to page.pretty_title, wiki_page_path(page), class: tag_class(page.tag) %>
      • <% end %>
    diff --git a/app/views/wiki_pages/index.html.erb b/app/views/wiki_pages/index.html.erb index 9033b7c1f..7a03e3d34 100644 --- a/app/views/wiki_pages/index.html.erb +++ b/app/views/wiki_pages/index.html.erb @@ -5,7 +5,7 @@ <%= table_for @wiki_pages, width: "100%" do |t| %> <% t.column "Title" do |wiki_page| %> - <%= link_to_wiki wiki_page.title %> + <%= link_to_wiki wiki_page.title %> <% end %> <% t.column "Last edited" do |wiki_page| %> <%= time_ago_in_words_tagged(wiki_page.updated_at) %> diff --git a/app/views/wiki_pages/show.html.erb b/app/views/wiki_pages/show.html.erb index e3581da4e..de2863565 100644 --- a/app/views/wiki_pages/show.html.erb +++ b/app/views/wiki_pages/show.html.erb @@ -6,7 +6,7 @@ <% content_for(:content) do %>

    - <%= link_to @wiki_page.pretty_title, posts_path(:tags => @wiki_page.title), :class => "tag-type-#{@wiki_page.category_name}" %> + <%= link_to @wiki_page.pretty_title, posts_path(tags: @wiki_page.title), class: tag_class(@wiki_page.tag) %> <% if @wiki_page.is_locked? %> (locked)