Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ブログ記事公開時に通知先のターゲットを選択できるようにした #8346

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c69cba9
articlesにtargetカラムを作成、モデルにtargetのenumを追加
ayu-0505 Jan 21, 2025
0861872
viewで選択画面を追加、コントローラーでパラメーターが取得できるように変更
ayu-0505 Jan 21, 2025
8b5997d
通知ターゲットにて通知しない以外を選択した場合はダイアログが出るように変更
ayu-0505 Jan 22, 2025
a73b54c
通知しない選択時に通知が飛ばないように受信者の設定を変更、合わせて受信者設定メソッドをより汎用性がある名前に変更
ayu-0505 Jan 22, 2025
ade650e
通知しないに対応する変数名を変更
ayu-0505 Jan 23, 2025
b187cc9
警告文をsubmitボタン押下時に表示されるように変更
ayu-0505 Jan 30, 2025
494f04c
公開するボタンの時のみ警告文が表示され、WIPでは表示されないように変更
ayu-0505 Jan 30, 2025
ee8d0b3
article.jsをリファクタリングした(関数化してDRYにする、早期リターンの条件対象を変更)
ayu-0505 Jan 30, 2025
dce1c3d
一度通知したブログ記事においてはターゲット表示が出ないように修正
ayu-0505 Jan 30, 2025
456add6
通知ターゲットは初回公開時のみ飛ぶことを新規作成フォームにて明記、公開したものは通知済みであることを編集フォームに追記
ayu-0505 Jan 31, 2025
d0f6380
articleのenum型のtargetにi18nの翻訳を追加、編集時の既通知連絡の文言に通知を行った対象を追加
ayu-0505 Jan 31, 2025
9d496b4
文字位置や適応タグの調整
ayu-0505 Feb 1, 2025
6301099
通知が届くようにコードを変更、テストを作成
ayu-0505 Feb 14, 2025
50e5db3
lintにおける指摘事項を修正
ayu-0505 Feb 14, 2025
3b5cb50
articleのシステムテスト修正、テストで発見された不具合の修正
ayu-0505 Feb 19, 2025
a2a9c91
テストにおける使用メソッド名を変更に合わせて修正、ダイアログの影響でFlakyになっていた箇所を修正
ayu-0505 Feb 19, 2025
d401188
ファイル名を実態に沿うように変更
ayu-0505 Feb 19, 2025
b3cbcaa
バリデーションエラー時にpublished_atが入ってしまい、本来の初回投稿時の時間と齟齬が出る問題を修正
ayu-0505 Feb 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions app/controllers/articles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def show
end

def new
@article = Article.new
@article = Article.new(target: 'all')
end

def edit; end
Expand All @@ -38,9 +38,7 @@ def create
@article.user = current_user if @article.user.nil?
set_wip
if @article.save
# Newspaper.publish(:create_article, { article: @article })
# 上のコードのコメントアウトは、以下のissueのための一時的なものなので、mergeされ次第コメントアウトを外すこと。
# https://github.com/fjordllc/bootcamp/issues/8244
Newspaper.publish(:create_article, { article: @article })

redirect_to redirect_url(@article), notice: notice_message(@article)
else
Expand Down Expand Up @@ -95,6 +93,7 @@ def article_params
thumbnail_type
summary
display_thumbnail_in_body
target
]
article_attributes.push(:published_at) unless params[:commit] == 'WIP'
article_attributes.push(:token) if params[:commit] == 'WIP'
Expand Down
61 changes: 61 additions & 0 deletions app/javascript/article-target.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
document.addEventListener('DOMContentLoaded', () => {
const radioButtons = document.querySelectorAll(
'input[name="article[target]"]'
)
const defaultTarget = document.querySelector(
'input[name="article[target]"]:checked'
)
if (!defaultTarget) {
return
}
let target = defaultTarget.value
let targetName = document
.querySelector(`label[for="${defaultTarget.id}"]`)
.textContent.trim()
let text = setAlertText(target, targetName)

radioButtons.forEach((radio) => {
radio.addEventListener('change', (event) => {
target = event.target.value
targetName = document
.querySelector(`label[for="${radio.id}"]`)
.textContent.trim()
text = setAlertText(target, targetName)
})
})

const form = document.getElementById('article_form')
const submitButtons = form.querySelectorAll(
'input[type="submit"',
'button[type="submit"]'
)
const publishButton = document.getElementById('js-shortcut-submit')
let isPublish = false
publishButton.addEventListener('click', () => {
isPublish = true
})

form.addEventListener('submit', (event) => {
if (isPublish) {
submitButtons.forEach((button) => {
button.removeAttribute('data-disable-with')
})
if (!window.confirm(text)) {
event.preventDefault()
isPublish = false
} else {
submitButtons.forEach((button) => {
button.disabled = true
})
}
}
})
})

function setAlertText(target, targetName) {
if (target === 'none') {
return '「通知しない」が選択されています。よろしいですか?'
} else {
return `この記事を公開すると、${targetName}に通知が送られます。もし記事に誤った情報が含まれている場合、その通知を受け取った人を介してSNSなどで拡散される可能性があります。そのため、記事内の年月日などに間違いがないか、しっかり確認してください。公開しても問題ありませんか?`
}
}
1 change: 1 addition & 0 deletions app/javascript/packs/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import '../micro-reports.js'
import '../answer.js'
import '../new-answer.js'
import '../coding-test.js'
import '../article-target.js'

import VueMounter from '../VueMounter.js'
import Questions from '../components/questions.vue'
Expand Down
2 changes: 1 addition & 1 deletion app/models/announcement_notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def call(payload)
DiscordNotifier.with(announce: announcement).announced.notify_now
Watch.create!(user: announcement.user, watchable: announcement)

receivers = User.announcement_receiver(announcement.target).reject { |receiver| receiver == announcement.sender }
receivers = User.notification_receiver(announcement.target).reject { |receiver| receiver == announcement.sender }
PostAnnouncementJob.perform_later(announcement, receivers)
end
end
19 changes: 19 additions & 0 deletions app/models/article.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ class Article < ApplicationRecord
blue: 12
}

enum target: {
all: 0,
students: 1,
job_seekers: 2,
none: 3
}, _prefix: true

belongs_to :user
alias sender user
include ActionView::Helpers::AssetUrlHelper
Expand All @@ -34,6 +41,8 @@ class Article < ApplicationRecord
content_type: %w[image/png image/jpg image/jpeg],
size: { less_than: 10.megabytes }

after_validation :reset_published_at_if_invalid

paginates_per 24
acts_as_taggable

Expand All @@ -57,6 +66,10 @@ def published?
!wip?
end

def before_initial_publish?
published_at.nil?
end

def generate_token!
self.token ||= SecureRandom.urlsafe_base64
end
Expand All @@ -70,4 +83,10 @@ def will_be_published?
def set_published_at
self.published_at = Time.current
end

def reset_published_at_if_invalid
return unless errors.any?

self.published_at = nil
end
end
2 changes: 1 addition & 1 deletion app/models/article_notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def call(payload)
article = payload[:article]
return unless article.saved_change_to_attribute?(:published_at, from: nil)

receivers = User.students_trainees_mentors_and_admins.reject { |receiver| receiver == article.user }
receivers = User.notification_receiver(article.target).reject { |receiver| receiver == article.sender }
send_notification(article:, receivers:)
end

Expand Down
4 changes: 3 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -446,14 +446,16 @@ class User < ApplicationRecord
)

class << self
def announcement_receiver(target)
def notification_receiver(target)
case target
when 'all'
User.unretired
when 'students'
User.admins_and_mentors.or(User.students)
when 'job_seekers'
User.admins_and_mentors.or(User.job_seekers)
when 'none'
User.none
else
User.none
end
Expand Down
37 changes: 36 additions & 1 deletion app/views/articles/_form.html.slim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
= render 'errors', object: @article
= form_with(model: article, local: true) do |f|
= form_with(model: article, local: true, id: 'article_form') do |f|
.form__items
.form-item
.row
Expand Down Expand Up @@ -139,6 +139,41 @@
p
| WIPで保存した場合は、記事公開日時は保存されません。

- if article.before_initial_publish?
.form-item
label.a-form-label
| 通知ターゲット
.a-form-help
p
| 通知は初回公開時のみ行われます。
ul.block-checks.is-3-items
li.block-checks__item
.a-block-check.is-radio
= f.radio_button :target, 'all', id: 'all', class: 'a-toggle-checkbox'
label.a-block-check__label(for='all')
= t('activerecord.enums.article.target.all')
li.block-checks__item
.a-block-check.is-radio
= f.radio_button :target, 'students', id: 'students', class: 'a-toggle-checkbox'
label.a-block-check__label(for='students')
= t('activerecord.enums.article.target.students')
li.block-checks__item
.a-block-check.is-radio
= f.radio_button :target, 'job_seekers', id: 'job_seekers', class: 'a-toggle-checkbox'
label.a-block-check__label(for='job_seekers')
= t('activerecord.enums.article.target.job_seekers')
li.block-checks__item
.a-block-check.is-radio
= f.radio_button :target, 'none', id: 'none', class: 'a-toggle-checkbox'
label.a-block-check__label(for='none')
=t('activerecord.enums.article.target.none')
- elsif article.target == 'none'
p
| この記事は通知を行わずに公開しました。
- else
p
= "この記事は初回公開時に#{t("activerecord.enums.article.target.#{article.target}")}に対してすでに通知を行っています。"

.form-actions
ul.form-actions__items
li.form-actions__item.is-main
Expand Down
5 changes: 5 additions & 0 deletions config/locales/ja.yml
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@ ja:
orange: Orange
brown: Brown
blue: Blue
target:
all: 全員(退会者を除く)
students: 現役生のみ
job_seekers: 就職希望者のみ
none: 通知しない
errors:
models:
campaign:
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20250121062719_add_target_to_articles.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddTargetToArticles < ActiveRecord::Migration[6.1]
def change
add_column :articles, :target, :integer
end
end
1 change: 1 addition & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
t.integer "thumbnail_type", default: 0, null: false
t.string "token"
t.boolean "display_thumbnail_in_body", default: true, null: false
t.integer "target", default: 0
t.index ["user_id"], name: "index_articles_on_user_id"
end

Expand Down
5 changes: 5 additions & 0 deletions test/models/article_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ class ArticleTest < ActiveSupport::TestCase
assert_not articles(:article3).published?
end

test '#before_initial_publish?' do
assert_not articles(:article1).before_initial_publish?
assert articles(:article3).before_initial_publish?
end

test '#generate_token!' do
test_article = Article.create(
title: 'サンプル記事',
Expand Down
24 changes: 18 additions & 6 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,14 @@ class UserTest < ActiveSupport::TestCase
assert user.invalid?
end

test 'announcment for all' do
target = User.announcement_receiver('all')
test 'notification for all' do
target = User.notification_receiver('all')
assert_includes(target, users(:kimura))
assert_not_includes(target, users(:yameo))
end

test 'announcment for students' do
target = User.announcement_receiver('students')
test 'notification for students' do
target = User.notification_receiver('students')
assert_includes(target, users(:kimura))
assert_includes(target, users(:komagata))
assert_includes(target, users(:mentormentaro))
Expand All @@ -230,8 +230,8 @@ class UserTest < ActiveSupport::TestCase
assert_not_includes(target, users(:kensyu))
end

test 'announcment for job_seekers' do
target = User.announcement_receiver('job_seekers')
test 'notification for job_seekers' do
target = User.notification_receiver('job_seekers')
assert_includes(target, users(:jobseeker))
assert_includes(target, users(:komagata))
assert_includes(target, users(:sotugyou))
Expand All @@ -241,6 +241,18 @@ class UserTest < ActiveSupport::TestCase
assert_not_includes(target, users(:yameo))
end

test 'notification for none' do
target = User.notification_receiver('none')
assert_not_includes(target, users(:kimura))
assert_not_includes(target, users(:jobseeker))
assert_not_includes(target, users(:komagata))
assert_not_includes(target, users(:mentormentaro))
assert_not_includes(target, users(:sotugyou))
assert_not_includes(target, users(:advijirou))
assert_not_includes(target, users(:kensyu))
assert_not_includes(target, users(:yameo))
end

test '#follow' do
kimura = users(:kimura)
hatsuno = users(:hatsuno)
Expand Down
6 changes: 4 additions & 2 deletions test/system/article/tags_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ class Article::TagsTest < ApplicationSystemTestCase
end
assert page.has_text?(tags.first)
assert page.has_text?(tags.second)
click_on '公開する'

page.accept_confirm do
click_on '公開する'
end
assert_text '記事を作成しました'
created_article = Article.find_by(title: 'タグ追加のテスト記事')
assert_equal tags, created_article.tag_list.sort
end
Expand Down
Loading