Commit Graph

1329 Commits

Author SHA1 Message Date
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
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. 05042ed54a Fix crash when a notification is deleted while a bundle is being sent 2026-02-13 09:22:51 +01:00
Mike Dalessio 4ead778e12 Backfill tests for Event::Description strings' html-safety 2026-02-12 15:52:56 -05:00
Mike Dalessio 851f13a934 Render inline code in card titles (#2518)
* Remove unused marked JS dependency

* Remove unused redcarpet dependency

* Render inline code in card titles

Add card_html_title helper that HTML-escapes input then converts
backtick-wrapped text to <code> elements. Apply to card titles in
board preview, card detail, public views, and notification emails.
Style inline code elements in titles to match description styling.

Co-authored-by: Andy Smith <andy@37signals.com>

---------

Co-authored-by: Andy Smith <andy@37signals.com>
2026-02-12 12:07:40 -05: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 8c5b47d774 Group notification emails by board (#2506)
* Group notification emails by board

Notification bundle emails now group items by board instead of
showing the board name redundantly for each card. Each board
section has a linked header and an <hr> separator.

* Adjust board title styles and remove rem units

* Sort board groups alphabetically

Sort notification email board sections alphabetically by name
(case-insensitive).

Also, rewrite mailer tests to use Nokogiri::HTML5 for precise DOM
assertions instead of regex matching.

---------

Co-authored-by: Andy Smith <andy@37signals.com>
2026-02-09 11:29:11 -05: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 e311d8f521 Omit inactive users from board mention picker
The board-scoped mention prompt was returning all users with board access,
including those marked as inactive. This adds the `.active` scope to match
the behavior of the account-wide mention prompt.

Fixes https://app.fizzy.do/5986089/cards/3861
2026-02-03 18:42:15 -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
Mike Dalessio 015bc3420e Fix boost button appearing prematurely on draft cards
When updating a draft card (e.g., adding a background image), the turbo
stream response was using the published card container partial, which
includes the reactions/boost button. Draft cards should use the draft
container partial which intentionally omits reactions.

Fixes https://app.fizzy.do/5986089/cards/3970
2026-02-03 09:28:08 -05:00
Stanko K.R. a4bec6e407 Ignore missing attachments 2026-02-03 13:51:22 +01:00
Stanko K.R. 4b70c1cc3b Replace custom IO object with a transport manager IO 2026-02-02 19:31:18 +01:00
Stanko K.R. 77d9331d90 Add missing method to IO object 2026-02-02 19:06:37 +01:00
Stanko K.R. 5c929238b7 Rename jobs 2026-02-02 18:22:07 +01:00
Stanko K.R. b06d95a20d Delete accounts for failed imports 2026-02-02 16:24:10 +01:00
Stanko K.R. 96a1d7d2d3 Fix crash on successful import email 2026-02-02 14:47:58 +01:00
Stanko K.R. a6609e1bbc Add test got the import job 2026-02-02 13:03:26 +01:00
Stanko K.R. 27c73ff42b Rename validate to check
Validate is an ActiveModel method, overriding it could have unintended side-effects so I opted to rename it
2026-02-02 12:36:48 +01:00
Stanko K.R. 992f15066b Replace RubyZip with ZipKit
ZipKit can read and write files directly from S3 which makes both imports and exports way more efficient in terms of disk usage.
2026-02-02 12:36:48 +01:00
Stanko K.R. 184150006f Unify ZIP file interactions between all exports 2026-02-02 12:36:48 +01:00
Stanko K.R. e0cee2c770 Simplify the import test 2026-02-02 12:36:48 +01:00
Stanko K.R. f50d6d093d Implement cursors for imports 2026-02-02 12:36:48 +01:00
Stanko K.R. 63f6be4ef5 Resolve import conflicts 2026-02-02 12:36:48 +01:00
Stanko K.R. b97934ff48 Fix crash when exporting ActiveStorage files 2026-02-02 12:36:48 +01:00
Stanko K.R. 00c5f7feab Break import and export objects into record sets 2026-02-02 12:36:48 +01:00
Stanko K.R. 19ae555c2e Convert Account::SingleUserExport to User::DataExport 2026-02-02 12:36:48 +01:00
Stanko K.R. 161efb0b8e Implement basic imports 2026-02-02 12:36:48 +01:00
Stanko K.R. 6f17e3867d Split Export model into user and whole-account export 2026-02-02 12:36:31 +01:00
Rosa Gutierrez 63d9f29b1e Return internal account ID after signup completion
This matches what we return from `/my/identity` endpoint for each
account.
2026-01-30 13:41:37 +01:00
Rosa Gutierrez 614527e9c2 Add JSON format support to signup flow for native API clients
The magic link verification endpoint now returns `requires_signup_completion`
to indicate whether the client needs to collect a name and complete signup.
The signup completion endpoint now supports JSON responses with the account ID.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-30 13:41:37 +01:00
Kevin McConnell e105ca6fb5 Remove zero-padding from account slug
This shortens URLs in the self-hosted case, where external account ID is
typically a small number.
2026-01-28 11:59:42 +00:00
Rosa Gutierrez c059e0b216 Add JSON format support to session destroy for native API clients
This is so that native API clients can get the session record deleted
server-side. They still need to take care of clearing any cookies
they've set in web views.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-27 19:59:30 +01:00
Denis Švara a75d56d243 Merge branch 'main' into pins-api
* main: (127 commits)
  Align board name start so it doesn't get too big
  Clean up card/events header layout
  Sync email to Stripe when user changes email address (#2432)
  Fix IDOR in webhook activation endpoint (#2431)
  Add card reactions to API docs and reactions_url to card JSON (#2427)
  Remove unnecessary claude plan
  Allow boosts on cards (#2411)
  Revert "Fix notification click URL by using correct data property"
  Add migration to remove draft cards from search index
  Guard search indexing with searchable? check
  Forbid comments on draft cards
  prefactor: update search to use published cards
  Fix notification click URL by using correct data property
  Wait for service worker to be active before subscribing
  Fix stuck state when permission granted but no subscription
  Extract Card::Commentable
  Include arm64 build in Docker workflow
  Remove unnecessary `await` in push handler
  Correctly initialise WebPush connection (#2417)
  Update models, views, and fixtures for polymorphic reactions
  ...
2026-01-27 15:37:47 +01:00
Jeremy Daer 4efa4da809 Fix IDOR in webhook activation endpoint (#2431)
The webhook activation controller was using account-scoped lookup
instead of board-scoped lookup, allowing users to reactivate webhooks
on boards they don't have access to.

This was an oversight when board-scoping was added to the main webhooks
controller - the activations controller was missed in that update.

The fix adds the BoardScoped concern to properly restrict webhook
activation to boards the user has explicit access to.
2026-01-24 17:06:12 -08:00
Mike Dalessio 16745e89d1 Add card reactions to API docs and reactions_url to card JSON (#2427)
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-23 16:55:50 -05:00
Mike Dalessio cb61b36715 Allow boosts on cards (#2411)
* Add reactions association to Card model

Adds `has_many :reactions` to Card, matching the implementation in Comment.
This allows cards to be reacted to directly with emoji reactions.

The association:
- Orders reactions chronologically
- Uses polymorphic `:reactable` interface
- Deletes reactions when card is destroyed

Includes test coverage for the new association.

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

* Add reactions UI to card detail view

- Create Cards::ReactionsController with turbo stream responses
- Add card reactions views (reactions list, new form, menu partial)
- Position reaction button flush-right when no reactions exist
- Show reactions on bottom-left when present, with button inline
- Handle narrow viewports by flowing button below meta section
- Add controller tests for card reactions

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

* Add boost count to card preview

Display reaction count alongside comment count on card tiles in board
columns. Introduces a .card__counts wrapper to group both counts with
proper positioning on all viewport sizes.

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

* Consolidate reaction views for cards and comments

Extract shared views to app/views/reactions/ using polymorphic routing.
Both card and comment reactions now render the same partials, with a
helper to compute the correct path prefix for nested routes.

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

* Fix emoji picker width in card reactions

Override .card .popup { inline-size: 260px } for the reaction popup
so the emoji grid displays without horizontal scrollbar.

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

* Hide boost count where comment count is hidden

Add .card__boosts alongside .card__comments in:
- Tray view (display: none)
- Cards with background images (opacity toggle on hover)

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

* Fix reactions button overlap with zoom button in card footer

When a card has a background image, the footer contains both a reactions
button and a zoom button. Previously they would overlap because the
reactions button was absolutely positioned to the bottom-right.

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

* Tighten up previews

* Move card reactions from meta partial to container

Fixes two issues:

1. Reactions were duplicated each time a card was assigned because the
   turbo stream replaced the meta div with the full perma/meta partial,
   which included reactions outside the replaced element.

2. Draft cards incorrectly showed the boost button because the meta
   partial was shared between published and draft containers.

Moving reactions to _container.html.erb (published cards only) fixes
both issues and keeps the meta partial focused on meta content.

Fixes https://app.fizzy.do/5986089/cards/3837
Fixes https://app.fizzy.do/5986089/cards/3835

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

* WIP adjust perma page

* Position and style the footer

* Render the stamp in the card body instead of the footer

* Fix undefined local variable in public cards view

Changed `card` to `@card` when rendering the stamp partial.

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

* Preload card reactions to avoid N+1 queries

Include reactions and their reacters in the preloaded scope, matching
what we already do for comments.

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

* Add model tests for card and comment reaction cleanup

Test that:
- Creating a card reaction touches the card's last_active_at
- Reactions are deleted when their parent comment is destroyed
- Reactions are deleted when their parent card is destroyed

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

* Minor improvements to reactions code

- Use size instead of count in boosts partial to avoid extra SQL query
  on already-loaded collection
- Add else clause to reaction_path_prefix_for to raise ArgumentError
  for unknown reactable types instead of silently returning nil

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

* Bump specificity of reaction popup in the card perma

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Andy Smith <andy@37signals.com>
2026-01-23 13:25:55 -05:00
Mike Dalessio 0d138df952 Guard search indexing with searchable? check
Only index cards and comments if their card is `published?`, using a
new method `searchable?` that is implemented on both Card and Comment.
This prevents draft cards and their comments from being indexed.

When drafts are published, the existing `update_in_search_index`
callback creates the record via `upsert!`, or else ensures unpublished
records are removed from the index.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-23 12:13:08 -05:00
Mike Dalessio 9aa15e9fb2 Forbid comments on draft cards
- Add Card#commentable? method that returns true only for published cards.
- CardsController#create ensures that the card is commentable.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-23 11:28:26 -05:00
Mike Dalessio bb2d3a59b5 prefactor: update search to use published cards
We're about to make a change to assert that draft cards are not added
to the search index, and so let's make the intention behind these
tests clear first.
2026-01-23 11:07:43 -05:00
Mike Dalessio 1aea6850f0 Extract Card::Commentable 2026-01-23 10:30:12 -05:00
Kevin McConnell 23fe40fa81 Correctly initialise WebPush connection (#2417)
The `ipaddr` arg here is being interpreted as a positional arg (since
the keyword arg doesn't exist), which results in an invalid connection
object. This was causing push notifications to silently fail.

We should initialise the property on the object instead.
2026-01-22 12:03:02 -08:00
Mike Dalessio ffab4cad6f Update models, views, and fixtures for polymorphic reactions
- Reaction: belongs_to :reactable (polymorphic) instead of :comment
- Comment: has_many :reactions with as: :reactable
- Views: use reaction.reactable instead of reaction.comment
- Fixtures: use polymorphic syntax for reactable association

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-22 12:29:54 -05:00
Kevin McConnell 468fe5f16b Only index up to 32KB of search content
To prevent overflowing the columns in the search index tables (and also
just to avoid creating massive indexes), let's limit the searchable
content to the first 32KB.
2026-01-21 09:52:23 +00:00
Rosa Gutierrez 967f76de99 Fix notification broadcast test for turbo-rails 2.0.21
turbo-rails 2.0.21 changed `capture_turbo_stream_broadcasts` to only
return broadcasts produced within the block, matching its documented
behavior (see hotwired/turbo-rails#736).

Since `broadcast_unread` uses `broadcast_prepend_later_to` (async via
job), we need to wrap the call in `perform_enqueued_jobs` so the
broadcast actually happens within the assertion block.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-19 19:49:35 +01:00
Mike Dalessio 6b8a571fd3 Merge pull request #2383 from basecamp/flavorjones/3722-self-assign
Fix "M" hotkey to not use stale user ID from the fragment cache
2026-01-16 13:47:18 -05:00
Mike Dalessio 2527f07365 Fix "M" hotkey using stale user from fragment cache
Add a SelfAssignmentsController to resolve the current user at request
time, avoiding the use of an incorrect user id baked into a cached URL.

ref: #2211 originally added the "M" hotkey
ref: https://app.fizzy.do/5986089/cards/3722
2026-01-16 13:41:33 -05:00