From d79d9e8b46b58367b5aed9154d78004fbd5c14b0 Mon Sep 17 00:00:00 2001 From: Pranav Date: Mon, 23 Sep 2024 23:39:03 -0700 Subject: [PATCH] fix: Include uncategorized articles in the all article section to allow edit/delete (#10153) Fixes https://github.com/chatwoot/chatwoot/issues/9935 Fixes https://github.com/chatwoot/chatwoot/issues/8213 The articles were grouped by category, with locale being a derived attribute from the category. If a category was deleted, the article wouldn't appear on the dashboard. However, due to a bug, it would show up in the uncategorized section on the public portal, leaving agents unable to edit or update the article. To address this issue, I've added a locale attribute directly to the article. This attribute is automatically set from the category or the portal's default locale if not supplied. The API parameters now use this attribute to filter articles. As a result, the dashboard will display articles even if they're not associated with a category, improving the overall workflow. **Main updates:** - Add locale attribute to the Article model. Add db migration to back fill the data based on the above logic. - Add a new scope search_by_locale and use it instead of search_by_category_locale. - Update the ERB template to include the locale filter. - Move from `joins` to `left_outer_joins` to include the articles with no categories. --------- Co-authored-by: Sojan --- Gemfile.lock | 2 +- app/models/article.rb | 15 ++++++++++-- app/views/public/api/v1/portals/show.html.erb | 2 +- .../20240923215335_add_locale_to_article.rb | 24 +++++++++++++++++++ db/schema.rb | 3 ++- .../v1/accounts/articles_controller_spec.rb | 14 +++++++++++ spec/factories/articles.rb | 1 + spec/models/article_spec.rb | 19 +++++++++++++++ 8 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20240923215335_add_locale_to_article.rb diff --git a/Gemfile.lock b/Gemfile.lock index 12c121e0a..75fbd2e4f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -819,7 +819,7 @@ GEM rack-proxy (>= 0.6.1) railties (>= 5.2) semantic_range (>= 2.3.0) - webrick (1.8.1) + webrick (1.8.2) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) diff --git a/app/models/article.rb b/app/models/article.rb index 77903aeb7..b96333797 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -5,6 +5,7 @@ # id :bigint not null, primary key # content :text # description :text +# locale :string default("en"), not null # meta :jsonb # position :integer # slug :string not null @@ -55,12 +56,14 @@ class Article < ApplicationRecord # ensuring that the position is always set correctly before_create :add_position_to_article + before_create :add_locale_to_article after_save :category_id_changed_action, if: :saved_change_to_category_id? enum status: { draft: 0, published: 1, archived: 2 } scope :search_by_category_slug, ->(category_slug) { where(categories: { slug: category_slug }) if category_slug.present? } scope :search_by_category_locale, ->(locale) { where(categories: { locale: locale }) if locale.present? } + scope :search_by_locale, ->(locale) { where(locale: locale) if locale.present? } scope :search_by_author, ->(author_id) { where(author_id: author_id) if author_id.present? } scope :search_by_status, ->(status) { where(status: status) if status.present? } scope :order_by_updated_at, -> { reorder(updated_at: :desc) } @@ -83,11 +86,11 @@ class Article < ApplicationRecord ) def self.search(params) - records = joins( + records = left_outer_joins( :category ).search_by_category_slug( params[:category_slug] - ).search_by_category_locale(params[:locale]).search_by_author(params[:author_id]).search_by_status(params[:status]) + ).search_by_locale(params[:locale]).search_by_author(params[:author_id]).search_by_status(params[:status]) records = records.text_search(params[:query]) if params[:query].present? records @@ -140,6 +143,14 @@ class Article < ApplicationRecord update_article_position_in_category end + def add_locale_to_article + self.locale = if category.present? + category.locale + else + portal.default_locale + end + end + def add_position_to_article # on creation if a position is already present, ignore it return if position.present? diff --git a/app/views/public/api/v1/portals/show.html.erb b/app/views/public/api/v1/portals/show.html.erb index 958848108..9e545776d 100644 --- a/app/views/public/api/v1/portals/show.html.erb +++ b/app/views/public/api/v1/portals/show.html.erb @@ -12,7 +12,7 @@ <%# Uncategorized articles %>
- <% if @portal.articles.where(status: :published, category_id: nil).count > 0 %> + <% if @portal.articles.where(status: :published, category_id: nil, locale: @locale).count > 0 %> <%= render "public/api/v1/portals/uncategorized-block", category: "Uncategorized", portal: @portal %> <% end %>
diff --git a/db/migrate/20240923215335_add_locale_to_article.rb b/db/migrate/20240923215335_add_locale_to_article.rb new file mode 100644 index 000000000..28a335c83 --- /dev/null +++ b/db/migrate/20240923215335_add_locale_to_article.rb @@ -0,0 +1,24 @@ +class AddLocaleToArticle < ActiveRecord::Migration[7.0] + def change + add_column :articles, :locale, :string, default: 'en', null: false + + set_locale_from_category + end + + private + + def set_locale_from_category + Article.find_in_batches do |article_batch| + article_batch.each do |article| + locale = if article.category.present? + article.category.locale + else + article.portal.default_locale + end + # rubocop:disable Rails/SkipsModelValidations + article.update_columns(locale: locale) + # rubocop:enable Rails/SkipsModelValidations + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 3ad9562b5..abb14d1e9 100644 --- a/db/schema.rb +++ b/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: 2024_07_26_220747) do +ActiveRecord::Schema[7.0].define(version: 2024_09_23_215335) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "pg_trgm" @@ -147,6 +147,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_07_26_220747) do t.jsonb "meta", default: {} t.string "slug", null: false t.integer "position" + t.string "locale", default: "en", null: false t.index ["associated_article_id"], name: "index_articles_on_associated_article_id" t.index ["author_id"], name: "index_articles_on_author_id" t.index ["slug"], name: "index_articles_on_slug", unique: true diff --git a/spec/controllers/api/v1/accounts/articles_controller_spec.rb b/spec/controllers/api/v1/accounts/articles_controller_spec.rb index 62380a5ee..754629b7b 100644 --- a/spec/controllers/api/v1/accounts/articles_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/articles_controller_spec.rb @@ -194,6 +194,20 @@ RSpec.describe 'Api::V1::Accounts::Articles', type: :request do expect(json_response['payload'].count).to be 2 end + it 'get all articles with uncategorized articles' do + article2 = create(:article, account_id: account.id, portal: portal, category: nil, locale: 'en', author_id: agent.id) + expect(article2.id).not_to be_nil + + get "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles", + headers: agent.create_new_auth_token, + params: {} + expect(response).to have_http_status(:success) + json_response = response.parsed_body + expect(json_response['payload'].count).to be 2 + expect(json_response['payload'][0]['id']).to eq article2.id + expect(json_response['payload'][0]['category']['id']).to be_nil + end + it 'get all articles with searched params' do article2 = create(:article, account_id: account.id, portal: portal, category: category, author_id: agent.id) expect(article2.id).not_to be_nil diff --git a/spec/factories/articles.rb b/spec/factories/articles.rb index e68ed8f4b..e53549090 100644 --- a/spec/factories/articles.rb +++ b/spec/factories/articles.rb @@ -2,6 +2,7 @@ FactoryBot.define do factory :article, class: 'Article' do account_id { 1 } category_id { 1 } + locale { 'en' } author_id { 1 } title { "#{Faker::Movie.title} #{SecureRandom.hex}" } content { 'MyText' } diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index ff2734640..800f8fc9a 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -40,6 +40,25 @@ RSpec.describe Article do end end + describe 'add_locale_to_article' do + let(:portal) { create(:portal, config: { allowed_locales: %w[en es pt], default_locale: 'es' }) } + let(:category) { create(:category, slug: 'category_1', locale: 'pt', portal_id: portal.id) } + + it 'adds locale to article from category' do + article = create(:article, category_id: category.id, content: 'This is the content', description: 'this is the description', + slug: 'this-is-title', title: 'this is title', + portal_id: portal.id, author_id: user.id) + expect(article.locale).to eq(category.locale) + end + + it 'adds locale to article from portal' do + article = create(:article, content: 'This is the content', description: 'this is the description', + slug: 'this-is-title', title: 'this is title', + portal_id: portal.id, author_id: user.id) + expect(article.locale).to eq(portal.default_locale) + end + end + describe 'search' do let!(:portal_2) { create(:portal, account_id: account.id, config: { allowed_locales: %w[en es] }) } let!(:category_2) { create(:category, slug: 'category_2', locale: 'es', portal_id: portal_1.id) }