Update onboarding suggested tag logic (#19477)

* Onboarding suggested tags should include supported

* Migrate tags to add suggested boolean column

* Use new suggested attribute

* Sync Settings.suggested_tags with new form setting

* Better sort order

* Test logic update

* Add a little comment

app/controllers/admin/tags_controller.rb

@@ -6,6 +6,7 @@ module Admin
       id supported rules_markdown short_summary pretty_name bg_color_hex
       text_color_hex user_id alias_for badge_id requires_approval
       social_preview_template wiki_body_markdown submission_template
+      suggested
     ].freeze
 
     before_action :set_default_options, only: %i[index]

app/controllers/tags_controller.rb

@@ -66,7 +66,7 @@ class TagsController < ApplicationController
   def onboarding
     skip_authorization
 
-    @tags = Tag.where(name: Settings::General.suggested_tags)
+    @tags = Tags::SuggestedForOnboarding.call
       .select(ONBOARDING_API_ATTRIBUTES)
 
     render json: @tags

app/models/tag.rb

@@ -56,6 +56,7 @@ class Tag < ActsAsTaggableOn::Tag
   before_save :calculate_hotness_score
   before_save :mark_as_updated
 
+  after_save :update_suggested_tags, if: :saved_change_to_suggested?
   after_commit :bust_cache
 
   # @note Even though we have a data migration script (see further
@@ -81,6 +82,8 @@ class Tag < ActsAsTaggableOn::Tag
   scope :eager_load_serialized_data, -> {}
   scope :supported, -> { where(supported: true) }
 
+  scope :suggested_for_onboarding, -> { where(suggested: true) }
+
   # possible social previews templates for articles with a particular tag
   def self.social_preview_templates
     Rails.root.join("app/views/social_previews/articles").children.map { |ch| File.basename(ch, ".html.erb") }
@@ -293,4 +296,8 @@ class Tag < ActsAsTaggableOn::Tag
   def mark_as_updated
     self.updated_at = Time.current # Acts-as-taggable didn't come with this by default
   end
+
+  def update_suggested_tags
+    Settings::General.suggested_tags = Tag.suggested_for_onboarding.pluck(:name).join(",")
+  end
 end

app/queries/tags/suggested_for_onboarding.rb

@@ -0,0 +1,33 @@
+module Tags
+  class SuggestedForOnboarding
+    MAX = 45
+
+    def self.call(...)
+      new(...).call
+    end
+
+    def initialize(...); end
+
+    def call
+      return suggested_tags if suggested_tags.count >= MAX
+
+      Tag
+        .where(suggested_for_onboarding_or_supported)
+        .order("suggested DESC, supported DESC, taggings_count DESC, name ASC")
+        .limit(MAX)
+    end
+
+    private
+
+    def suggested_tags
+      @suggested_tags ||= Tag.suggested_for_onboarding
+    end
+
+    def suggested_for_onboarding_or_supported
+      builder = Tag.arel_table
+      supported = builder[:supported].eq(true)
+      suggested = builder[:suggested].eq(true)
+      suggested.or(supported)
+    end
+  end
+end

app/services/settings/general/upsert.rb

@@ -34,8 +34,18 @@ module Settings
       def self.create_tags_if_necessary(settings)
         return unless (settings.keys & TAG_PARAMS).any?
 
-        tags = Settings::General.suggested_tags + Settings::General.sidebar_tags
-        Tag.find_or_create_all_with_like_by_name(tags)
+        create_sidebar_tags
+        create_suggested_tags
+      end
+
+      def self.create_sidebar_tags
+        Tag.find_or_create_all_with_like_by_name(Settings::General.sidebar_tags)
+      end
+
+      def self.create_suggested_tags
+        suggested = Tag.find_or_create_all_with_like_by_name(Settings::General.suggested_tags)
+        Tag.where(suggested: true).update_all(suggested: false)
+        Tag.where(id: suggested).update_all(suggested: true)
       end
 
       def self.upload_logo(image)

app/views/admin/tags/_form.html.erb

@@ -12,6 +12,13 @@
       <p class="crayons-field__description">Allows the tag to be a searchable result when writing a post or a listing</p>
     </label>
   </div>
+  <div class="crayons-field crayons-field--checkbox mt-3">
+    <%= f.check_box :suggested, class: "crayons-checkbox" %>
+    <label class="crayons-field__label" for="tag_suggested">
+      Suggested
+      <p class="crayons-field__description">Prioritizes the tag during user onboarding</p>
+    </label>
+  </div>
   <div class="crayons-field mt-3">
     <%= f.label :badge_id, class: "crayons-field__label" %>
     <%= f.select(:badge_id, options_for_select(badges_for_options, tag.badge_id), { include_blank: true }, { class: "crayons-select" }) %>

db/migrate/20230517132219_add_suggested_to_tags.rb

@@ -0,0 +1,5 @@
+class AddSuggestedToTags < ActiveRecord::Migration[7.0]
+  def change
+    add_column :tags, :suggested, :boolean, default: false
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_03_30_145039) do
+ActiveRecord::Schema[7.0].define(version: 2023_05_17_132219) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "pg_stat_statements"
@@ -1109,6 +1109,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_03_30_145039) do
     t.string "social_image"
     t.string "social_preview_template", default: "article"
     t.text "submission_template"
+    t.boolean "suggested", default: false
     t.boolean "supported", default: false
     t.integer "taggings_count", default: 0
     t.string "text_color_hex"

lib/data_update_scripts/20230517132257_populate_suggested_tags_from_settings.rb

@@ -0,0 +1,20 @@
+module DataUpdateScripts
+  class PopulateSuggestedTagsFromSettings
+    cattr_accessor :suggested_tags
+
+    # We've added a new column on Tag (suggested) that needs to be populated
+    # from a list that has previously been stored in Settings::General.suggested_tags
+    # later this can go away
+    def run(suggested_tags: settings_suggested_tags)
+      return unless suggested_tags.any?
+
+      Tag.where(name: suggested_tags).update_all suggested: true
+    end
+
+    private
+
+    def settings_suggested_tags
+      suggested_tags || Settings::General.suggested_tags
+    end
+  end
+end

spec/lib/data_update_scripts/populate_suggested_tags_from_settings_spec.rb

@@ -0,0 +1,29 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20230517132257_populate_suggested_tags_from_settings.rb",
+)
+
+describe DataUpdateScripts::PopulateSuggestedTagsFromSettings do
+  let(:suggested_tags) { %w[some tags go here] }
+
+  before do
+    create(:tag, name: "some")
+    create(:tag, name: "tags")
+    create(:tag, name: "go")
+    create(:tag, name: "here")
+    create(:tag, name: "otherwise")
+    create(:tag, name: "nothing")
+  end
+
+  around do |example|
+    original = described_class.suggested_tags
+    described_class.suggested_tags = suggested_tags
+    example.run
+    described_class.suggested_tags = original
+  end
+
+  it "updates tags to use new boolean attribute (instead of settings)" do
+    described_class.new.run
+    expect(Tag.where(suggested: true).pluck(:name)).to include(*%w[some tags go here])
+  end
+end

spec/queries/tags/suggested_for_onboarding_spec.rb

@@ -0,0 +1,31 @@
+require "rails_helper"
+
+RSpec.describe Tags::SuggestedForOnboarding, type: :query do
+  subject(:result) { described_class.call }
+
+  let!(:suggested_tags) do
+    create(:tag, suggested: true)
+    create(:tag, suggested: true)
+    create(:tag, suggested: true)
+    Tag.where(suggested: true)
+  end
+
+  it "starts with Settings::General::SuggestedTags" do
+    expect(result).to match_array(suggested_tags)
+  end
+
+  context "when suggested tags aren't enough" do
+    let!(:supported) { create(:tag, supported: true) }
+
+    it "adds supported tags" do
+      expect(Tag.count < Tags::SuggestedForOnboarding::MAX).to be_truthy
+      expect(result).to include(*suggested_tags)
+      expect(result).to include(supported)
+    end
+
+    it "sorts suggested first, then supported" do
+      expect(suggested_tags).to include(result.first)
+      expect(result).to include(result.first)
+    end
+  end
+end

spec/requests/tags_spec.rb

@@ -220,12 +220,14 @@ RSpec.describe "Tags", proper_status: true do
       expect(response_tag["taggings_count"]).to eq(tag.taggings_count)
     end
 
-    it "returns only suggested tags" do
-      not_suggested_tag = create(:tag, name: "definitelynotasuggestedtag")
+    it "returns suggested and supported tags" do
+      not_suggested_but_supported = create(:tag, name: "notsuggestedbutsupported", supported: true, suggested: false)
+      neither_suggested_nor_supported = create(:tag, name: "definitelynotasuggestedtag", supported: false)
 
       get onboarding_tags_path, headers: headers
 
-      expect(response.parsed_body.filter { |t| t["name"] == not_suggested_tag.name }).to be_empty
+      expect(response.parsed_body.filter { |t| t["name"] == not_suggested_but_supported.name }).not_to be_empty
+      expect(response.parsed_body.filter { |t| t["name"] == neither_suggested_nor_supported.name }).to be_empty
     end
 
     it "sets the correct edge caching surrogate key for all tags" do