Skip to content

Open unread comments from workspace#159

Merged
HamptonMakes merged 1 commit into
mainfrom
hampton/open-unread-comment
Jul 23, 2026
Merged

Open unread comments from workspace#159
HamptonMakes merged 1 commit into
mainfrom
hampton/open-unread-comment

Conversation

@HamptonMakes

@HamptonMakes HamptonMakes commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

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

  • Route each attention item through its newest unread notification
  • Mark the notification read and deep-link to the exact thread, including resolved threads
  • Add request coverage for unresolved and resolved unread comments

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 failures

Update Jul 23, 17:12: Prevent hover prefetch from consuming notifications

  • Disable Turbo on the side-effecting attention link, matching the inbox implementation
  • Assert the exact notification href and data-turbo=false in request coverage

Generated with Amp

@HamptonMakes
HamptonMakes marked this pull request as ready for review July 23, 2026 21:04

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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" %>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated by Amp 🤖

@HamptonMakes
HamptonMakes force-pushed the hampton/open-unread-comment branch from ec14442 to 2744eff Compare July 23, 2026 21:42
@HamptonMakes
HamptonMakes merged commit 876c1cf into main Jul 23, 2026
4 checks passed
@HamptonMakes
HamptonMakes deleted the hampton/open-unread-comment branch July 23, 2026 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant