Skip to content
Merged
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
aa7b91c
ユーザー毎の未確認日報のレポート数をキャッシュ化
hirokiej Jun 6, 2025
b4fb497
report.rbにユーザーIDでフィルタリングするスコープuserを追加
hirokiej Jun 6, 2025
fa389f3
日報本文下に、そのユーザーの未確認の日報数を表示できるようにした
hirokiej Jun 6, 2025
c43337b
メンターモードONの時に未チェックの日報数を表示するようにした
hirokiej Jun 6, 2025
bc548a0
スタッフログイン時に未チェックの日報件数を表示
hirokiej Jun 9, 2025
2ce53fb
日報個別からユーザーの日報一覧へのリンクのデザイン
machida Jun 9, 2025
6827695
未チェックの日報数に応じたCSSクラス判定用の定数を定義
hirokiej Jun 28, 2025
93dafb5
未チェックの日報の件数に応じてCSSクラスを返すuser_report_count_classメソッドを追加
hirokiej Jun 28, 2025
ebdf285
未チェックの日報数に応じて表示メッセージを変更するunchecked_report_messageを追加
hirokiej Jun 28, 2025
e46f3b9
メンターモードがオンの時、未チェックの日報数によって表示を変更するようSlimを修正
hirokiej Jun 28, 2025
76ec737
user_report_count_classのテストを追加
hirokiej Jun 28, 2025
ad86a19
unchecked_report_messageのテストを追加
hirokiej Jun 28, 2025
7f417f0
ユーザー日報一覧へのリンクで別タブが開くようにした
hirokiej Jun 29, 2025
adea367
特定のユーザーの未チェクの日報数のキャッシュを削除する処理を追加
hirokiej Jul 12, 2025
88b84b1
未チェック日報数表示に関するメソッドと定数をReportsHelperから UserDecoratorへ移動
hirokiej Jul 24, 2025
8fc44fd
RuboCop対応のため、UserDecoratorをモジュールに切り分けた
hirokiej Jul 24, 2025
1ac7588
UserDecorator/report_status.rbに処理を移行したため、テストも同様に移行
hirokiej Jul 27, 2025
6f94e8b
日報数によるクラス判定の定数をデータにまとめ、メソッドも修正
hirokiej Jul 29, 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
1 change: 1 addition & 0 deletions app/decorators/user_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module UserDecorator

include Role
include Retire
include ReportStatus

def twitter_url
"https://twitter.com/#{twitter_account}"
Expand Down
27 changes: 27 additions & 0 deletions app/decorators/user_decorator/report_status.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# frozen_string_literal: true

module UserDecorator
module ReportStatus
REPORT_COUNT_LEVELS = {
'is-success' => 0..1,
'is-primary' => 2..4,
'is-warning' => 5..9
}.freeze

LAST_UNCHECKED_REPORT_COUNT = 1

def user_report_count_class(count)
REPORT_COUNT_LEVELS.find { |_, range| range.include?(count) }&.first || 'is-danger'
end

def unchecked_report_message(count, user)
if count.zero?
"#{user.login_name}さんの日報へ"
elsif count == LAST_UNCHECKED_REPORT_COUNT
"#{user.login_name}さんの未チェックの日報はこれで最後です。"
else
"#{user.login_name}さんの未チェックの日報が#{count}件あります。"
end
end
end
end
1 change: 1 addition & 0 deletions app/javascript/stylesheets/application.sass
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
@import application/blocks/auth-form/skip-practices

@import application/blocks/cards/card-counts
@import application/blocks/cards/card-body-main-actions

@import application/blocks/company/company-links
@import application/blocks/company/company-profile
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
.card-body-main-actions
+position(relative, 1)
padding-block: .75rem
margin-inline: 1rem
border-radius: 4px
+media-breakpoint-up(md)
padding-inline: 2rem
margin-bottom: 1.5rem
+media-breakpoint-down(sm)
padding-inline: .75rem
margin-bottom: 1rem
&.is-success
background-color: #f8fff2
border: 1px solid var(--success)
color: var(--success-text)
&.is-primary
border: 1px solid var(--primary)
background-color: #f5f5ff
color: var(--primary-text)
&.is-warning
border: 1px solid var(--warning)
background-color: #fff9e7
color: var(--warning-text)
&.is-danger
border: 1px solid var(--danger)
background-color: var(--danger-tint)
color: var(--danger-text)
Comment on lines +12 to +27
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

アクセシビリティの改善を検討してください

色のみでステータスを示すスタイリングは、色覚に障害のあるユーザーにとってアクセシブルではありません。アイコンやボーダースタイルなどの追加の視覚的手がかりを検討してください。

以下のような改善案を検討してください:

  &.is-success
    background-color: #f8fff2
    border: 1px solid var(--success)
    color: var(--success-text)
+   &::before
+     content: "✓"
+     margin-right: 0.5rem

同様に他の状態についても適切なアイコンやインジケーターを追加することをお勧めします。

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
&.is-success
background-color: #f8fff2
border: 1px solid var(--success)
color: var(--success-text)
&.is-primary
border: 1px solid var(--primary)
background-color: #f5f5ff
color: var(--primary-text)
&.is-warning
border: 1px solid var(--warning)
background-color: #fff9e7
color: var(--warning-text)
&.is-danger
border: 1px solid var(--danger)
background-color: var(--danger-tint)
color: var(--danger-text)
&.is-success
background-color: #f8fff2
border: 1px solid var(--success)
color: var(--success-text)
&::before
content: ""
margin-right: 0.5rem
&.is-primary
border: 1px solid var(--primary)
background-color: #f5f5ff
color: var(--primary-text)
&.is-warning
border: 1px solid var(--warning)
background-color: #fff9e7
color: var(--warning-text)
&.is-danger
border: 1px solid var(--danger)
background-color: var(--danger-tint)
color: var(--danger-text)
🤖 Prompt for AI Agents
In
app/javascript/stylesheets/application/blocks/cards/_card-body-main-actions.sass
lines 12 to 27, the current styling uses only color changes to indicate status,
which is not accessible for users with color vision deficiencies. To fix this,
add additional visual indicators such as icons or distinct border styles for
each status class (is-success, is-primary, is-warning, is-danger). Ensure these
indicators are clearly visible and consistent across all statuses to improve
accessibility.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hirokiej こちらの指摘に対応してください。(問題なければそのようにコメントして議論が決着するところまでやってください)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown
Contributor Author

@hirokiej hirokiej Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@komagata さん
承知しました!ありがとうございます🙇

@machida さん
こちらのcoderabbitaiの件ですが、提出物の確認部分でも日数だけでなく、色での判断もしています。
image
今回も件数を表示しているので修正は不要だと感じております。
デザインに関わる部分なので、町田さんの意見をいただけたら幸いです🙇

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hirokiej 色での判断は不要ですー。理由は、提出物は期限があり、その期限にどれくらい迫っているかをすぐに確認できるように色が用意されているが、日報にはそれがないためです。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@machida さん
ご返信ありがとうございます!
提出物のUIの意図も理解できました🙏
それでは、こちら問題なしということで対応いたします🙇

@coderabbitai
こちら、色での判断はしていないので問題ありません。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hirokiej さん、ご説明ありがとうございます!

machidaさんからの説明もとても参考になりました。提出物と日報でのUI設計の意図の違いがよく理解できました。確かに、こちらの実装では件数が数値として明確に表示されており、色はあくまで視覚的な補助として機能しているため、アクセシビリティの問題はありませんね。

ご対応いただき、ありがとうございました!

(´・ω・`)


✏️ Learnings added
Learnt from: hirokiej
PR: fjordllc/bootcamp#8740
File: app/javascript/stylesheets/application/blocks/cards/_card-body-main-actions.sass:12-27
Timestamp: 2025-07-29T07:46:11.138Z
Learning: In this codebase, color coding serves different purposes for different features: submissions (提出物) use color indicators to show deadline urgency which is important for quick assessment, while daily reports (日報) don't require such urgency indicators. When implementing status indicators, the primary information should be conveyed through text/numbers rather than color alone for accessibility.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


.card-body-main-actions__description
+text-block(.875rem 1.4)
margin-bottom: .75em
text-align: center

.card-body-main-actions__items
display: flex
justify-content: center

.card-body-main-actions__item
text-align: center
flex: 1
+media-breakpoint-up(md)
max-width: 16rem

.card-body-main-actions__item-cancel
+hover-link-reversal
color: var(--muted-text)
font-size: .8125rem
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
color: #4e732e
&.is-unparticipationed.is-available
border: 1px solid var(--primary)
background-color: #e9f9ff
color: #116e94
background-color: #f5f5ff
color: #28248c
&.is-unparticipationed.is-capacity-over
border: 1px solid var(--warning)
background-color: #fff9e7
Expand All @@ -27,7 +27,7 @@
color: var(--danger)

.event-main-actions__description
+text-block(.8125rem 1.4)
+text-block(.875rem 1.4)
margin-bottom: .75em
text-align: center

Expand Down
10 changes: 10 additions & 0 deletions app/models/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ def delete_unchecked_report_count
Rails.cache.delete 'unchecked_report_count'
end

def user_unchecked_report_count(user)
Rails.cache.fetch "#{user.id}-user_unchecked_report_count" do
Report.unchecked.not_wip.user(user).count
end
end

def delete_user_unchecked_report_count(user_id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

キャッシュを削除するメソッドがありますが、どこにも使われていないみたいですー

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘ありがとうございます!
目視で数値が減っていたので、追加しなかったのですが問題ありありでした💦
修正します。

Rails.cache.delete "#{user_id}-user_unchecked_report_count"
end

def unchecked_product_count
Rails.cache.fetch 'unchecked_product_count' do
Product.unhibernated_user_products.unchecked.not_wip.count
Expand Down
2 changes: 2 additions & 0 deletions app/models/check_callbacks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ def after_destroy(check)
def delete_report_cache(check)
return unless check.checkable_type == 'Report'

report = check.checkable
Cache.delete_unchecked_report_count
Cache.delete_user_unchecked_report_count(report.user_id)
end

def delete_product_cache(check)
Expand Down
2 changes: 2 additions & 0 deletions app/models/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ class Report < ApplicationRecord # rubocop:todo Metrics/ClassLength
.default_order
}

scope :user, ->(user) { where(user_id: user.id) }

class << self
def faces
@faces ||= emotions.keys
Expand Down
1 change: 1 addition & 0 deletions app/models/report_callbacks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ def after_create(report)

def after_destroy(report)
Cache.delete_unchecked_report_count
Cache.delete_user_unchecked_report_count(report.user_id)
delete_notification(report)
end

Expand Down
1 change: 1 addition & 0 deletions app/models/report_notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class ReportNotifier
def call(_name, _started, _finished, _unique_id, payload)
report = payload[:report]
Cache.delete_unchecked_report_count
Cache.delete_user_unchecked_report_count(report.user_id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この日報の通知機能は今回実装する機能とは特に関係がないように思ったのですが、どうでしょうか?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘ありがとうございます🙇

_report_body.html.slimがレンダリングされるたびにキャッシュを参照し、存在しない場合は再計算されます。
しかし、日報の更新があった際にキャッシュを削除しないと、古い値が表示される可能性があります。
そのため、Cache.delete_unchecked_report_countと同様に、キャッシュ削除の処理を記述しています。

#fetchは、キャッシュが存在しない場合はデフォルト値を設定するためにブロックで値を取得する
Rails のキャッシュ機構 - Railsガイド

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なるほど、モデル名に引っ張られて勘違いしていたみたいです。
日報をwipから公開に更新した時に未確認日報数が変わるのでキャッシュを削除する必要があるということですね


return unless report.first_public?

Expand Down
17 changes: 17 additions & 0 deletions app/views/reports/_report_body.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,23 @@
.card-body__description
.a-long-text.is-md.js-markdown-view(data-taskable-id="#{report.id}" data-taskable-type='Report' data-taskable="#{report.taskable?(current_user).to_s}")
= report.description
- if staff_login?
- count = Cache.user_unchecked_report_count(report.user)
.card-body-main-actions(class="#{report.user.user_report_count_class(count)} #{mentor_login? ? 'is-only-mentor' : ''}")
.card-body-main-actions__description
p
= report.user.unchecked_report_message(count, report.user)
ul.card-body-main-actions__items
li.card-body-main-actions__item
= link_to user_reports_path(report.user), class: "card-body-main-actions__action a-button is-sm #{report.user.user_report_count_class(count)} is-block", target: '_blank', rel: 'noopener' do
| #{report.user.login_name}さんの日報一覧へ
- else
.card-body-main-actions.p-0
ul.card-body-main-actions__items
li.card-body-main-actions__item
= link_to user_reports_path(report.user), class: 'card-body-main-actions__action a-button is-sm is-secondary is-block', target: '_blank', rel: 'noopener' do
| #{report.user.login_name}さんの日報一覧へ

hr.a-border-tint
= render 'reactions/reactions', reactionable: report

Expand Down
29 changes: 29 additions & 0 deletions test/decorators/user_decorator/report_status.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# frozen_string_literal: true

require 'test_helper'
require 'active_decorator_test_case'

module UserDecorator
class ReportStatusTest < ActiveDecoratorTestCase
test 'user_report_count_class' do
assert_equal 'is-success', user_report_count_class(0)
assert_equal 'is-success', user_report_count_class(1)

assert_equal 'is-primary', user_report_count_class(2)
assert_equal 'is-primary', user_report_count_class(4)

assert_equal 'is-warning', user_report_count_class(5)
assert_equal 'is-warning', user_report_count_class(9)

assert_equal 'is-danger', user_report_count_class(10)
assert_equal 'is-danger', user_report_count_class(100)
end

test 'unchecked_report_message' do
user = users(:hajime)
assert_equal "#{user.login_name}さんの日報へ", unchecked_report_message(0, user)
assert_equal "#{user.login_name}さんの未チェックの日報はこれで最後です。", unchecked_report_message(1, user)
assert_equal "#{user.login_name}さんの未チェックの日報が2件あります。", unchecked_report_message(2, user)
end
end
end