DEV: Cleanup todos from codebase (#30394)
This PR involves cleaning up the codebase from my (@keegangeorge's) todos. In particular: - Remove Form Template related todos (these are no longer in the roadmap) - Remove old left-over AI summarization related code after moving to AI (https://github.com/discourse/discourse-ai/pull/658) - Update one form template related spec
This commit is contained in:
Keegan George
GitHub
parent
fa9606016c
commit
380910aedd
|
|
@ -14,8 +14,6 @@
|
|||
</span>
|
||||
{{/if}}
|
||||
|
||||
{{! TODO(@keegan): Update implementation to use <ComboBox/> instead }}
|
||||
{{! Current using <select> as it integrates easily with FormData (will update in v2) }}
|
||||
<select
|
||||
name={{@id}}
|
||||
class="form-template-field__dropdown"
|
||||
|
|
|
|||
|
|
@ -29,8 +29,6 @@ export default class FormTemplateFieldMultiSelect extends Component {
|
|||
</span>
|
||||
{{/if}}
|
||||
|
||||
{{! TODO(@keegan): Update implementation to use <MultiSelect/> instead }}
|
||||
{{! Current using <select multiple> as it integrates easily with FormData (will update in v2) }}
|
||||
<select
|
||||
name={{@id}}
|
||||
required={{if @validations.required "required" ""}}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@
|
|||
width: 100%;
|
||||
}
|
||||
|
||||
// TODO(@keegan): Remove after updating dropdown/multi-select to use select-kit
|
||||
&__dropdown {
|
||||
border: 1px solid var(--primary-medium);
|
||||
border-radius: var(--d-input-border-radius);
|
||||
|
|
|
|||
|
|
@ -1,18 +0,0 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
# TODO(@keegan): Remove after removing SiteSetting.summarization_strategy
|
||||
|
||||
require "enum_site_setting"
|
||||
|
||||
class SummarizationStrategy < EnumSiteSetting
|
||||
def self.valid_value?(val)
|
||||
true
|
||||
end
|
||||
|
||||
def self.values
|
||||
@values ||=
|
||||
Summarization::Base.available_strategies.map do |strategy|
|
||||
{ name: strategy.display_name, value: strategy.model }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -1837,8 +1837,6 @@ en:
|
|||
summary_percent_filter: "When a user clicks 'Summarize This Topic', show the top % of posts"
|
||||
summary_max_results: "Maximum posts returned by 'Summarize This Topic'"
|
||||
summary_timeline_button: "Show a 'Summarize' button in the timeline"
|
||||
summarization_strategy: "Additional ways to summarize content registered by plugins"
|
||||
custom_summarization_allowed_groups: "Groups allowed to summarize contents using the `summarization_strategy`."
|
||||
|
||||
enable_personal_messages: "DEPRECATED, use the 'personal message enabled groups' setting instead. Allow trust level 1 (configurable via min trust to send messages) users to create messages and reply to messages. Note that staff can always send messages no matter what."
|
||||
personal_message_enabled_groups: "Allow users in these groups to CREATE personal messages. IMPORTANT: 1) all users can REPLY to messages. 2) Admins and mods can CREATE messages to any user. 3) Trust level groups include higher levels; choose trust_level_1 to allow TL1, TL2, TL3, TL4 but not allow TL0. 4) Group interaction settings override this setting for messaging specific groups."
|
||||
|
|
|
|||
|
|
@ -2720,18 +2720,6 @@ uncategorized:
|
|||
client: true
|
||||
default: false
|
||||
|
||||
summarization_strategy:
|
||||
client: true
|
||||
hidden: true
|
||||
default: ""
|
||||
enum: "SummarizationStrategy"
|
||||
validator: "SummarizationValidator"
|
||||
custom_summarization_allowed_groups:
|
||||
hidden: true
|
||||
type: group_list
|
||||
list_type: compact
|
||||
default: "3|13" # 3: @staff, 13: @trust_level_3
|
||||
|
||||
automatic_topic_heat_values: true
|
||||
|
||||
# View heat thresholds
|
||||
|
|
|
|||
|
|
@ -118,8 +118,6 @@ class DiscoursePluginRegistry
|
|||
|
||||
define_filtered_register :list_suggested_for_providers
|
||||
|
||||
define_filtered_register :summarization_strategies
|
||||
|
||||
define_filtered_register :post_action_notify_user_handlers
|
||||
|
||||
define_filtered_register :post_strippers
|
||||
|
|
|
|||
|
|
@ -1,102 +0,0 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
# TODO(@keegan): Remove after removing SiteSetting.summarization_strategy
|
||||
# Keeping because its still needed for SiteSetting to function.
|
||||
# Remove after settings are migrated to AI
|
||||
|
||||
# Base class that defines the interface that every summarization
|
||||
# strategy must implement.
|
||||
# Above each method, you'll find an explanation of what
|
||||
# it does and what it should return.
|
||||
|
||||
module Summarization
|
||||
class Base
|
||||
class << self
|
||||
def available_strategies
|
||||
DiscoursePluginRegistry.summarization_strategies
|
||||
end
|
||||
|
||||
def find_strategy(strategy_model)
|
||||
available_strategies.detect { |s| s.model == strategy_model }
|
||||
end
|
||||
|
||||
def selected_strategy
|
||||
return if SiteSetting.summarization_strategy.blank?
|
||||
|
||||
find_strategy(SiteSetting.summarization_strategy)
|
||||
end
|
||||
|
||||
def can_see_summary?(target, user)
|
||||
return false if SiteSetting.summarization_strategy.blank?
|
||||
return false if target.class == Topic && target.private_message?
|
||||
|
||||
has_cached_summary = SummarySection.exists?(target: target, meta_section_id: nil)
|
||||
return has_cached_summary if user.nil?
|
||||
|
||||
has_cached_summary || can_request_summary_for?(user)
|
||||
end
|
||||
|
||||
def can_request_summary_for?(user)
|
||||
return false unless user
|
||||
|
||||
user_group_ids = user.group_ids
|
||||
|
||||
SiteSetting.custom_summarization_allowed_groups_map.any? do |group_id|
|
||||
user_group_ids.include?(group_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# Some strategies could require other conditions to work correctly,
|
||||
# like site settings.
|
||||
# This method gets called when admins attempt to select it,
|
||||
# checking if we met those conditions.
|
||||
def correctly_configured?
|
||||
raise NotImplemented
|
||||
end
|
||||
|
||||
# Strategy name to display to admins in the available strategies dropdown.
|
||||
def display_name
|
||||
raise NotImplemented
|
||||
end
|
||||
|
||||
# If we don't meet the conditions to enable this strategy,
|
||||
# we'll display this hint as an error to admins.
|
||||
def configuration_hint
|
||||
raise NotImplemented
|
||||
end
|
||||
|
||||
# The idea behind this method is "give me a collection of texts,
|
||||
# and I'll handle the summarization to the best of my capabilities.".
|
||||
# It's important to emphasize the "collection of texts" part, which implies
|
||||
# it's not tied to any model and expects the "content" to be a hash instead.
|
||||
#
|
||||
# @param content { Hash } - Includes the content to summarize, plus additional
|
||||
# context to help the strategy produce a better result. Keys present in the content hash:
|
||||
# - resource_path (optional): Helps the strategy build links to the content in the summary (e.g. "/t/-/:topic_id/POST_NUMBER")
|
||||
# - content_title (optional): Provides guidance about what the content is about.
|
||||
# - contents (required): Array of hashes with content to summarize (e.g. [{ poster: "asd", id: 1, text: "This is a text" }])
|
||||
# All keys are required.
|
||||
# @param &on_partial_blk { Block - Optional } - If the strategy supports it, the passed block
|
||||
# will get called with partial summarized text as its generated.
|
||||
#
|
||||
# @param current_user { User } - User requesting the summary.
|
||||
#
|
||||
# @returns { Hash } - The summarized content, plus chunks if the content couldn't be summarized in one pass. Example:
|
||||
# {
|
||||
# summary: "This is the final summary",
|
||||
# chunks: [
|
||||
# { ids: [topic.first_post.post_number], summary: "this is the first chunk" },
|
||||
# { ids: [post_1.post_number, post_2.post_number], summary: "this is the second chunk" },
|
||||
# ],
|
||||
# }
|
||||
def summarize(content, current_user)
|
||||
raise NotImplemented
|
||||
end
|
||||
|
||||
# Returns the string we'll store in the selected strategy site setting.
|
||||
def model
|
||||
raise NotImplemented
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -1,21 +0,0 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
# TODO(@keegan): Remove after removing SiteSetting.summarization_strategy
|
||||
|
||||
class SummarizationValidator
|
||||
def initialize(opts = {})
|
||||
@opts = opts
|
||||
end
|
||||
|
||||
def valid_value?(val)
|
||||
strategy = Summarization::Base.find_strategy(val)
|
||||
|
||||
return true unless strategy
|
||||
|
||||
strategy.correctly_configured?.tap { |is_valid| @strategy = strategy unless is_valid }
|
||||
end
|
||||
|
||||
def error_message
|
||||
@strategy.configuration_hint
|
||||
end
|
||||
end
|
||||
|
|
@ -69,7 +69,7 @@ describe "Admin Customize Form Templates", type: :system do
|
|||
it "should prefill form data" do
|
||||
visit("/admin/customize/form-templates/#{form_template.id}")
|
||||
expect(form_template_page).to have_name_value(form_template.name)
|
||||
# TODO(@keegan) difficult to test the ace editor content, todo later
|
||||
expect(ace_editor).to have_content(form_template.template)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -26,6 +26,11 @@ module PageObjects
|
|||
visible: false,
|
||||
)
|
||||
end
|
||||
|
||||
def has_content?(content)
|
||||
editor_content = all(".ace_line").map(&:text).join("\n")
|
||||
editor_content == content
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in New Issue