diff --git a/app/controllers/admin/display_ads_controller.rb b/app/controllers/admin/display_ads_controller.rb index 250081a06..3048a7004 100644 --- a/app/controllers/admin/display_ads_controller.rb +++ b/app/controllers/admin/display_ads_controller.rb @@ -58,7 +58,8 @@ module Admin private def display_ad_params - params.permit(:organization_id, :body_markdown, :placement_area, :published, :approved, :name, :display_to) + params.permit(:organization_id, :body_markdown, :placement_area, :published, :approved, :name, :display_to, + :tag_list) end def authorize_admin diff --git a/app/javascript/display-ad/tags.jsx b/app/javascript/display-ad/tags.jsx index fd212f856..f7d15ad8e 100644 --- a/app/javascript/display-ad/tags.jsx +++ b/app/javascript/display-ad/tags.jsx @@ -8,7 +8,7 @@ import { TagAutocompleteSelection } from '@crayons/MultiSelectAutocomplete/TagAu import { MultiSelectAutocomplete } from '@crayons'; /** - * Tags for the article form. Allows users to search and select up to 4 tags. + * Tags for the display ads admin form. Allows users to search and select up to 10 tags. * * @param {Function} onInput Callback to sync selections to article form state * @param {string} defaultValue Comma separated list of any currently selected tags diff --git a/app/javascript/packs/admin/displayAds.jsx b/app/javascript/packs/admin/displayAds.jsx index 8314c8197..2ad29f35c 100644 --- a/app/javascript/packs/admin/displayAds.jsx +++ b/app/javascript/packs/admin/displayAds.jsx @@ -9,15 +9,29 @@ Document.prototype.ready = new Promise((resolve) => { return null; }); -function saveTags() {} +function saveTags(selectionString) { + document.getElementsByClassName('js-tags-textfield')[0].value = + selectionString; +} function loadTagsField() { + let defaultValue = ''; + const hiddenTagsField = + document.getElementsByClassName('js-tags-textfield')[0]; + + if (hiddenTagsField) { + defaultValue = hiddenTagsField.value.replaceAll(' ', ', '); + } + const displayAdsTargetedTags = document.getElementById( 'display-ad-targeted-tags', ); if (displayAdsTargetedTags) { - render(, displayAdsTargetedTags); + render( + , + displayAdsTargetedTags, + ); } } diff --git a/app/models/article.rb b/app/models/article.rb index 0f791081e..7885169fe 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -2,6 +2,7 @@ class Article < ApplicationRecord include CloudinaryHelper include ActionView::Helpers include Reactable + include TagListValidateable include UserSubscriptionSourceable include PgSearch::Model @@ -768,12 +769,7 @@ class Article < ApplicationRecord # check there are not too many tags return errors.add(:tag_list, I18n.t("models.article.too_many_tags")) if tag_list.size > MAX_TAG_LIST_SIZE - # check tags names aren't too long and don't contain non alphabet characters - tag_list.each do |tag| - new_tag = Tag.new(name: tag) - new_tag.validate_name - new_tag.errors.messages[:name].each { |message| errors.add(:tag, "\"#{tag}\" #{message}") } - end + validate_tag_name(tag_list) end def remove_tag_adjustments_from_tag_list diff --git a/app/models/concerns/tag_list_validateable.rb b/app/models/concerns/tag_list_validateable.rb new file mode 100644 index 000000000..433bee608 --- /dev/null +++ b/app/models/concerns/tag_list_validateable.rb @@ -0,0 +1,11 @@ +module TagListValidateable + extend ActiveSupport::Concern + # we check tags names aren't too long and don't contain non alphabet characters + def validate_tag_name(tag_list) + tag_list.each do |tag| + new_tag = Tag.new(name: tag) + new_tag.validate_name + new_tag.errors.messages[:name].each { |message| errors.add(:tag, "\"#{tag}\" #{message}") } + end + end +end diff --git a/app/models/display_ad.rb b/app/models/display_ad.rb index 98146799c..8ce62b302 100644 --- a/app/models/display_ad.rb +++ b/app/models/display_ad.rb @@ -1,4 +1,6 @@ class DisplayAd < ApplicationRecord + include TagListValidateable + acts_as_taggable_on :tags resourcify ALLOWED_PLACEMENT_AREAS = %w[sidebar_left sidebar_left_2 sidebar_right post_comments].freeze @@ -7,6 +9,7 @@ class DisplayAd < ApplicationRecord "Sidebar Right", "Below the comment section"].freeze + MAX_TAG_LIST_SIZE = 10 POST_WIDTH = 775 SIDEBAR_WIDTH = 350 @@ -18,6 +21,7 @@ class DisplayAd < ApplicationRecord validates :placement_area, presence: true, inclusion: { in: ALLOWED_PLACEMENT_AREAS } validates :body_markdown, presence: true + validate :validate_tag before_save :process_markdown after_save :generate_display_ad_name @@ -50,6 +54,13 @@ class DisplayAd < ApplicationRecord ALLOWED_PLACEMENT_AREAS_HUMAN_READABLE[ALLOWED_PLACEMENT_AREAS.find_index(placement_area)] end + def validate_tag + # check there are not too many tags + return errors.add(:tag_list, I18n.t("models.article.too_many_tags")) if tag_list.size > MAX_TAG_LIST_SIZE + + validate_tag_name(tag_list) + end + private def generate_display_ad_name diff --git a/app/models/tag.rb b/app/models/tag.rb index 644482abf..42204bfd6 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -37,6 +37,7 @@ class Tag < ActsAsTaggableOn::Tag belongs_to :badge, optional: true has_many :articles, through: :taggings, source: :taggable, source_type: "Article" + has_many :display_ads, through: :taggings, source: :taggable, source_type: "DisplayAd" mount_uploader :profile_image, ProfileImageUploader mount_uploader :social_image, ProfileImageUploader diff --git a/app/views/admin/display_ads/_form.html.erb b/app/views/admin/display_ads/_form.html.erb index 7487872a6..30bacc580 100644 --- a/app/views/admin/display_ads/_form.html.erb +++ b/app/views/admin/display_ads/_form.html.erb @@ -24,6 +24,11 @@
+ + <% end %>
diff --git a/spec/models/display_ad_spec.rb b/spec/models/display_ad_spec.rb index 7bbff0e66..c2a78982f 100644 --- a/spec/models/display_ad_spec.rb +++ b/spec/models/display_ad_spec.rb @@ -137,4 +137,44 @@ RSpec.describe DisplayAd, type: :model do expect(described_class.search_ads("foo")).to eq([]) end end + + describe ".validate_tag" do + it "rejects more than 10 tags" do + eleven_tags = "one, two, three, four, five, six, seven, eight, nine, ten, eleven" + expect(build(:display_ad, + name: "This is an Ad", + body_markdown: "Ad Body", + placement_area: "post_comments", + tag_list: eleven_tags).valid?).to be(false) + end + + it "rejects tags with length > 30" do + tags = "'testing tag length with more than 30 chars', tag" + expect(build(:display_ad, + name: "This is an Ad", + body_markdown: "Ad Body", + placement_area: "post_comments", + tag_list: tags).valid?).to be(false) + end + + it "rejects tag with non-alphanumerics" do + expect do + build(:display_ad, + name: "This is an Ad", + body_markdown: "Ad Body", + placement_area: "post_comments", + tag_list: "c++").validate! + end.to raise_error(ActiveRecord::RecordInvalid) + end + + it "always downcase tags" do + tags = "UPPERCASE, CAPITALIZE" + display_ad = create(:display_ad, + name: "This is an Ad", + body_markdown: "Ad Body", + placement_area: "post_comments", + tag_list: tags) + expect(display_ad.tag_list).to eq(tags.downcase.split(", ")) + end + end end