Commit Graph

1772 Commits

Author SHA1 Message Date
Stanko Krtalić 6e9de6a017 Revert "Push event payload body updates" 2026-02-27 19:59:18 +01:00
Jirka Hutárek 97d188fae4 Rename new_board_name to new_location_name for clarity in event payload handling 2026-02-27 18:54:15 +01:00
Jirka Hutárek 44235da9c1 Add fallback message for card triage when column name is missing 2026-02-27 18:47:41 +01:00
Jirka Hutárek 7702e49afa Handle collection change the same as board change 2026-02-27 18:42:49 +01:00
Jirka Hutárek 566dd8ded2 Update default message 2026-02-27 18:30:12 +01:00
Jirka Hutárek 856d17c510 Handle missing fields 2026-02-27 18:26:50 +01:00
Jirka Hutárek 9678bdbd24 Add more specific messages to event push payload body 2026-02-27 18:17:56 +01:00
Jirka Hutárek b93752dbb5 Mention column name and creator in push event body when card is moved between columns 2026-02-27 18:10:37 +01:00
Jay Ohms 74eac289ee Improve the structure so users_with_active_accounts is available on identity 2026-02-27 09:17:12 -05:00
Rosa Gutierrez 13268ef668 Add base_url to native push notification payload
Include the server's base URL so native apps can identify which Fizzy
instance (SaaS or self-hosted) a notification originated from.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 92cb749e16 Fix tests for renamed fixtures and new stacked notifications
Clear assignee's existing notifications in setup since Notifier now
uses create_or_find_by instead of create, and reload the association
to avoid caching after destroy_all.

Update fixture references after rebase: use `logo_assignment_kevin`
as base notification and `logo_mentioned_david` for mention tests.
Update source to `logo_published` in tests that need generic card events.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Fernando Olivares b1cdb72381 Add creator initials and avatar color to push notification payload
Move avatar_background_color logic from helper to User::Avatar concern
so it can be accessed from models. Include creator_id, creator_initials,
and creator_avatar_color in native push notifications for local avatar
rendering on iOS.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Fernando Olivares 113e61d418 Add avatar_url so we always get an avatar even when the user hasn't set one 2026-02-25 19:31:13 +01:00
Rosa Gutierrez 7864748be9 Simplify PushTarget by removing template method pattern
Each target now implements process directly with its own logic,
rather than using processable?/perform_push hooks. The pushable?
check is done once in Notification#push before iterating targets.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 0948533da7 Rename push to process on PushTarget for clearer semantics
"Push to target" reads naturally - we push the notification to the
target. "Target processes" also makes sense - the target receives
and handles the notification in its own way.

- Add class method PushTarget.process(notification) that instantiates
  and calls the instance method
- Rename instance method from push to process
- Add private push_to helper in Pushable for readable iteration

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 3621df8f0f Consolidate push jobs into single Notification::PushJob
Replace separate WebPushJob and NativePushJob with a single PushJob
that calls notification.push, which iterates over registered targets.
Each target handles its own delivery - Web pushes synchronously via
the pool, Native enqueues device-level jobs via deliver_later_to.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 4f19c42958 Move category and high_priority to payload classes
Use polymorphism instead of case statements in Native push target:
- DefaultPayload#category returns "default", #high_priority? returns false
- EventPayload#category returns "assignment"/"comment"/"card" based on action
- MentionPayload#category returns "mention", #high_priority? returns true

This simplifies the Native push target by delegating source-specific
logic to the appropriate payload classes.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 9299300dbf Move push priority concerns from Event and Mention into Native push target 2026-02-25 19:31:13 +01:00
Fernando Olivares c7d0559191 Change priority notification level for mentions and assignments 2026-02-25 19:31:13 +01:00
Rosa Gutierrez b5205ce6b2 Rename Push to PushTarget for better readability
PushTarget#push reads more naturally than Push#push. The push target
is the thing that pushes notifications to a specific destination.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 06c9ece058 Move payload method to Notification and make accessors public
The notification now owns its payload via #payload method in Pushable,
allowing direct access like notification.payload.title. Push classes
simply use the notification's payload rather than building it themselves.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 1d2ed91e89 Extract payload building into dedicated classes
Separates notification payload construction from push delivery by
introducing DefaultPayload, EventPayload, and MentionPayload classes
that encapsulate the title, body, and URL generation for each
notification type.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 05819f84a2 Refactor notification push system with registry pattern
Replace NotificationPusher with a cleaner architecture:

- Add Notification::Pushable concern with push target registry
- Add Notification::Push base class with template methods
- Add Notification::Push::Web for web push (OSS)
- Add Notification::Push::Native for native push (SaaS)
- Add Notification::WebPushJob and Notification::NativePushJob

Key design:
- Registry pattern: Notification.register_push_target(:web)
- Template method: push calls should_push? then perform_push
- Subclasses override should_push? (with super) and perform_push
- Each target handles its own job enqueueing

Also:
- Add Notification#pushable? for checking push eligibility
- Add Notification#identity delegation to user
- Reorganize tests to match new class structure

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Tidy up saas engine a bit more
2026-02-25 19:31:13 +01:00
Fernando Olivares 7e0470a692 Send the URL instead of path in notifications 2026-02-25 19:31:13 +01:00
Fernando Olivares 3c54cd84fc Add native push notification infrastructure
- Add action_push_native gem and SaaS configuration
- Add device registration API and UI
- Add User::Devices concern
- Add NotificationPusher::Native for push delivery
- Add tests and fixtures for native push
2026-02-25 19:31:13 +01:00
Rosa Gutierrez 60b3c9c772 Fix duplicate push notification for mention and comment event
Event notifiers used the `mentionees` DB association to exclude mentioned
users from comment/card notifications. Since mentions are created async
via Mention::CreateJob, a race condition meant the mentionee list could
be empty when the event notification job ran first, causing the user to
receive both a comment and a mention push notification.

Use `scan_mentionees` instead, which scans the rich text body directly
for mentioned users without depending on Mention records existing yet.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-24 15:56:27 +01:00
Stanko K.R. 2f9d4533c5 Relativize absolute links to the instance on export 2026-02-24 08:22:01 +01:00
Stanko K.R. 4eadce8ffe Automatically update account slugs in links 2026-02-24 08:11:47 +01:00
Stanko K.R. 55779f8ac4 Organize the record sets better 2026-02-24 07:36:51 +01:00
Stanko K.R. d8ed2f44b8 Reconcile card count after import 2026-02-24 07:36:20 +01:00
Fernando 8d2075b653 Merge pull request #2580 from basecamp/fix-hyphenated-search
Fix hyphenated search
2026-02-20 12:14:38 -06:00
Rosa Gutierrez 3e5233239b Fix push notification not firing on notification creation
- Rails only applies the last callback when `after_create_commit` and
  `after_update_commit` reference the same method name [1]:

> However, if you use the `after_create_commit` and the
`after_update_commit` callback with the same method name, it will only
allow the last callback defined to take effect, as they both internally
alias to `after_commit` which overrides previously defined callbacks
with the same method name.

- Push notifications were never sent when a notification was first
  created — only when the source was updated
- Replaced the two callbacks with a single `after_save_commit`, which
  fires on both create and update, with the
  `source_id_previously_changed?` guard (true for both new records and
  source changes)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

[1] https://guides.rubyonrails.org/active_record_callbacks.html#aliases-for-after-commit
2026-02-20 18:37:33 +01:00
Fernando Olivares 554182f4b4 Fix stemmer stripping hyphens instead of splitting on them
The stemmer was concatenating tokens across hyphens (e.g. "BC3-IOS-1D8B"
→ "bc3ios1d8b") while the query sanitizer split on them, causing MySQL
MATCH AGAINST to never find the indexed token. Replace non-word chars
with spaces instead of stripping them so indexing and querying tokenize
consistently.

Requires search:reindex after deploy to rebuild existing search records.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-19 16:16:39 -06:00
Jeremy Daer 953942694f Handle MissingEOCD as invalid zip file, not job failure (#2572)
ZipKit::FileReader raises MissingEOCD for truncated, corrupted, or
non-zip files. This exception is a direct StandardError subclass, not
a subclass of InvalidStructure or ReadError, so it escaped as an
unhandled job failure instead of being caught and surfaced to the user.

Broaden the rescue in ZipFile::Reader#initialize to catch ReadError
(parent of InvalidStructure), MissingEOCD, and UnsupportedFeature.
2026-02-18 15:18:56 -08:00
Alp Keser 610cf7a190 Merge pull request #2398 from basecamp/notifications-tray-api
Add JSON response support for `notifications/tray`
2026-02-18 13:19:59 +03:00
Stanko Krtalić 4b2586769b Merge pull request #2556 from basecamp/fix-notifications-staying-unread-on-card-visit
Reinstate card readings
2026-02-17 14:15:24 +01:00
Stanko K.R. 9d4db14fc3 Reinstate card readings 2026-02-17 13:57:24 +01:00
Jeremy Daer a253c31dee Batch webhook delivery cleanup to prevent DB lock contention (#2555)
Follow-up to #2550 which added a created_at index and increased cleanup
frequency. The delete itself was still unbatched and vulnerable whenever
a backlog accumulates (first run after deploy, clock skew, etc).

Delete in small batches with a pause between each to let other
transactions through, following the SolidQueue pattern.
2026-02-16 15:31:10 -08:00
Rosa Gutierrez 95114b1d67 Fix race condition between concurrent Event and Mention notifier jobs
When concurrent NotifyRecipientsJobs process an Event and a Mention for
the same user+card, Rails' dirty tracking can skip writing source_type
in the UPDATE if it hasn't changed from the stale in-memory value,
leaving source_type and source_id mismatched (e.g. source_type='Event'
with a Mention's source_id, resulting in a nil source).

Force source_type to always be included in the UPDATE via
source_type_will_change! to prevent this.

Here's a sample timeline of this race condition happening in the real
world (with simplified IDs):

Two `NotifyRecipientsJob` involving notification `03fklpu`, same card,
same user, same comment — enqueued within 50ms of each other:

1. Both jobs load notification `03fklpu` — it has source_type='Mention'
   (from a previous job)
2. `EventNotifier` writes at 21:22:36.051: ```sql SET
   source_type='Event', source_id=<event_id>, unread_count=1 ```
3. `source_type` included because it changed ('Mention' → 'Event')
4. `MentionNotifier` writes at 21:22:36.057 (~6ms later): ```sql SET
   source_id=<mention_id>, unread_count=1 ```
5. No `source_type`! It was `'Mention'` when loaded and `'Mention'` is
   what it's setting → not dirty → skipped
6. Final DB state: `source_type='Event'` (from step 2, untouched),
   `source_id=<mention_id>` (from step 3)

`Notification.source` now does `Event.find(<mention_id>) → nil.`

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-16 16:50:46 +01:00
Alp Keser a48c88cb5d Stabilize notifications tray test ordering via fixture updated_at timestamps 2026-02-16 18:33:32 +03:00
Alp Keser bcc0f93bb9 Preload notification card associations used by JSON responses 2026-02-16 17:58:33 +03:00
Rosa Gutierrez b742963f7c Add account ownership check to import GID resolution
The convert_gids_to_sgids method resolved GIDs globally and minted
valid SGIDs without verifying the record belongs to the importing
account. This adds the same account_id check that the export path
already has, plus RecordNotFound handling for invalid GIDs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-16 11:49:00 +01:00
Mike Dalessio dc492ef17e Merge pull request #2540 from basecamp/fix-quotes
Fix double-escaped HTML entities in basecamp and campfire webhook payloads
2026-02-13 09:26:00 -05:00
Mike Dalessio 0c10622672 Fix double-escaped HTML entities in webhook payloads
`Event::Description#to_plain_text` now returns an html-safe string to
prevent double-escaping when the ERB template renders the basecamp
and campfire payloads.

ref: https://3.basecamp.com/2914079/buckets/27/card_tables/cards/9574495379
2026-02-13 03:25:59 -05:00
Stanko K.R. ae3d277868 Fix race condition on notification creation 2026-02-13 09:14:24 +01:00
Alp Keser abcbb74ac8 Make notification ordering deterministic. 2026-02-12 15:32:10 +03:00
Stanko K.R. 36ee253a1a Stack notifications everywhere
We had client-side notification stacking in the tray since launch, but now we want to stack notifications in the notifications page, in API responses and in email bundles.
2026-02-12 10:29:50 +01:00
Stanko K.R. 37d7f5c569 Give more detail about why the import failed 2026-02-11 13:47:21 +01:00
Mike Dalessio d611b2cc6c Move comment events when card moves to a new board (#2486)
When a card moves between boards, comment events were staying on the
old board. Now they move with the card.
2026-02-04 15:04:28 -05:00
Mike Dalessio 7f473eda66 Validate ActionText associations during import check phase
ActionTextRichTextRecordSet now calls check_associations_dont_exist to
ensure imported ActionText records only reference records within the
same import, matching the behavior of the parent RecordSet class.
2026-02-03 13:46:28 -05:00