Commit Graph

708 Commits

Author SHA1 Message Date
Rosa Gutierrez 9ded80d448 Add test for notifying new mentionees when editing a comment
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-24 15:56:27 +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. 0648583289 Fix other test failures caused by default host manipulation 2026-02-24 08:35:11 +01:00
Stanko K.R. 66fdcae960 Fix CI failure with no host set 2026-02-24 08:28:52 +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
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 85dc8ba455 Add repeated punctuation search stemmer test 2026-02-19 16:23:26 -06:00
Fernando Olivares 3932fce309 Add tests for hyphenated string search
Cover the stemmer tokenizing hyphenated input into separate words and
end-to-end search finding cards with hyphenated titles.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-19 16:16:44 -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
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 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
Mike Dalessio 4ead778e12 Backfill tests for Event::Description strings' html-safety 2026-02-12 15:52:56 -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 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
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. 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
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 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
Rosa Gutierrez ee636f1994 Delete orphaned watches and pins when a card is moved to a private board
For watchers and users with pins that don't have access to the private
board where the card was moved to.
2026-01-14 19:38:15 +01:00
Rosa Gutierrez 5e099c4c06 Delete pins belonging to cards in a board with revoked access
As part of cleaning up inaccessible data in the board after revoking
access.
2026-01-13 16:25:59 +01:00
Jeremy Daer 006e61ea58 Allow ActionText embed reuse and safe purge (#2346) 2026-01-12 10:30:18 -08:00
Stanko Krtalić eceb6c9b43 Add self-service account deletion (#2246)
* Add self-service account deletion

* Disable access to cancelled accounts & implement Stripe interactions

* Add tests

* Fix failing tests

* Remove cancelled accounts from lists

* Fix incorrect redirect

* Use _path instead of _url for consistency

* Don't track how far the inicieration job got

We still want the step tracking so that the job can be interrupted. But, since the scope is idempotent, we don't need to track how far it got.

* Specify the exact time when the data will be deleted

* Fix crash due to unadvancable cursor

* Rename up_for_incineration to due_for_incineration

* Regenrate the schema

* Fix incorrect path check

* Migrate the SQLite schema

* Only show the cancel button on cancellable accounts

* Check that a subscirption method exists before calling it

* Ignore job failures due to missing records when an account gets deleted

* Skip sending notifications on cancelled accounts

* Use collbacks to integrate

* Add a blank line between queue_as and discard_on

* Break checks into methods

* Inline methods

* Rename Account::IncinerateJob

* Run migrations

* Migrate SQLite
2026-01-12 14:21:05 +01:00
Jorge Manrubia 3457c871e1 Revert "Add CJK (Chinese, Japanese, Korean) search support" 2026-01-08 21:07:14 +01:00
Jorge Manrubia db804b8450 Merge pull request #2299 from Maklu/fix-cjk-search
Add CJK (Chinese, Japanese, Korean) search support
2026-01-08 12:00:46 +01:00
Jorge Manrubia e34716a537 Merge pull request #2221 from pinzonjulian/patch-1
Fix test state pollution caused by filter/search_test
2026-01-07 16:40:18 +01:00