diff --git a/.github/workflows/plugin-linting.yml b/.github/workflows/plugin-linting.yml index c807794..6d2bb97 100644 --- a/.github/workflows/plugin-linting.yml +++ b/.github/workflows/plugin-linting.yml @@ -55,3 +55,12 @@ jobs: - name: Rubocop if: ${{ !cancelled() }} run: bundle exec rubocop . + + - name: Syntax Tree + if: ${{ !cancelled() }} + run: | + if test -f .streerc; then + bundle exec stree check Gemfile $(git ls-files '*.rb') $(git ls-files '*.rake') + else + echo "Stree config not detected for this repository. Skipping." + fi diff --git a/.github/workflows/plugin-tests.yml b/.github/workflows/plugin-tests.yml index 9d390bc..f30a5be 100644 --- a/.github/workflows/plugin-tests.yml +++ b/.github/workflows/plugin-tests.yml @@ -80,7 +80,7 @@ jobs: - name: Get yarn cache directory id: yarn-cache-dir - run: echo "::set-output name=dir::$(yarn cache dir)" + run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT - name: Yarn cache uses: actions/cache@v3 @@ -130,7 +130,7 @@ jobs: shell: bash run: | if [ 0 -lt $(find plugins/${{ github.event.repository.name }}/spec -type f -name "*.rb" 2> /dev/null | wc -l) ]; then - echo "::set-output name=files_exist::true" + echo "files_exist=true" >> $GITHUB_OUTPUT fi - name: Plugin RSpec @@ -142,7 +142,7 @@ jobs: shell: bash run: | if [ 0 -lt $(find plugins/${{ github.event.repository.name }}/test/javascripts -type f \( -name "*.js" -or -name "*.es6" \) 2> /dev/null | wc -l) ]; then - echo "::set-output name=files_exist::true" + echo "files_exist=true" >> $GITHUB_OUTPUT fi - name: Plugin QUnit diff --git a/.rubocop.yml b/.rubocop.yml index d46296c..fb14dfa 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,2 +1,2 @@ inherit_gem: - rubocop-discourse: default.yml + rubocop-discourse: stree-compat.yml diff --git a/.streerc b/.streerc new file mode 100644 index 0000000..0bc4379 --- /dev/null +++ b/.streerc @@ -0,0 +1,2 @@ +--print-width=100 +--plugins=plugin/trailing_comma diff --git a/Gemfile b/Gemfile index 7da32ec..31d8bf7 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,8 @@ # frozen_string_literal: true -source 'https://rubygems.org' +source "https://rubygems.org" group :development do - gem 'rubocop-discourse' + gem "rubocop-discourse" + gem "syntax_tree" end diff --git a/Gemfile.lock b/Gemfile.lock index 87ff84d..348ff70 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,6 +6,7 @@ GEM parallel (1.22.1) parser (3.1.2.1) ast (~> 2.4.1) + prettier_print (1.2.0) rainbow (3.1.1) regexp_parser (2.6.0) rexml (3.2.5) @@ -27,6 +28,8 @@ GEM rubocop-rspec (2.13.2) rubocop (~> 1.33) ruby-progressbar (1.11.0) + syntax_tree (5.1.0) + prettier_print (>= 1.2.0) unicode-display_width (2.3.0) PLATFORMS @@ -34,6 +37,7 @@ PLATFORMS DEPENDENCIES rubocop-discourse + syntax_tree BUNDLED WITH 2.1.4 diff --git a/app/controllers/house_ads_controller.rb b/app/controllers/house_ads_controller.rb index c1e140a..ab4b76a 100644 --- a/app/controllers/house_ads_controller.rb +++ b/app/controllers/house_ads_controller.rb @@ -5,10 +5,7 @@ module ::AdPlugin requires_plugin AdPlugin.plugin_name def index - render_json_dump( - house_ads: HouseAd.all.map(&:to_hash), - settings: HouseAdSetting.all - ) + render_json_dump(house_ads: HouseAd.all.map(&:to_hash), settings: HouseAdSetting.all) end def show @@ -17,11 +14,7 @@ module ::AdPlugin def create ad = HouseAd.create(house_ad_params) - if ad.valid? - render_json_dump(house_ad: ad.to_hash) - else - render_json_error(ad) - end + ad.valid? ? render_json_dump(house_ad: ad.to_hash) : render_json_error(ad) end def update @@ -31,18 +24,14 @@ module ::AdPlugin ad = HouseAd.create(house_ad_params.except(:id)) end - if ad.valid? - render_json_dump(house_ad: ad.to_hash) - else - render_json_error(ad) - end + ad.valid? ? render_json_dump(house_ad: ad.to_hash) : render_json_error(ad) end def destroy if ad = HouseAd.find(house_ad_params[:id]) ad.destroy else - render_json_error(I18n.t('not_found'), status: 404) + render_json_error(I18n.t("not_found"), status: 404) end end diff --git a/app/models/house_ad.rb b/app/models/house_ad.rb index fb54820..68ec19d 100644 --- a/app/models/house_ad.rb +++ b/app/models/house_ad.rb @@ -37,7 +37,7 @@ module ::AdPlugin end def self.alloc_id - DistributedMutex.synchronize('adplugin-house-ad-id') do + DistributedMutex.synchronize("adplugin-house-ad-id") do max_id = AdPlugin.pstore_get("ad:_id") max_id = 1 unless max_id AdPlugin.pstore_set("ad:_id", max_id + 1) @@ -46,7 +46,7 @@ module ::AdPlugin end def self.find(id) - if r = AdPlugin::pstore_get("ad:#{id}") + if r = AdPlugin.pstore_get("ad:#{id}") from_hash(r) else nil @@ -54,18 +54,18 @@ module ::AdPlugin end def self.all - PluginStoreRow.where(plugin_name: AdPlugin.plugin_name) + PluginStoreRow + .where(plugin_name: AdPlugin.plugin_name) .where("key LIKE 'ad:%'") .where("key != 'ad:_id'") - .map do |psr| - from_hash(PluginStore.cast_value(psr.type_name, psr.value)) - end.sort_by { |ad| ad.id } + .map { |psr| from_hash(PluginStore.cast_value(psr.type_name, psr.value)) } + .sort_by { |ad| ad.id } end def save if self.valid? self.id = self.class.alloc_id if self.id.to_i <= 0 - AdPlugin::pstore_set("ad:#{id}", to_hash) + AdPlugin.pstore_set("ad:#{id}", to_hash) self.class.publish_if_ads_enabled true else @@ -80,15 +80,11 @@ module ::AdPlugin end def to_hash - { - id: @id, - name: @name, - html: @html - } + { id: @id, name: @name, html: @html } end def destroy - AdPlugin::pstore_delete("ad:#{id}") + AdPlugin.pstore_delete("ad:#{id}") self.class.publish_if_ads_enabled end @@ -97,6 +93,5 @@ module ::AdPlugin AdPlugin::HouseAdSetting.publish_settings end end - end end diff --git a/app/models/house_ad_setting.rb b/app/models/house_ad_setting.rb index 3911028..3f9bef8 100644 --- a/app/models/house_ad_setting.rb +++ b/app/models/house_ad_setting.rb @@ -3,67 +3,65 @@ module ::AdPlugin class HouseAdSetting DEFAULTS = { - topic_list_top: '', - topic_above_post_stream: '', - topic_above_suggested: '', - post_bottom: '', - topic_list_between: '' + topic_list_top: "", + topic_above_post_stream: "", + topic_above_suggested: "", + post_bottom: "", + topic_list_between: "", } def self.all settings = DEFAULTS.dup - PluginStoreRow.where(plugin_name: AdPlugin.plugin_name) + PluginStoreRow + .where(plugin_name: AdPlugin.plugin_name) .where("key LIKE 'ad-setting:%'") - .each do |psr| - settings[psr.key[11..-1].to_sym] = psr.value - end + .each { |psr| settings[psr.key[11..-1].to_sym] = psr.value } settings end def self.settings_and_ads settings = AdPlugin::HouseAdSetting.all - ad_names = settings.values.map { |v| v.split('|') }.flatten.uniq + ad_names = settings.values.map { |v| v.split("|") }.flatten.uniq ads = AdPlugin::HouseAd.all.select { |ad| ad_names.include?(ad.name) } { - settings: settings.merge( - after_nth_post: SiteSetting.house_ads_after_nth_post, - after_nth_topic: SiteSetting.house_ads_after_nth_topic, - house_ads_frequency: SiteSetting.house_ads_frequency - ), - creatives: ads.inject({}) { |h, ad| h[ad.name] = ad.html; h } + settings: + settings.merge( + after_nth_post: SiteSetting.house_ads_after_nth_post, + after_nth_topic: SiteSetting.house_ads_after_nth_topic, + house_ads_frequency: SiteSetting.house_ads_frequency, + ), + creatives: + ads.inject({}) do |h, ad| + h[ad.name] = ad.html + h + end, } end def self.update(setting_name, value) - unless DEFAULTS.keys.include?(setting_name.to_sym) - raise Discourse::NotFound - end + raise Discourse::NotFound unless DEFAULTS.keys.include?(setting_name.to_sym) - ad_names = value&.split('|') || [] + ad_names = value&.split("|") || [] - if value && ad_names.any? { |v| v !~ HouseAd::NAME_REGEX } - raise Discourse::InvalidParameters - end + raise Discourse::InvalidParameters if value && ad_names.any? { |v| v !~ HouseAd::NAME_REGEX } - unless ad_names.empty? - ad_names = (HouseAd.all.map(&:name) & ad_names) - end + ad_names = (HouseAd.all.map(&:name) & ad_names) unless ad_names.empty? - new_value = ad_names.join('|') + new_value = ad_names.join("|") if value.nil? || new_value == DEFAULTS[setting_name.to_sym] - AdPlugin::pstore_delete("ad-setting:#{setting_name}") + AdPlugin.pstore_delete("ad-setting:#{setting_name}") else - AdPlugin::pstore_set("ad-setting:#{setting_name}", new_value) + AdPlugin.pstore_set("ad-setting:#{setting_name}", new_value) end publish_settings end def self.publish_settings - MessageBus.publish('/site/house-creatives', settings_and_ads) + MessageBus.publish("/site/house-creatives", settings_and_ads) end end end diff --git a/plugin.rb b/plugin.rb index 1cd7689..72f7034 100755 --- a/plugin.rb +++ b/plugin.rb @@ -9,13 +9,13 @@ register_asset "stylesheets/adplugin.scss" -add_admin_route 'admin.adplugin.house_ads.title', 'houseAds' +add_admin_route "admin.adplugin.house_ads.title", "houseAds" enabled_site_setting :discourse_adplugin_enabled module ::AdPlugin def self.plugin_name - 'discourse-adplugin'.freeze + "discourse-adplugin".freeze end def self.pstore_get(key) @@ -32,11 +32,11 @@ module ::AdPlugin end after_initialize do - require_dependency File.expand_path('../app/models/house_ad', __FILE__) - require_dependency File.expand_path('../app/models/house_ad_setting', __FILE__) - require_dependency File.expand_path('../app/controllers/house_ads_controller', __FILE__) - require_dependency File.expand_path('../app/controllers/house_ad_settings_controller', __FILE__) - require_dependency 'application_controller' + require_dependency File.expand_path("../app/models/house_ad", __FILE__) + require_dependency File.expand_path("../app/models/house_ad_setting", __FILE__) + require_dependency File.expand_path("../app/controllers/house_ads_controller", __FILE__) + require_dependency File.expand_path("../app/controllers/house_ad_settings_controller", __FILE__) + require_dependency "application_controller" add_to_serializer :site, :house_creatives do AdPlugin::HouseAdSetting.settings_and_ads @@ -45,7 +45,7 @@ after_initialize do add_to_serializer :topic_view, :tags_disable_ads do return false if !SiteSetting.tagging_enabled || !SiteSetting.no_ads_for_tags.present? return false if object.topic.tags.empty? - !(SiteSetting.no_ads_for_tags.split('|') & object.topic.tags.map(&:name)).empty? + !(SiteSetting.no_ads_for_tags.split("|") & object.topic.tags.map(&:name)).empty? end class ::AdstxtController < ::ApplicationController @@ -59,18 +59,18 @@ after_initialize do end class AdPlugin::Engine < ::Rails::Engine - engine_name 'adplugin' + engine_name "adplugin" isolate_namespace AdPlugin end AdPlugin::Engine.routes.draw do - root to: 'house_ads#index' - resources :house_creatives, only: [:index, :show, :create, :update, :destroy], controller: 'house_ads' - resources :house_settings, only: [:update], controller: 'house_ad_settings' + root to: "house_ads#index" + resources :house_creatives, only: %i[index show create update destroy], controller: "house_ads" + resources :house_settings, only: [:update], controller: "house_ad_settings" end Discourse::Application.routes.append do - get '/ads.txt' => "adstxt#index" - mount ::AdPlugin::Engine, at: '/admin/plugins/pluginad', constraints: AdminConstraint.new + get "/ads.txt" => "adstxt#index" + mount ::AdPlugin::Engine, at: "/admin/plugins/pluginad", constraints: AdminConstraint.new end end diff --git a/spec/models/house_ad_setting_spec.rb b/spec/models/house_ad_setting_spec.rb index 1e21071..cea2bd4 100644 --- a/spec/models/house_ad_setting_spec.rb +++ b/spec/models/house_ad_setting_spec.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -require 'rails_helper' +require "rails_helper" describe AdPlugin::HouseAdSetting do let(:defaults) { AdPlugin::HouseAdSetting::DEFAULTS } - describe '#all' do + describe "#all" do subject { AdPlugin::HouseAdSetting.all } it "returns defaults when nothing has been set" do @@ -13,60 +13,58 @@ describe AdPlugin::HouseAdSetting do end it "returns defaults and overrides" do - AdPlugin::pstore_set('ad-setting:topic_list_top', 'Banner') - expect(subject[:topic_list_top]).to eq('Banner') - expect(subject.except(:topic_list_top)).to eq( - defaults.except(:topic_list_top) - ) + AdPlugin.pstore_set("ad-setting:topic_list_top", "Banner") + expect(subject[:topic_list_top]).to eq("Banner") + expect(subject.except(:topic_list_top)).to eq(defaults.except(:topic_list_top)) end end - describe '#update' do + describe "#update" do before do AdPlugin::HouseAd.create(name: "Banner", html: "

Banner

") AdPlugin::HouseAd.create(name: "Donate", html: "

Donate

") end it "can set override for the first time" do - expect { - AdPlugin::HouseAdSetting.update(:topic_list_top, 'Banner|Donate') - }.to change { PluginStoreRow.count }.by(1) - expect(AdPlugin::HouseAdSetting.all[:topic_list_top]).to eq('Banner|Donate') + expect { AdPlugin::HouseAdSetting.update(:topic_list_top, "Banner|Donate") }.to change { + PluginStoreRow.count + }.by(1) + expect(AdPlugin::HouseAdSetting.all[:topic_list_top]).to eq("Banner|Donate") end it "can update an existing override" do - AdPlugin::pstore_set('ad-setting:topic_list_top', 'Banner') - expect { - AdPlugin::HouseAdSetting.update(:topic_list_top, 'Banner|Donate') - }.to_not change { PluginStoreRow.count } - expect(AdPlugin::HouseAdSetting.all[:topic_list_top]).to eq('Banner|Donate') + AdPlugin.pstore_set("ad-setting:topic_list_top", "Banner") + expect { AdPlugin::HouseAdSetting.update(:topic_list_top, "Banner|Donate") }.to_not change { + PluginStoreRow.count + } + expect(AdPlugin::HouseAdSetting.all[:topic_list_top]).to eq("Banner|Donate") end it "removes ad names that don't exist" do - AdPlugin::HouseAdSetting.update(:topic_list_top, 'Coupon|Banner|Donate') - expect(AdPlugin::HouseAdSetting.all[:topic_list_top]).to eq('Banner|Donate') + AdPlugin::HouseAdSetting.update(:topic_list_top, "Coupon|Banner|Donate") + expect(AdPlugin::HouseAdSetting.all[:topic_list_top]).to eq("Banner|Donate") end it "can reset to default" do - AdPlugin::pstore_set('ad-setting:topic_list_top', 'Banner') - expect { - AdPlugin::HouseAdSetting.update(:topic_list_top, '') - }.to change { PluginStoreRow.count }.by(-1) - expect(AdPlugin::HouseAdSetting.all[:topic_list_top]).to eq('') + AdPlugin.pstore_set("ad-setting:topic_list_top", "Banner") + expect { AdPlugin::HouseAdSetting.update(:topic_list_top, "") }.to change { + PluginStoreRow.count + }.by(-1) + expect(AdPlugin::HouseAdSetting.all[:topic_list_top]).to eq("") end it "raises error on invalid setting name" do - expect { - AdPlugin::HouseAdSetting.update(:nope, 'Click Me') - }.to raise_error(Discourse::NotFound) - expect(AdPlugin::pstore_get('ad-setting:nope')).to be_nil + expect { AdPlugin::HouseAdSetting.update(:nope, "Click Me") }.to raise_error( + Discourse::NotFound, + ) + expect(AdPlugin.pstore_get("ad-setting:nope")).to be_nil end it "raises error on invalid value" do - expect { - AdPlugin::HouseAdSetting.update(:topic_list_top, '