Skip to content

fix: package activity linking fails with 'not same trip' error#45

Merged
Systemsaholic merged 3 commits intomainfrom
feature/issue-43-package-link-fix
Mar 19, 2026
Merged

fix: package activity linking fails with 'not same trip' error#45
Systemsaholic merged 3 commits intomainfrom
feature/issue-43-package-link-fix

Conversation

@Systemsaholic
Copy link
Copy Markdown
Owner

@Systemsaholic Systemsaholic commented Mar 19, 2026

Summary

  • Root cause: linkChildrenToPackage compared activity.tripId directly, but regular activities have trip_id = null in the DB — only floating packages store it. The migration backfilled existing data, so it worked initially but broke for any activity created after the migration.
  • Fix: Resolve each child's trip through the itinerary chain (itinerary_day_id → itinerary_days → itineraries → trip_id) instead of using the trip_id column directly.

Test plan

  • Bookings tab → select activities → add to package (was failing, should work now)
  • Package detail → link activities (was failing, should work now)
  • Cross-trip linking still correctly rejected

fixes #43

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Strengthened validation when linking activities to booking packages to ensure activities belong to the correct trip and handle edge cases appropriately.
    • Enhanced activity linking with optimistic UI updates, providing immediate visual feedback and faster response times when managing bookings.

The linkChildrenToPackage validation compared activity.tripId directly,
but regular activities have trip_id=null (only floating packages store it).
Now resolves each child's trip through itinerary_day_id → itinerary_days
→ itineraries → trip_id, matching how trips are associated in the schema.

fixes #43

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tailfire-client Ready Ready Preview, Comment Mar 19, 2026 9:19pm
tailfire-ota Ready Ready Preview, Comment Mar 19, 2026 9:19pm

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 19, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR fixes activity-to-package linking by introducing optimistic cache updates with error recovery on the frontend and enhancing backend trip ID resolution logic to infer missing trip identifiers from itinerary day references when direct trip IDs are unavailable.

Changes

Cohort / File(s) Summary
Frontend mutation optimization
apps/admin/src/hooks/use-bookings.ts
Enhanced useLinkActivities with optimistic updates: filters unlinked-activities from cache on mutation start, invalidates related queries immediately for UI responsiveness, and recovers cache state via targeted refetches on error.
Backend trip validation enhancement
apps/api/src/trips/activities.service.ts
Upgraded linkChildrenToPackage trip validation to resolve missing tripId by calling getTripIdFromDayId() when itineraryDayId is present, enabling activities linked through day references to pass the same-trip check.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 Hops of joy, a package dance,
Activities now get their chance,
Optimistic bounds and day-trips known,
No more errors on this home,
The rabbit cheers—bugs all flown! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the bug fix (package activity linking fails with 'not same trip' error) and matches the core change—resolving trip validation via itinerary chain instead of direct tripId comparison.
Linked Issues check ✅ Passed The PR addresses issue #43 by fixing the validation logic in ActivitiesService to infer tripId from itinerary relations and adding optimistic UI updates in the hook, enabling activities on the same trip to be linked regardless of null activity.tripId.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing package activity linking: the service resolves trip via itinerary chain for validation, and the hook adds optimistic updates and proper cache invalidation for the linking operation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/issue-43-package-link-fix
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Activities now disappear from the unlinked list immediately when added
to a package, without waiting for a server refetch. On error, the
cache is invalidated to restore correct state.

fixes #43

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The package expanded row uses useBookingLinkedActivities which has its
own query key. Must invalidate it alongside unlinked list removal so
newly linked activities appear in the package immediately.

fixes #43

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Systemsaholic Systemsaholic merged commit b07e6de into main Mar 19, 2026
3 of 4 checks passed
@Systemsaholic Systemsaholic deleted the feature/issue-43-package-link-fix branch March 19, 2026 21:23
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.

Add activity to package fails

1 participant