DEV: Revert 3 commits for plugin API to add directory columns (#13423)

app/assets/javascripts/discourse/app/components/table-header-toggle.js

@@ -9,7 +9,6 @@ export default Component.extend({
   chevronIcon: null,
   columnIcon: null,
   translated: false,
-  automatic: false,
   onActiveRender: null,
 
   toggleProperties() {
@@ -32,9 +31,6 @@ export default Component.extend({
   },
   didReceiveAttrs() {
     this._super(...arguments);
-    if (!this.automatic && !this.translated) {
-      this.set("labelKey", this.field);
-    }
     this.set("id", `table-header-toggle-${this.field.replace(/\s/g, "")}`);
     this.toggleChevron();
   },

app/assets/javascripts/discourse/app/controllers/edit-user-directory-columns.js

@@ -58,7 +58,7 @@ export default Controller.extend(ModalFunctionality, {
       .forEach((column, index) => {
         column.setProperties({
           position: column.automatic_position || index + 1,
-          enabled: column.type === "automatic",
+          enabled: column.automatic,
         });
       });
     this.set("columns", resetColumns);

app/assets/javascripts/discourse/app/controllers/users.js

@@ -28,22 +28,13 @@ export default Controller.extend({
     this.set("nameInput", params.name);
     this.set("order", params.order);
 
-    const userFieldColumns = this.columns.filter(
+    const custom_field_columns = this.columns.filter((c) => !c.automatic);
-      (c) => c.type === "user_field"
+    const user_field_ids = custom_field_columns
-    );
+      .map((c) => c.user_field_id)
-    const userFieldIds = userFieldColumns.map((c) => c.user_field_id).join("|");
+      .join("|");
-
-    const pluginColumns = this.columns.filter((c) => c.type === "plugin");
-    const pluginColumnIds = pluginColumns.map((c) => c.id).join("|");
 
     return this.store
-      .find(
+      .find("directoryItem", Object.assign(params, { user_field_ids }))
-        "directoryItem",
-        Object.assign(params, {
-          user_field_ids: userFieldIds,
-          plugin_column_ids: pluginColumnIds,
-        })
-      )
       .then((model) => {
         const lastUpdatedAt = model.get("resultSetMeta.last_updated_at");
         this.setProperties({

app/assets/javascripts/discourse/app/helpers/directory-item-helpers.js

@@ -1,37 +0,0 @@
-import { htmlSafe } from "@ember/template";
-import { number } from "discourse/lib/formatter";
-import { registerUnbound } from "discourse-common/lib/helpers";
-import I18n from "I18n";
-
-registerUnbound("mobile-directory-item-label", function (args) {
-  // Args should include key/values { item, column }
-  const count = args.item.get(args.column.name);
-  return htmlSafe(I18n.t(`directory.${args.column.name}`, { count }));
-});
-
-registerUnbound("directory-item-value", function (args) {
-  // Args should include key/values { item, column }
-  return htmlSafe(
-    `<span class='number'>${number(args.item.get(args.column.name))}</span>`
-  );
-});
-
-registerUnbound("directory-item-user-field-value", function (args) {
-  // Args should include key/values { item, column }
-  const value =
-    args.item.user && args.item.user.user_fields
-      ? args.item.user.user_fields[args.column.user_field_id]
-      : null;
-  const content = value || "-";
-  return htmlSafe(`<span class='user-field-value'>${content}</span>`);
-});
-
-registerUnbound("directory-column-is-automatic", function (args) {
-  // Args should include key/values { column }
-  return args.column.type === "automatic";
-});
-
-registerUnbound("directory-column-is-user-field", function (args) {
-  // Args should include key/values { column }
-  return args.column.type === "user_field";
-});

app/assets/javascripts/discourse/app/helpers/directory-item-label.js

@@ -0,0 +1,10 @@
+import { htmlSafe } from "@ember/template";
+import { registerUnbound } from "discourse-common/lib/helpers";
+import I18n from "I18n";
+
+export default registerUnbound("mobile-directory-item-label", function (args) {
+  // Args should include key/values { item, column }
+
+  const count = args.item.get(args.column.name);
+  return htmlSafe(I18n.t(`directory.${args.column.name}`, { count }));
+});

app/assets/javascripts/discourse/app/helpers/directory-item-user-field-value.js

@@ -0,0 +1,16 @@
+import { htmlSafe } from "@ember/template";
+import { registerUnbound } from "discourse-common/lib/helpers";
+
+export default registerUnbound(
+  "directory-item-user-field-value",
+  function (args) {
+    // Args should include key/values { item, column }
+
+    const value =
+      args.item.user && args.item.user.user_fields
+        ? args.item.user.user_fields[args.column.user_field_id]
+        : null;
+    const content = value || "-";
+    return htmlSafe(`<span class='user-field-value'>${content}</span>`);
+  }
+);

app/assets/javascripts/discourse/app/helpers/directory-item-value.js

@@ -0,0 +1,11 @@
+import { htmlSafe } from "@ember/template";
+import { registerUnbound } from "discourse-common/lib/helpers";
+import { number } from "discourse/lib/formatter";
+
+export default registerUnbound("directory-item-value", function (args) {
+  // Args should include key/values { item, column }
+
+  return htmlSafe(
+    `<span class='number'>${number(args.item.get(args.column.name))}</span>`
+  );
+});

app/assets/javascripts/discourse/app/templates/components/directory-item.hbs

@@ -1,10 +1,10 @@
 <td>{{user-info user=item.user}}</td>
 {{#each columns as |column|}}
   <td>
-    {{#if (directory-column-is-user-field column=column)}}
+    {{#if column.automatic}}
-      {{directory-item-user-field-value item=item column=column}}
-    {{else}}
       {{directory-item-value item=item column=column}}
+    {{else}}
+      {{directory-item-user-field-value item=item column=column}}
     {{/if}}
   </td>
 {{/each}}

app/assets/javascripts/discourse/app/templates/components/directory-table.hbs

@@ -7,7 +7,6 @@
         icon=column.icon
         order=order
         asc=asc
-        automatic=(directory-column-is-automatic column=column)
         translated=column.user_field_id
         onActiveRender=setActiveHeader
       }}

app/assets/javascripts/discourse/app/templates/group-index.hbs

@@ -35,11 +35,11 @@
                 {{d-button action=(action "bulkClearAll") label="topics.bulk.clear_all"}}
               </th>
             {{/if}}
-            {{table-header-toggle order=order asc=asc field="username_lower" labelKey="username" class="username" automatic=true}}
+            {{table-header-toggle order=order asc=asc field="username_lower" labelKey="username" class="username"}}
             <th class="group-owner"></th>
-            {{table-header-toggle order=order asc=asc field="added_at" labelKey="groups.member_added" automatic=true}}
+            {{table-header-toggle order=order asc=asc field="added_at" labelKey="groups.member_added"}}
-            {{table-header-toggle order=order asc=asc field="last_posted_at" labelKey="last_post" automatic=true}}
+            {{table-header-toggle order=order asc=asc field="last_posted_at" labelKey="last_post"}}
-            {{table-header-toggle order=order asc=asc field="last_seen_at" labelKey="last_seen" automatic=true}}
+            {{table-header-toggle order=order asc=asc field="last_seen_at" labelKey="last_seen"}}
             <th>
               {{#if isBulk}}
                 {{group-member-dropdown

app/assets/javascripts/discourse/app/templates/group-requests.hbs

@@ -12,8 +12,8 @@
     {{#load-more selector=".group-members tr" action=(action "loadMore")}}
       <table class="group-members">
         <thead>
-          {{table-header-toggle order=order asc=asc field="username_lower" labelKey="username" automatic=true}}
+          {{table-header-toggle order=order asc=asc field="username_lower" labelKey="username"}}
-          {{table-header-toggle order=order asc=asc field="requested_at" labelKey="groups.member_requested" automatic=true}}
+          {{table-header-toggle order=order asc=asc field="requested_at" labelKey="groups.member_requested"}}
           <th>{{i18n "groups.requests.reason"}}</th>
           <th></th>
           <th></th>

app/assets/javascripts/discourse/app/templates/mobile/components/directory-item.hbs

@@ -1,7 +1,7 @@
 {{user-info user=item.user}}
 
 {{#each columns as |column|}}
-  {{#if (directory-column-is-automatic column=column)}}
+  {{#if column.automatic}}
     <div class="user-stat">
       <span class="value">
         {{directory-item-value item=item column=column}}

app/assets/javascripts/discourse/app/templates/modal/edit-user-directory-columns.hbs

@@ -8,12 +8,10 @@
           <div class="left-content">
             <label class="column-name">
               {{input type="checkbox" checked=column.enabled}}
-              {{#if (directory-column-is-automatic column=column)}}
+              {{#if column.automatic}}
                 {{directory-table-header-title field=column.name labelKey=labelKey icon=column.icon}}
-              {{else if (directory-column-is-user-field column=column)}}
-                {{directory-table-header-title field=column.user_field.name translated=true}}
               {{else}}
-                {{directory-table-header-title field=(i18n column.name) translated=true}}
+                {{directory-table-header-title field=column.user_field.name translated=true}}
               {{/if}}
             </label>
           </div>

app/assets/javascripts/discourse/tests/helpers/create-pretender.js

@@ -944,7 +944,7 @@ export function applyDefaultHandlers(pretender) {
         {
           id: 1,
           name: "likes_received",
-          type: "automatic",
+          automatic: true,
           enabled: true,
           automatic_position: 1,
           position: 1,
@@ -954,7 +954,7 @@ export function applyDefaultHandlers(pretender) {
         {
           id: 2,
           name: "likes_given",
-          type: "automatic",
+          automatic: true,
           enabled: true,
           automatic_position: 2,
           position: 2,
@@ -964,7 +964,7 @@ export function applyDefaultHandlers(pretender) {
         {
           id: 3,
           name: "topic_count",
-          type: "automatic",
+          automatic: true,
           enabled: true,
           automatic_position: 3,
           position: 3,
@@ -974,7 +974,7 @@ export function applyDefaultHandlers(pretender) {
         {
           id: 4,
           name: "post_count",
-          type: "automatic",
+          automatic: true,
           enabled: true,
           automatic_position: 4,
           position: 4,
@@ -984,7 +984,7 @@ export function applyDefaultHandlers(pretender) {
         {
           id: 5,
           name: "topics_entered",
-          type: "automatic",
+          automatic: true,
           enabled: true,
           automatic_position: 5,
           position: 5,
@@ -994,7 +994,7 @@ export function applyDefaultHandlers(pretender) {
         {
           id: 6,
           name: "posts_read",
-          type: "automatic",
+          automatic: true,
           enabled: true,
           automatic_position: 6,
           position: 6,
@@ -1004,7 +1004,7 @@ export function applyDefaultHandlers(pretender) {
         {
           id: 7,
           name: "days_visited",
-          type: "automatic",
+          automatic: true,
           enabled: true,
           automatic_position: 7,
           position: 7,
@@ -1014,7 +1014,7 @@ export function applyDefaultHandlers(pretender) {
         {
           id: 9,
           name: null,
-          type: "user_field",
+          automatic: false,
           enabled: false,
           automatic_position: null,
           position: 8,

app/assets/javascripts/discourse/tests/setup-tests.js

@@ -232,7 +232,7 @@ function setupTestsCommon(application, container, config) {
     PreloadStore.store(
       "directoryColumns",
       JSON.parse(
-        '[{"name":"likes_given","type":"automatic","icon":"heart","user_field_id":null},{"name":"posts_read","type":"automatic","icon":null,"user_field_id":null},{"name":"likes_received","type":"automatic","icon":"heart","user_field_id":null},{"name":"topic_count","type":"automatic","icon":null,"user_field_id":null},{"name":"post_count","type":"automatic","icon":null,"user_field_id":null},{"name":"topics_entered","type":"automatic","icon":null,"user_field_id":null},{"name":"days_visited","type":"automatic","icon":null,"user_field_id":null},{"name":"Favorite Color","type":"user_field","icon":null,"user_field_id":3}]'
+        '[{"name":"likes_given","automatic":true,"icon":"heart","user_field_id":null},{"name":"posts_read","automatic":true,"icon":null,"user_field_id":null},{"name":"likes_received","automatic":true,"icon":"heart","user_field_id":null},{"name":"topic_count","automatic":true,"icon":null,"user_field_id":null},{"name":"post_count","automatic":true,"icon":null,"user_field_id":null},{"name":"topics_entered","automatic":true,"icon":null,"user_field_id":null},{"name":"days_visited","automatic":true,"icon":null,"user_field_id":null},{"name":"Favorite Color","automatic":false,"icon":null,"user_field_id":3}]'
       )
     );
 

app/controllers/application_controller.rb

@@ -618,18 +618,16 @@ class ApplicationController < ActionController::Base
   end
 
   def directory_columns_json
-    types = DirectoryColumn.types
     DirectoryColumn
       .left_joins(:user_field)
       .where(enabled: true)
       .order(:position)
-      .pluck('directory_columns.id',
+      .pluck('directory_columns.name',
-             'directory_columns.name',
+             'directory_columns.automatic',
-             'directory_columns.type',
              'directory_columns.icon',
              'user_fields.id',
              'user_fields.name')
-      .map { |column| { id: column[0], name: column[1] || column[5], type: types.key(column[2]), icon: column[3], user_field_id: column[4] } }
+      .map { |column| { name: column[0] || column[4], automatic: column[1], icon: column[2], user_field_id: column[3] } }
       .to_json
   end
 

app/controllers/directory_columns_controller.rb

@@ -50,7 +50,7 @@ class DirectoryColumnsController < ApplicationController
       new_directory_column_attrs.push({
         user_field_id: user_field.id,
         enabled: false,
-        type: DirectoryColumn.types[:user_field],
+        automatic: false,
         position: next_position
       })
 

app/controllers/directory_items_controller.rb

@@ -26,14 +26,13 @@ class DirectoryItemsController < ApplicationController
       result = result.references(:user).where.not(users: { username: params[:exclude_usernames].split(",") })
     end
 
-    order = params[:order] || DirectoryColumn.automatic_column_names.first
+    order = params[:order] || DirectoryItem.headings.first
     dir = params[:asc] ? 'ASC' : 'DESC'
-    if DirectoryColumn.active_column_names.include?(order.to_sym)
+    if DirectoryItem.headings.include?(order.to_sym)
       result = result.order("directory_items.#{order} #{dir}, directory_items.id")
     elsif params[:order] === 'username'
       result = result.order("users.#{order} #{dir}, directory_items.id")
     else
-      # Ordering by user field value
       user_field = UserField.find_by(name: params[:order])
       if user_field
         result = result
@@ -99,10 +98,6 @@ class DirectoryItemsController < ApplicationController
       serializer_opts[:user_field_ids] = params[:user_field_ids]&.split("|")&.map(&:to_i)
     end
 
-    if params[:plugin_column_ids]
-      serializer_opts[:plugin_column_ids] = params[:plugin_column_ids]&.split("|")&.map(&:to_i)
-    end
-
     serialized = serialize_data(result, DirectoryItemSerializer, serializer_opts)
     render_json_dump(directory_items: serialized,
                      meta: {

app/models/directory_column.rb

@@ -1,37 +1,5 @@
 # frozen_string_literal: true
 
 class DirectoryColumn < ActiveRecord::Base
-  self.inheritance_column = nil
-
-  def self.automatic_column_names
-    @automatic_column_names ||= [:likes_received,
-                   :likes_given,
-                   :topics_entered,
-                   :topic_count,
-                   :post_count,
-                   :posts_read,
-                   :days_visited]
-  end
-
-  def self.active_column_names
-    DirectoryColumn.where(type: [:automatic, :plugin]).where(enabled: true).pluck(:name).map(&:to_sym)
-  end
-
-  @@plugin_directory_columns = []
-
-  enum type: { automatic: 0, user_field: 1, plugin: 2 }
-
   belongs_to :user_field
-
-  def self.add_plugin_directory_column(name)
-    @@plugin_directory_columns << name
-  end
-
-  def self.plugin_directory_columns
-    @@plugin_directory_columns
-  end
-
-  def self.clear_plugin_directory_columns
-    @@plugin_directory_columns = []
-  end
 end

app/models/directory_item.rb

@@ -4,7 +4,15 @@ class DirectoryItem < ActiveRecord::Base
   belongs_to :user
   has_one :user_stat, foreign_key: :user_id, primary_key: :user_id
 
-  @@plugin_queries = []
+  def self.headings
+    @headings ||= [:likes_received,
+                   :likes_given,
+                   :topics_entered,
+                   :topic_count,
+                   :post_count,
+                   :posts_read,
+                   :days_visited]
+  end
 
   def self.period_types
     @types ||= Enum.new(all: 1,
@@ -26,18 +34,6 @@ class DirectoryItem < ActiveRecord::Base
     Time.zone.at(val.to_i)
   end
 
-  def self.add_plugin_query(details)
-    @@plugin_queries << details
-  end
-
-  def self.plugin_queries
-    @@plugin_queries
-  end
-
-  def self.clear_plugin_queries
-    @@plugin_queries = []
-  end
-
   def self.refresh_period!(period_type, force: false)
 
     Discourse.redis.set("directory_#{period_types[period_type]}", Time.zone.now.to_i)
@@ -57,26 +53,30 @@ class DirectoryItem < ActiveRecord::Base
 
     ActiveRecord::Base.transaction do
       # Delete records that belonged to users who have been deleted
-      DB.exec("DELETE FROM directory_items
+      DB.exec "DELETE FROM directory_items
                 USING directory_items di
                 LEFT JOIN users u ON (u.id = user_id AND u.active AND u.silenced_till IS NULL AND u.id > 0)
                 WHERE di.id = directory_items.id AND
                       u.id IS NULL AND
-                      di.period_type = :period_type", period_type: period_types[period_type])
+                      di.period_type = :period_type", period_type: period_types[period_type]
 
       # Create new records for users who don't have one yet
-
+      DB.exec "INSERT INTO directory_items(period_type, user_id, likes_received, likes_given, topics_entered, days_visited, posts_read, topic_count, post_count)
-      column_names = DirectoryColumn.automatic_column_names + DirectoryColumn.plugin_directory_columns
-      DB.exec("INSERT INTO directory_items(period_type, user_id, #{column_names.map(&:to_s).join(", ")})
                 SELECT
                     :period_type,
                     u.id,
-                    #{Array.new(column_names.count) { |_| 0 }.join(", ") }
+                    0,
+                    0,
+                    0,
+                    0,
+                    0,
+                    0,
+                    0
                 FROM users u
                 LEFT JOIN directory_items di ON di.user_id = u.id AND di.period_type = :period_type
                 WHERE di.id IS NULL AND u.id > 0 AND u.silenced_till IS NULL AND u.active
                 #{SiteSetting.must_approve_users ? 'AND u.approved' : ''}
-              ", period_type: period_types[period_type])
+      ", period_type: period_types[period_type]
 
       # Calculate new values and update records
       #
@@ -84,18 +84,7 @@ class DirectoryItem < ActiveRecord::Base
       # TODO
       # WARNING: post_count is a wrong name, it should be reply_count (excluding topic post)
       #
-      #
+      DB.exec "WITH x AS (SELECT
-      query_args = {
-        period_type: period_types[period_type],
-        since: since,
-        like_type: UserAction::LIKE,
-        was_liked_type: UserAction::WAS_LIKED,
-        new_topic_type: UserAction::NEW_TOPIC,
-        reply_type: UserAction::REPLY,
-        regular_post_type: Post.types[:regular]
-      }
-
-      DB.exec("WITH x AS (SELECT
                     u.id user_id,
                     SUM(CASE WHEN p.id IS NOT NULL AND t.id IS NOT NULL AND ua.action_type = :was_liked_type THEN 1 ELSE 0 END) likes_received,
                     SUM(CASE WHEN p.id IS NOT NULL AND t.id IS NOT NULL AND ua.action_type = :like_type THEN 1 ELSE 0 END) likes_given,
@@ -135,12 +124,13 @@ class DirectoryItem < ActiveRecord::Base
         di.post_count <> x.post_count )
 
       ",
-              query_args
+                  period_type: period_types[period_type],
-             )
+                  since: since,
-
+                  like_type: UserAction::LIKE,
-      plugin_queries.each do |plugin_query|
+                  was_liked_type: UserAction::WAS_LIKED,
-        DB.exec(plugin_query, query_args)
+                  new_topic_type: UserAction::NEW_TOPIC,
-      end
+                  reply_type: UserAction::REPLY,
+                  regular_post_type: Post.types[:regular]
 
       if period_type == :all
         DB.exec <<~SQL

app/serializers/directory_column_serializer.rb

@@ -3,7 +3,7 @@
 class DirectoryColumnSerializer < ApplicationSerializer
   attributes :id,
              :name,
-             :type,
+             :automatic,
              :enabled,
              :automatic_position,
              :position,

app/serializers/directory_item_serializer.rb

@@ -20,7 +20,7 @@ class DirectoryItemSerializer < ApplicationSerializer
              :time_read
 
   has_one :user, embed: :objects, serializer: UserSerializer
-  attributes *DirectoryColumn.active_column_names
+  attributes *DirectoryItem.headings
 
   def id
     object.user_id

db/migrate/20210609133551_add_type_to_directory_columns.rb

@@ -1,18 +0,0 @@
-# frozen_string_literal: true
-
-class AddTypeToDirectoryColumns < ActiveRecord::Migration[6.1]
-  def up
-    add_column :directory_columns, :type, :integer, default: 0, null: false
-
-    DB.exec(
-      <<~SQL
-        UPDATE directory_columns
-        SET type = CASE WHEN automatic THEN 0 ELSE 1 END;
-      SQL
-    )
-  end
-
-  def down
-    remove_column :directory_columns, :type, :integer, default: 0, null: false
-  end
-end

db/post_migrate/20210609152431_remove_directory_column_automatic.rb

@@ -1,11 +0,0 @@
-# frozen_string_literal: true
-
-class RemoveDirectoryColumnAutomatic < ActiveRecord::Migration[6.1]
-  def up
-    remove_column :directory_columns, :automatic
-  end
-
-  def down
-    raise ActiveRecord::IrreversibleMigration
-  end
-end

lib/plugin/instance.rb

@@ -373,17 +373,6 @@ class Plugin::Instance
     assets
   end
 
-  def add_directory_column(column_name, query:, icon: nil)
-    validate_directory_column_name(column_name)
-    directory_column = DirectoryColumn
-      .find_or_create_by(name: column_name, icon: icon, type: DirectoryColumn.types[:plugin]) do |column|
-        column.position = DirectoryColumn.maximum("position") + 1
-        column.enabled = false
-      end
-    DirectoryColumn.add_plugin_directory_column(column_name)
-    DirectoryItem.add_plugin_query(query)
-  end
-
   def delete_extra_automatic_assets(good_paths)
     return unless Dir.exists? auto_generated_path
 
@@ -975,11 +964,6 @@ class Plugin::Instance
 
   private
 
-  def validate_directory_column_name(column_name)
-    match = /^[_a-z]+$/.match(column_name)
-    raise "Invalid directory column name '#{column_name}'. Can only contain a-z and underscores" unless match
-  end
-
   def write_asset(path, contents)
     unless File.exists?(path)
       ensure_directory(path)

spec/components/plugin/instance_spec.rb

@@ -600,33 +600,4 @@ describe Plugin::Instance do
       expect(ApiKeyScope.scope_mappings.dig(:groups, :create, :actions)).to contain_exactly(*actions)
     end
   end
-
-  describe '#add_directory_column' do
-    let!(:plugin) { Plugin::Instance.new }
-
-    after do
-      DirectoryItem.clear_plugin_queries
-      DirectoryColumn.clear_plugin_directory_columns
-    end
-
-    it 'creates a directory column record' do
-      plugin.add_directory_column('random_c', query: "SELECT COUNT(*) FROM users", icon: 'recycle')
-
-      expect(DirectoryColumn.find_by(name: 'random_c', icon: 'recycle', enabled: false).present?).to be(true)
-    end
-
-    it 'errors when the column_name contains invalid characters' do
-      expect {
-        plugin.add_directory_column('Capital', query: "SELECT COUNT(*) FROM users", icon: 'recycle')
-      }.to raise_error(RuntimeError)
-
-      expect {
-        plugin.add_directory_column('has space', query: "SELECT COUNT(*) FROM users", icon: 'recycle')
-      }.to raise_error(RuntimeError)
-
-      expect {
-        plugin.add_directory_column('has_number_1', query: "SELECT COUNT(*) FROM users", icon: 'recycle')
-      }.to raise_error(RuntimeError)
-    end
-  end
 end

spec/requests/admin/user_fields_controller_spec.rb

@@ -130,7 +130,7 @@ describe Admin::UserFieldsController do
         DirectoryColumn.create(
           user_field_id: user_field.id,
           enabled: false,
-          type: DirectoryColumn.types[:user_field],
+          automatic: false,
           position: next_position
         )
         expect {