Skip to content

fix(huddle): posts made while clocked in join the live session; Session shows shifts only - #854

Merged
Dharp02 merged 10 commits into
feat/848-huddle-all-groupingfrom
fix/853-session-grouping-live-session
Nov 11, 2026
Merged

Dharp02 merged 10 commits into
feat/848-huddle-all-groupingfrom
fix/853-session-grouping-live-session

Conversation

@Dharp02

@Dharp02 Dharp02 commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #853

Stacked on #850 (the All grouping). Retarget to main once #850 merges.

Overview

73C7969A-4B8C-4368-BC45-60F829DE475D.mov

A post from Huddle's message box never carried the author's clock session. So under Group by → Session it got its own "Off the clock" thread, even in the middle of a shift. Now a post made while you're clocked in joins that shift's thread, and Session lists shifts only. Posts made off the clock stay in All, Day and Person.

Changes

  • meteor-backend/server/huddle.js, clock.js
    • The huddle.createPost method links a post to the author's open session in the posting team when the post names none, and stamps it sessionUpdate: true.
      • Where this applies: everything that calls that method, which is Huddle's message box on web and mobile, plus REST and MCP.
      • Where it doesn't: server-side callers of the shared createHuddlePost, which are Pulse video posts, Pulse plans and the wrap-up fallback. They never link on their own.
    • An update is never the session's plan. SESSION_POST_FILTER keeps updates out of every lookup for "the session's post": the clock-out gate (clockStop), Pulse wrap-ups (appendWrapUp) and the Clock page's wrap-up editor (getMyPostForSession). So a mid-shift "grabbing lunch" in a session with no plan never gets the wrap-up written onto it.
    • Clock-in clears the stamp on the plan post it links. Existing posts have no stamp, so they behave as before.
    • Scheduled posts stay unlinked, because they appear later, when the shift may be over. An explicit clockEventId is validated as before.
    • It returns { id, clockEventId? }.
  • superChatFeed.ts
    • Session grouping skips posts with no session.
    • The feed's plan pick (the "Plan" label) skips updates, matching the backend.
    • Removed: the nosession: key, the "… · Off the clock" title and the "Posted without clocking in" line.
    • Search still finds off-the-clock posts by "off the clock" in the other groupings.
  • Huddle.tsx
    • The Session description reads "Each clock-in to clock-out with its plan, updates and wrap-up".
    • A post link to an off-the-clock post opens its Day thread instead of failing in Session.
    • When you post from Session, Huddle opens the thread the post landed in: the live shift, or, when you're clocked out, that day's Day thread. Otherwise the post would vanish from view. The thread opens after the feed refresh, so a shift's first post is already there.
  • Release note: a new section, "Updates stay in your shift", in release-notes/1.2.0.md (not tagged yet). The All section's post-link bullet now mentions Day.

Known limitation

Testing

  • npm run lint, npm run typecheck, npm run format: clean.
  • Unit tests: 911/911 pass.
    • The off-the-clock test now checks that Session leaves those posts out and All, Day and Person keep them.
    • Tests that used session-less posts in Session only by chance now use Day, or give the post a session.
  • Backend (huddle-post-rest.test.ts): 3 new tests pass. They cover a post made while clocked in (linked, and the id is returned), while clocked out (not linked), and while clocked in to another team (not linked).
    • stores each supported format… fails with or without this change: deleting a post leaves its inline images in place.
  • E2E:
    • Huddle: 79/79 before the spec edits, then inbox-filters.spec.ts 12/12 after them.
    • New tests: Session leaves an off-the-clock post out while All and Day show it; a post sent while clocked in lands in the live session thread; a link to an off-the-clock post opens its Day thread.
    • The Add an All Option to Huddle's Group By for One Ungrouped Thread #850 All post-link test now gives its post a session. The Group by description and search tests are updated.
    • Realtime, timers, timesheet, tickets and notifications: 166 passed. deep-links.spec.ts was updated (an off-the-clock post link now resolves to day:) and passes 6/6.
    • tickets/video-upload.spec.ts:41 still fails, as noted in Add an All Option to Huddle's Group By for One Ungrouped Thread #850.
  • Browser smoke test on the dev stack: I clocked in without a plan and posted from Huddle in Session view. The post appeared in the live session thread ("15:44–now · Live"), and no "Off the clock" thread appeared.

🤖 Generated with Claude Code

…on shows shifts only

A post from Huddle's message box never carried the author's clock session,
so the Session grouping put it in an "Off the clock" thread of its own,
even mid-shift.

- createHuddlePost links a post to the author's open session in the team
  when it names none (not for scheduled posts) and returns that id.
- The Session grouping leaves out posts with no session; they stay in All,
  Day and Person. The off-the-clock title and system line are removed.
- A post link to an off-the-clock post opens its Day thread; posting from
  Session while clocked out opens the Day the post went into.

Closes #853
Copilot AI balanced review requested due to automatic review settings November 6, 2026 20:45
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

🗑️ Preview Environment Cleaned Up

The preview container for this PR has been deleted.

Copilot AI left a comment

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.

🟡 Changes recommended

An empty Session grouping can block a clocked-in user’s first update, and deferred navigation can open an old conversation in a newly selected scope.

2 open issues
What changed in this PR

Updates Huddle so posts made during a live shift join that session automatically, while Session grouping shows only session-linked posts. Off-the-clock posts remain accessible in other groupings.

Changes:

  • Links unscheduled posts to the author’s open session in the posting team and returns the session ID.
  • Updates Session filtering, post-link routing, and post-send navigation.
  • Adds regression coverage and release notes for the new behavior.
File Description
tests/​e2e/​notifications/​deep-links.spec.ts Expects off-the-clock notification links to open Day threads.
tests/​e2e/​huddle/​inbox-filters.spec.ts Covers Session filtering, live-session posting, and post links.
src/​pages/​Huddle.tsx Updates Session description and destination-thread navigation.
src/​lib/​api.ts Adds the optional session ID to the create-post response type.
src/​features/​huddle/​superChatFeed.ts Excludes unlinked posts from Session and adds conversation-ID helpers.
src/​features/​huddle/​superChatFeed.test.ts Updates grouping and search tests for session-only threads.
release-notes/​1.2.0.md Documents shift-linked updates and off-the-clock navigation.
meteor-backend/​tests/​huddle-post-rest.test.ts Tests automatic linking, clocked-out posting, and team isolation.
meteor-backend/​server/​huddle.js Automatically links eligible posts to the author’s live session.

Review effort: Balanced

Comment thread src/features/huddle/superChatFeed.ts Resolved
Comment thread src/pages/Huddle.tsx Resolved
- Session with posts but no shifts shows the starter conversation, so its
  message box is still there; its hint says there are no shifts yet.
- Opening where a post landed is skipped if the reader switched team or
  view while it was sending.
Copilot AI balanced review requested due to automatic review settings November 6, 2026 21:01

Copilot AI left a comment

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.

🔵 Needs a closer look

The deferred Session landing can open the wrong Day thread across midnight and override grouping changes made while a post is sending or refreshing.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Medium severity Reuse the post date when opening its landing thread

src/​pages/​Huddle.tsx:1128

For an off-the-clock send from Session that crosses local midnight, postDate was already stamped before createPost, but this recomputes the date after the response. The post belongs to yesterday's Day thread while sessionLanding opens today's empty thread, so the newly sent update is not visible. Capture the date used in the create request and reuse it for the landing instead of reading the clock again.

Medium severity Prevent deferred landing from overriding grouping changes

src/​pages/​Huddle.tsx:1150

Changing Group by while a Session post is sending or its feed is refreshing leaves scopeKey unchanged, so this deferred landing can override the newly selected grouping. The landing's conversation ID switches the view back to Session, or to Day for an unlinked post. Track grouping/navigation changes during the send and apply the landing only if that intent is unchanged, in addition to the existing scope guard.

2 resolved since last review

Review effort: Balanced

…mid-send

- The Day thread a post opens in uses the date stamped on the post, not a
  second read of the clock, so a send across midnight opens the right day.
- Opening where a post landed is skipped if Group by changed while it sent.
@Dharp02

Dharp02 commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the two "previously missed" items from the Copilot overview in 42e1695:

  • Reuse the post date when opening its landing thread: postDate is now stamped once, before the send, and the landing's Day thread uses that same value. A send that crosses midnight opens the day the post actually went into.
  • Prevent deferred landing from overriding grouping changes: the landing is skipped if Group by changed during the send. The current grouping is tracked in threadByRef, alongside the existing scopeKeyRef check. There's a new e2e test: send from an older shift's thread while clocked in, hold the request, switch to Person, release it, and check Group by stays on Person. I confirmed it fails without the guard (it ended on Session).

There's no automated test for the midnight case; it would need the browser clock moved across midnight mid-send.

Copilot AI balanced review requested due to automatic review settings November 6, 2026 21:12

Copilot AI left a comment

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.

🔵 Needs a closer look

A pending Session send can update the destination page’s URL and add an unwanted history entry after the reader leaves Huddle because the guards do not verify the current pathname.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Prevent pending sends from navigating after leaving Huddle

src/​pages/​Huddle.tsx:1157

If the reader leaves Huddle while a Session post is sending or refreshing, the pending send can still pass these guards and add its conversation parameter to the destination page, pushing an unwanted history entry. Huddle stays mounted when hidden, and setParams writes to the live pathname. Check that window.location.pathname is still /app/huddle before opening the landing; the send's captured onScreen value would be stale.

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 6, 2026 21:20

Copilot AI left a comment

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.

🟡 Changes recommended

Deferred navigation can alter a newly opened page after leaving Huddle or open a conversation absent from the loaded feed when refreshes fail, replacing the composer with a conversation-not-found state.

2 open issues

Review effort: Balanced

Comment thread src/pages/Huddle.tsx Resolved
Comment thread src/pages/Huddle.tsx Resolved
- Skipped when the reader has left Huddle: it stays mounted under
  <Activity>, whose paused render can't tell, so the live URL decides.
- Skipped until the post is in the loaded feed (team) or the refreshed
  Personal list, so a failed refresh doesn't open a not-found thread.
Copilot AI balanced review requested due to automatic review settings November 6, 2026 21:45

Copilot AI left a comment

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.

🔵 Needs a closer look

Human assessment is needed because a pending send can replace a newly selected Session conversation and add an unexpected history entry despite the existing navigation checks.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@SarkarShubhdeep SarkarShubhdeep left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A few things before this goes in:

1. Auto-linking breaks the one-post-per-session assumption. createHuddlePost now links every post made while clocked in to the open session. Three places still treat the session's earliest post (SESSION_POST_SORT) as the session post:

  • clockStop: the plan-required clock-out check reads only that post's wrapUpAt
  • appendWrapUp (Pulse wrap-up)
  • huddle.getMyPostForSession (the Clock page's wrap-up editor)

In a team that doesn't require a plan, if someone clocks in with no plan and posts "grabbing lunch", that post gets the Plan label, the wrap-up editor starts with its text, and the wrap-up is saved onto it. That changes saved data, not just the view. timer-updates.js even says updates are kept off clockEventId so they're never taken for a plan. Could we either link only when the session already has a plan post, or have those three places look for the plan post specifically instead of the earliest one? If the current behaviour is intended, it probably needs a sign-off as a behaviour change.

2. It reaches beyond the web composer. Because the link is set in createHuddlePost, it also applies to REST, MCP, mobile and Pulse postVideo. In the Pulse clock-plan path, delivered() counts a post with any clockEventId as delivered, and a post can now have one before clockInWithPlan runs. That only happens when the person is already clocked in, and then the clock-in step links the same session anyway, so it looks harmless. Worth a line in the description, though.

3. CI hasn't fully run. This PR targets #850's branch, so Frontend Checks and Backend Unit Tests haven't run (only e2e). Please retarget to main once #850 merges.

Nits

  • Huddle.tsx: the comment // The grouping on screen now, for a send that finishes after it changed. is duplicated above threadByRef.
  • HUDDLE_PATH sits between the "All comes first and sits apart" comment and THREAD_BY_OPTIONS, so that comment now describes the wrong line.
  • starterConversation still has the old "any team" Personal hint, which conflicts with #852.

…ts plan

Review on #854: auto-linking made a mid-shift post the session's earliest
post, so the clock-out gate, the Pulse wrap-up and the Clock page's wrap-up
editor took it for the plan and wrote the wrap-up onto it.

- Only huddle.createPost (Huddle's message box, REST, MCP) joins the open
  session, and stamps the post sessionUpdate: true. Pulse posts and the
  shared createHuddlePost no longer link on their own.
- SESSION_POST_FILTER keeps updates out of every session-post lookup
  (clockStop, appendWrapUp, getMyPostForSession) and the feed's plan pick.
- Clock-in clears the stamp on the plan post it links.
- Review nits: a duplicated comment, and HUDDLE_PATH above its own comment.
Copilot AI balanced review requested due to automatic review settings November 6, 2026 22:45
@Dharp02

Dharp02 commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

@SarkarShubhdeep thanks, all fair. Addressed in 40717fc:

1. One post per session. You're right, this was a regression, not a trade-off. I went with your second option: the lookups now look for the plan specifically.

  • A post that joins the live session from Huddle is stamped sessionUpdate: true.
  • A shared SESSION_POST_FILTER keeps updates out of all three lookups (clockStop, appendWrapUp, getMyPostForSession) and out of the feed's plan pick.
  • Clock-in clears the stamp on the plan post it links.
  • In a session with no plan, "grabbing lunch" now shows in the shift but never gets the Plan label, never seeds the wrap-up editor, and never gets the wrap-up saved onto it. Clock-out starts a fresh wrap-up post, as it did before this PR.
  • New backend test: with only an update, getMyPostForSession returns null, and once a plan is linked it returns the plan.

I didn't go with "link only when the session already has a plan": in a session with no plan, mid-shift posts would drop out of Session view, which is the confusion #853 is about.

2. Reach. The link now happens only in the huddle.createPost method (Huddle's message box, REST, MCP), not in the shared createHuddlePost. Pulse video and plan posts no longer link on their own, so the clock-plan delivered() case can't arise. The description says so.

3. CI. I'll retarget to main once #850 merges.

Nits: fixed the duplicated comment and moved HUDDLE_PATH above the "All comes first" comment. For the #852 hint, whichever PR merges second will take #852's wording.

Separately, the Plan/Wrap-up labels and the "⚠ no wrap-up" title warning are going away in their own PR. We've decided they add more confusion than they're worth.

Copilot AI left a comment

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.

🟡 Changes recommended

The live DDP path still selects session updates and retains stale sessionUpdate flags after promotion, which can block clock-out and exclude promoted plans from selection.

2 open issues

Review effort: Balanced

Comment thread meteor-backend/server/huddle.js Resolved
Comment thread meteor-backend/server/huddle.js Resolved
useSessionPost now skips posts flagged sessionUpdate, as the server's
SESSION_POST_FILTER does, so a shift with no plan takes the no-plan path
instead of saving its wrap-up onto an update clockStop ignores.

enrichPost always sends sessionUpdate as a boolean. The DDP client merges
changed fields, so leaving it out kept a promoted plan's stale true.
Copilot AI balanced review requested due to automatic review settings November 10, 2026 16:46

Copilot AI left a comment

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.

🔵 Needs a closer look

Human assessment is needed to evaluate a navigation race that can override a newly selected conversation when an earlier send or refresh completes.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@SarkarShubhdeep SarkarShubhdeep left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, all my earlier points are addressed. One small thing left:

Dashboard label: DashboardPage.tsx:833 shows "Posted their plan & clocked in" for any post with a clockEventId. Mid-shift updates now get one, so they'll be labelled as plans. post.clockEventId && !post.sessionUpdate fixes it.

Also, please confirm Frontend Checks and Backend Unit Tests pass after retargeting to main.

Recent activity called any post with a clockEventId "Posted their plan &
clocked in". Updates posted while clocked in now join the session too, so
the label also checks sessionUpdate.
Copilot AI balanced review requested due to automatic review settings November 10, 2026 17:56
@Dharp02

Dharp02 commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @SarkarShubhdeep. Fixed the dashboard label in 5ddd85c: recent activity now shows "Posted their plan & clocked in" only when the post is linked to a shift and isn't a mid-shift update (post.clockEventId && !post.sessionUpdate). The server now always sends sessionUpdate as true or false (f9feb31), so the check holds for every post.

I'll retarget this to main once #850 merges and confirm Frontend Checks and Backend Unit Tests pass there.

@SarkarShubhdeep

Copy link
Copy Markdown
Collaborator

Thanks @SarkarShubhdeep. Fixed the dashboard label in 5ddd85c: recent activity now shows "Posted their plan & clocked in" only when the post is linked to a shift and isn't a mid-shift update (post.clockEventId && !post.sessionUpdate). The server now always sends sessionUpdate as true or false (f9feb31), so the check holds for every post.

I'll retarget this to main once #850 merges and confirm Frontend Checks and Backend Unit Tests pass there.

Awesome thanks, approving it now.

Copilot AI left a comment

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.

🟢 Approval recommended

No active review issues were identified for the session-linked Huddle post changes.

No open issues 🎉

Review effort: Balanced

@Dharp02
Dharp02 merged commit 68280db into feat/848-huddle-all-grouping Nov 11, 2026
12 checks passed
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.

3 participants