Open unread comments from workspace#159
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec1444216d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <% @attention_plans.each do |plan| %> | ||
| <li class="attention__item"> | ||
| <%= link_to plan.title, plan_path(plan), class: "attention__link" %> | ||
| <%= link_to plan.title, notification_path(@attention_notification_ids.fetch(plan.id)), class: "attention__link" %> |
There was a problem hiding this comment.
Avoid marking notifications read on hover
When a user hovers a Needs attention title long enough for Turbo's hover prefetch, this link now issues a GET to notification_path(...) before any click; NotificationsController#show has the side effect of calling mark_read!, so simply moving the mouse over the workspace item can clear the unread badge/attention row without the user opening the comment. Disable prefetch/Turbo for this link (as the inbox panel does) or point it at a non-mutating deep link instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated by Amp 🤖
ec14442 to
2744eff
Compare
Why
Workspace attention links can land on an apparently comment-free plan when unread notifications belong to resolved threads, because resolved highlights are hidden by default.
What
Risk Assessment
Low — this only changes the destination of workspace attention links and reuses the existing notification redirect behavior.
References
bundle exec rspec spec/requests/plans_spec.rb spec/requests/notifications_spec.rb— 107 examples, 0 failuresUpdate Jul 23, 17:12: Prevent hover prefetch from consuming notifications
data-turbo=falsein request coverageGenerated with Amp