diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..a7141f6a6 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,142 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## What is Fizzy? + +Fizzy is a collaborative project management and issue tracking application built by 37signals/Basecamp. It's a kanban-style tool for teams to create and manage cards (tasks/issues) across boards, organize work into columns representing workflow stages, and collaborate via comments, mentions, and assignments. + +## Development Commands + +### Setup and Server +```bash +bin/setup # Initial setup (installs gems, creates DB, loads schema) +bin/dev # Start development server (runs on port 3006) +``` + +Development URL: http://fizzy.localhost:3006 +Login with: david@37signals.com (development fixtures), password will appear in the browser console + +### Testing +```bash +bin/rails test # Run unit tests (fast) +bin/rails test test/path/file_test.rb # Run single test file +bin/rails test:system # Run system tests (Capybara + Selenium) +bin/ci # Run full CI suite (style, security, tests) + +# For parallel test execution issues, use: +PARALLEL_WORKERS=1 bin/rails test +``` + +CI pipeline (`bin/ci`) runs: +1. Rubocop (style) +2. Bundler audit (gem security) +3. Importmap audit +4. Brakeman (security scan) +5. Application tests +6. System tests + +### Database +```bash +bin/rails db:fixtures:load # Load fixture data +bin/rails db:migrate # Run migrations +bin/rails db:reset # Drop, create, and load schema +``` + +### Other Utilities +```bash +bin/rails dev:email # Toggle letter_opener for email preview +bin/jobs # Manage Solid Queue jobs +bin/kamal deploy # Deploy (requires 1Password CLI for secrets) +``` + +## Architecture Overview + +### Multi-Tenancy (URL-Based) + +Fizzy uses **URL path-based multi-tenancy**: +- Each Account (tenant) has a unique `external_account_id` (7+ digits) +- URLs are prefixed: `/{account_id}/boards/...` +- Middleware (`AccountSlug::Extractor`) extracts the account ID from the URL and sets `Current.account` +- The slug is moved from `PATH_INFO` to `SCRIPT_NAME`, making Rails think it's "mounted" at that path +- All models include `account_id` for data isolation +- Background jobs automatically serialize and restore account context + +**Key insight**: This architecture allows multi-tenancy without subdomains or separate databases, making local development and testing simpler. + +### Authentication & Authorization + +**Passwordless magic link authentication**: +- Global `Identity` (email-based) can have `Users` in multiple Accounts +- Users belong to an Account and have roles: admin, member, system +- Sessions managed via signed cookies +- Board-level access control via `Access` records + +### Core Domain Models + +**Account** → The tenant/organization +- Has users, boards, cards, tags, webhooks +- Has entropy configuration for auto-postponement + +**Identity** → Global user (email) +- Can have Users in multiple Accounts +- Session management tied to Identity + +**User** → Account membership +- Belongs to Account and Identity +- Has role (admin/member/system) +- Board access via explicit `Access` records + +**Board** → Primary organizational unit +- Has columns for workflow stages +- Can be "all access" or selective +- Can be published publicly with shareable key + +**Card** → Main work item (task/issue) +- Sequential number within each Account +- Rich text description and attachments +- Lifecycle: triage → columns → closed/not_now +- Automatically postpones after inactivity ("entropy") + +**Event** → Records all significant actions +- Polymorphic association to changed object +- Drives activity timeline, notifications, webhooks +- Has JSON `particulars` for action-specific data + +### Entropy System + +Cards automatically "postpone" (move to "not now") after inactivity: +- Account-level default entropy period +- Board-level entropy override +- Prevents endless todo lists from accumulating +- Configurable via Account/Board settings + +### UUID Primary Keys + +All tables use UUIDs (UUIDv7 format, base36-encoded as 25-char strings): +- Custom fixture UUID generation maintains deterministic ordering for tests +- Fixtures are always "older" than runtime records +- `.first`/`.last` work correctly in tests + +### Background Jobs (Solid Queue) + +Database-backed job queue (no Redis): +- Custom `FizzyActiveJobExtensions` prepended to ActiveJob +- Jobs automatically capture/restore `Current.account` +- Mission Control::Jobs for monitoring + +Key recurring tasks (via `config/recurring.yml`): +- Deliver bundled notifications (every 30 min) +- Auto-postpone stale cards (hourly) +- Cleanup jobs for expired links, deliveries + +### Sharded Full-Text Search + +16-shard MySQL full-text search instead of Elasticsearch: +- Shards determined by account ID hash (CRC32) +- Search records denormalized for performance +- Models in `app/models/search/` + +## Coding style + +Please read the separate file `STYLE.md` for some guidance on coding style. diff --git a/README.md b/README.md index d05487602..ff8b48cb9 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ And then run the development server: bin/dev -You'll be able to access the app in development at http://development-tenant.fizzy.localhost:3006 +You'll be able to access the app in development at http://fizzy.localhost:3006 ## Running tests diff --git a/STYLE.md b/STYLE.md new file mode 100644 index 000000000..bf9dd3be3 --- /dev/null +++ b/STYLE.md @@ -0,0 +1,217 @@ + +# Style + +We aim to write code that is a pleasure to read, and we have a lot of opinions about how to do it well. Writing great code is an essential part of our programming culture, and we deliberately set a high bar for every code change anyone contributes. We care about how code reads, how code looks, and how code makes you feel when you read it. + +We love discussing code. If you have questions about how to write something, or if you detect some smell you are not quite sure how to solve, please ask away to other programmers. A Pull Request is a great way to do this. + +When writing new code, unless you are very familiar with our approach, try to find similar code elsewhere to look for inspiration. + +## Conditional returns + +In general, we prefer to use expanded conditionals over guard clauses. + +```ruby +# Bad +def todos_for_new_group + ids = params.require(:todolist)[:todo_ids] + return [] unless ids + @bucket.recordings.todos.find(ids.split(",")) +end + +# Good +def todos_for_new_group + if ids = params.require(:todolist)[:todo_ids] + @bucket.recordings.todos.find(ids.split(",")) + else + [] + end +end +``` + +This is because guard clauses can be hard to read, especially when they are nested. + +As an exception, we sometimes use guard clauses to return early from a method: + +* When the return is right at the beginning of the method. +* When the main method body is not trivial and involves several lines of code. + +```ruby +def after_recorded_as_commit(recording) + return if recording.parent.was_created? + + if recording.was_created? + broadcast_new_column(recording) + else + broadcast_column_change(recording) + end +end +``` + +## Methods ordering + +We order methods in classes in the following order: + +1. `class` methods +2. `public` methods with `initialize` at the top. +3. `private` methods + +## Invocation order + +We order methods vertically based on their invocation order. This helps us to understand the flow of the code. + +```ruby +class SomeClass + def some_method + method_1 + method_2 + end + + private + def method_1 + method_1_1 + method_1_2 + end + + def method_1_1 + # ... + end + + def method_1_2 + # ... + end + + def method_2 + method_2_1 + method_2_2 + end + + def method_2_1 + # ... + end + + def method_2_2 + # ... + end +end +``` + +## To bang or not to bang + +Should I call a method `do_something` or `do_something!`? + +As a general rule, we only use `!` for methods that have a correspondent counterpart without `!`. In particular, we don’t use `!` to flag destructive actions. There are plenty of destructive methods in Ruby and Rails that do not end with `!`. + +## Visibility modifiers + +We don't add a newline under visibility modifiers, and we indent the content under them. + +```ruby +class SomeClass + def some_method + # ... + end + + private + def some_private_method_1 + # ... + end + + def some_private_method_2 + # ... + end +end +``` + +If a module only has private methods, we mark it `private` at the top and add an extra new line after but don't indent. + +```ruby +class SomeModule + private + + def some_private_method + # ... + end +end +``` + +## CRUD operations from controllers + +In general, we favor a vanilla Rails approach to CRUD operations. We create and update models from Rails controllers passing the parameters directly to the model constructor or update method. We do not use services or form objects to handle these operations. + +There are exceptional scenarios where we need to perform more complex operations, and we use form objects or higher-level service methods to handle them. We use the same pattern for both creations and updates. + +Related to this, we prefer to avoid [nested attributes](https://api.rubyonrails.org/classes/ActiveRecord/NestedAttributes/ClassMethods.html). If you find yourself wanting to use `accepts_nested_attributes_for`, that's a good smell that you might want to consider using a form object instead. + +As an example, you can check how we create and update messages in HEY's: `MessagesController`: + +```ruby +class MessagesController < ApplicationController + def create + @entry = Entry.enter \ + new_message, + on: new_topic, + status: :drafted, + address: entry_addressed_param, + scheduled_delivery_at: entry_scheduled_delivery_at_param, + scheduled_bubble_up_on: entry_scheduled_bubble_up_on_param + + respond_to_saved_entry @entry + end + + def update + previously_scheduled = @entry.scheduled_delivery + + @entry.revise \ + message_params, + status: :drafted, + is_delivery_imminent: !entry_status_param.drafted?, + address: entry_addressed_param, + scheduled_delivery_at: entry_scheduled_delivery_at_param, + scheduled_bubble_up_on: entry_scheduled_bubble_up_on_param + + respond_to_saved_entry(@entry, previously_scheduled: previously_scheduled) + end +end + +class Entry < ApplicationRecord + def self.enter(*args, **kwargs) + Entry::Enter.new(*args, **kwargs).perform + end + + def revise(*args, **kwargs) + Entry::Revise.new(self, *args, **kwargs).perform + end +end +``` + +## Run async operations in jobs + +As a general rule, we write shallow job classes that delegate the logic itself to domain models: + +* We typically use the suffix `_later` to flag methods that enqueue a job. +* A common scenario is having a model class that enqueues a job that, when executed, invokes some method in that same class. In this case, we use the suffix `_now` for the regular synchronous method. + +```ruby +module Event::Relaying + extend ActiveSupport::Concern + + included do + after_create_commit :relay_later + end + + def relay_later + Event::RelayJob.perform_later(self) + end + + def relay_now + # ... + end +end + +class Event::RelayJob < ApplicationJob + def perform(event) + event.relay_now + end +end +``` diff --git a/app/assets/stylesheets/card-columns.css b/app/assets/stylesheets/card-columns.css index 351d37eda..217f9a5c2 100644 --- a/app/assets/stylesheets/card-columns.css +++ b/app/assets/stylesheets/card-columns.css @@ -18,7 +18,6 @@ --progress-max-cards: 20; --progress-max-height: 50dvh; - -ms-overflow-style: none; /* Hide x-scrollbar on Edge */ container-type: inline-size; display: grid; gap: var(--column-gap); @@ -29,12 +28,6 @@ overflow-y: hidden; padding-block-end: var(--column-width-collapsed); position: relative; - scrollbar-width: none; /* Hide x-scrollbar on FF */ - - /* Hide x-scrollbar on Chrome/Safari/Opera */ - &::-webkit-scrollbar { - display: none; - } /* When it has something expanded */ &:has(.card-columns__left .cards:not(.is-collapsed), .card-columns__right .cards:not(.is-collapsed)) { @@ -673,7 +666,6 @@ /* -------------------------------------------------------------------------- */ /* Surface a mini bubble if there are cards with bubbles inside */ - .cards--considering:has(.bubble:not([hidden])), .cards--doing.is-collapsed:has(.bubble:not([hidden])) { .cards__transition-container { --bubble-color: var(--card-color, oklch(var(--lch-blue-medium))); @@ -690,7 +682,7 @@ inline-size: 1em; inset: 0 0 auto auto; position: absolute; - translate: 0 0; + translate: 20% -20%; z-index: 1; } } diff --git a/app/assets/stylesheets/inputs.css b/app/assets/stylesheets/inputs.css index c8b935d7c..109bfbb26 100644 --- a/app/assets/stylesheets/inputs.css +++ b/app/assets/stylesheets/inputs.css @@ -49,8 +49,11 @@ } } + /* Target mobile Safari only */ @supports (hanging-punctuation: first) and (font: -apple-system-body) and (-webkit-appearance: none) { - font-size: max(16px, 1em) !important; + @media (hover: none) { + font-size: max(16px, 1em) !important; + } } } diff --git a/app/assets/stylesheets/utilities.css b/app/assets/stylesheets/utilities.css index 1c61680f1..5b73e1266 100644 --- a/app/assets/stylesheets/utilities.css +++ b/app/assets/stylesheets/utilities.css @@ -249,4 +249,14 @@ display: unset; } } + + .hide-scrollbar { + -ms-overflow-style: none; /* Edge */ + scrollbar-width: none; /* FF */ + + /* Chrome/Safari/Opera */ + &::-webkit-scrollbar { + display: none; + } + } } diff --git a/app/controllers/admin/stats_controller.rb b/app/controllers/admin/stats_controller.rb new file mode 100644 index 000000000..25d19ad31 --- /dev/null +++ b/app/controllers/admin/stats_controller.rb @@ -0,0 +1,20 @@ +class Admin::StatsController < AdminController + disallow_account_scope + + layout "public" + + def show + @accounts_total = Account.count + @accounts_last_7_days = Account.where(created_at: 7.days.ago..).count + @accounts_last_24_hours = Account.where(created_at: 24.hours.ago..).count + + @identities_total = Identity.count + @identities_last_7_days = Identity.where(created_at: 7.days.ago..).count + @identities_last_24_hours = Identity.where(created_at: 24.hours.ago..).count + + @top_accounts = Account + .where("cards_count > 0") + .order(cards_count: :desc) + .limit(20) + end +end diff --git a/app/controllers/concerns/authorization.rb b/app/controllers/concerns/authorization.rb index 9a3a8279a..46769992e 100644 --- a/app/controllers/concerns/authorization.rb +++ b/app/controllers/concerns/authorization.rb @@ -22,7 +22,7 @@ module Authorization end def ensure_staff - head :forbidden unless Current.user.staff? + head :forbidden unless Current.identity.staff? end def ensure_can_access_account diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 6d790c232..863522893 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -29,10 +29,14 @@ class JoinCodesController < ApplicationController private def set_join_code - @join_code ||= Account::JoinCode.active.find_by(code: params.expect(:code), account: Current.account) + @join_code ||= Account::JoinCode.find_by(code: params.expect(:code), account: Current.account) end def ensure_join_code_is_valid - head :not_found unless @join_code&.active? + if @join_code.nil? + head :not_found + elsif !@join_code.active? + render :inactive, status: :gone + end end end diff --git a/app/controllers/my/pins_controller.rb b/app/controllers/my/pins_controller.rb index 27e14535b..4468c75d5 100644 --- a/app/controllers/my/pins_controller.rb +++ b/app/controllers/my/pins_controller.rb @@ -1,6 +1,6 @@ class My::PinsController < ApplicationController def index @pins = Current.user.pins.includes(:card).ordered.limit(20) - fresh_when @pins + fresh_when etag: [ @pins, @pins.collect(&:card) ] end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 36d30d6fe..8c623b6ef 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,6 +1,6 @@ class SessionsController < ApplicationController # FIXME: Remove this before launch! - if Rails.env.remote? + unless Rails.env.local? http_basic_authenticate_with \ name: Rails.application.credentials.account_signup_http_basic_auth.name, password: Rails.application.credentials.account_signup_http_basic_auth.password, diff --git a/app/helpers/cards_helper.rb b/app/helpers/cards_helper.rb index 574f6375f..ba61cb94e 100644 --- a/app/helpers/cards_helper.rb +++ b/app/helpers/cards_helper.rb @@ -33,6 +33,10 @@ module CardsHelper title.join(" ") end + def card_drafted_or_added(card) + card.drafted? ? "Drafted" : "Added" + end + def card_social_tags(card) tag.meta(property: "og:title", content: "#{card.title} | #{card.board.name}") + tag.meta(property: "og:description", content: format_excerpt(@card&.description, length: 200)) + diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 0b04f6f44..6b8d20c04 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -15,11 +15,14 @@ module FiltersHelper user_filtering.selected_board_titles.collect { tag.strong it }.to_sentence.html_safe end - def filter_place_menu_item(path, label, icon, new_window: false, current: false) - link_to_params = new_window ? { target: "_blank" } : {} + def filter_place_menu_item(path, label, icon, new_window: false, current: false, turbo: true) + link_to_params = {} + link_to_params.merge!({ target: "_blank" }) if new_window + link_to_params.merge!({ data: { turbo: false } }) unless turbo + tag.li class: "popup__item", id: "filter-place-#{label.parameterize}", data: { filter_target: "item", navigable_list_target: "item" }, aria: { checked: current } do concat icon_tag(icon, class: "popup__icon") - concat(link_to(path, link_to_params.merge(class: "popup__btn btn")) do + concat(link_to(path, link_to_params.merge(class: "popup__btn btn"), data: { turbo: turbo }) do concat tag.span(label, class: "overflow-ellipsis") concat icon_tag("check", class: "checked flex-item-justify-end", "aria-hidden": true) end) diff --git a/app/helpers/hotkeys_helper.rb b/app/helpers/hotkeys_helper.rb index 3e75081f9..810a0f83e 100644 --- a/app/helpers/hotkeys_helper.rb +++ b/app/helpers/hotkeys_helper.rb @@ -5,7 +5,7 @@ module HotkeysHelper if key == "ctrl" && platform.mac? "⌘" elsif key == "enter" - "⏎" + platform.mac? ? "return" : "enter" elsif key == "shift" "⇧" else diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 029c328bb..355f60550 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -7,15 +7,22 @@ module NotificationsHelper end def event_notification_body(event) - name = event.creator.name + creator = event.creator.name case event_notification_action(event) - when "card_closed" then "Moved to Done by #{name}" - when "card_reopened" then "Reopened by #{name}" - when "card_published" then "Added by #{name}" - when "comment_created" then comment_notification_body(event) when "card_assigned" then "Assigned to #{event.assignees.none? ? "self" : event.assignees.pluck(:name).to_sentence}" - else name + when "card_unassigned" then "Unassigned by #{creator}" + when "card_published" then "Added by #{creator}" + when "card_closed" then "Moved to Done by #{creator}" + when "card_reopened" then "Reopened by #{creator}" + when "card_postponed" then "Moved to Not Now by #{creator}" + when "card_auto_postponed" then "Moved to Not Now due to inactivity" + when "card_title_changed" then "Renamed by #{creator}" + when "card_board_changed" then "Moved by #{creator}" + when "card_triaged" then "Moved to #{event.particulars.dig("particulars", "column")} by #{creator}" + when "card_sent_back_to_triage" then "Moved back to Maybe? by #{creator}" + when "comment_created" then comment_notification_body(event) + else creator end end diff --git a/app/javascript/controllers/frame_controller.js b/app/javascript/controllers/frame_controller.js index 7684a43ed..c905576ba 100644 --- a/app/javascript/controllers/frame_controller.js +++ b/app/javascript/controllers/frame_controller.js @@ -15,4 +15,8 @@ export default class extends Controller { this.element.reload() } } + + reload() { + this.element.reload() + } } diff --git a/app/mailers/magic_link_mailer.rb b/app/mailers/magic_link_mailer.rb index f23cc4fbf..617286f50 100644 --- a/app/mailers/magic_link_mailer.rb +++ b/app/mailers/magic_link_mailer.rb @@ -3,11 +3,6 @@ class MagicLinkMailer < ApplicationMailer @magic_link = magic_link @identity = @magic_link.identity - mail to: @identity.email_address, subject: "Your Fizzy verification code" + mail to: @identity.email_address, subject: "Your Fizzy code is #{ @magic_link.code }" end - - private - def default_url_options - Rails.application.config.action_mailer.default_url_options - end end diff --git a/app/mailers/notification/bundle_mailer.rb b/app/mailers/notification/bundle_mailer.rb index c821a1274..db3c81bc7 100644 --- a/app/mailers/notification/bundle_mailer.rb +++ b/app/mailers/notification/bundle_mailer.rb @@ -11,6 +11,6 @@ class Notification::BundleMailer < ApplicationMailer mail \ to: bundle.user.identity.email_address, - subject: "Latest Activity in Fizzy" + subject: "Fizzy#{ " (#{ Current.account.name })" if @user.identity.accounts.many? }: Latest Activity" end end diff --git a/app/models/card/activity_spike/detector.rb b/app/models/card/activity_spike/detector.rb index b589e0c26..b533318e3 100644 --- a/app/models/card/activity_spike/detector.rb +++ b/app/models/card/activity_spike/detector.rb @@ -21,11 +21,7 @@ class Card::ActivitySpike::Detector def register_activity_spike Card.suppressing_turbo_broadcasts do - if card.activity_spike - card.activity_spike.touch - else - card.create_activity_spike! - end + Card::ActivitySpike.find_or_create_by!(card: card).touch end end diff --git a/app/models/card/statuses.rb b/app/models/card/statuses.rb index 2639d88b7..d339acd17 100644 --- a/app/models/card/statuses.rb +++ b/app/models/card/statuses.rb @@ -20,7 +20,7 @@ module Card::Statuses private def update_created_at_on_publication if will_save_change_to_status? && status_in_database.inquiry.drafted? - self.created_at = Time.now + self.created_at = Time.current end end end diff --git a/app/models/filter.rb b/app/models/filter.rb index f579c6bf0..7a398d03d 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -36,7 +36,7 @@ class Filter < ApplicationRecord result.mentioning(term, user: creator) end - result + result.distinct end end diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index a4d87e72d..e6f63acaa 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -38,11 +38,13 @@ class Notification::Bundle < ApplicationRecord def deliver user.in_time_zone do - processing! + Current.with_account(user.account) do + processing! - Notification::BundleMailer.notification(self).deliver if deliverable? + Notification::BundleMailer.notification(self).deliver if deliverable? - delivered! + delivered! + end end end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 81e69ce07..24c1b3b4e 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -14,7 +14,8 @@ class Notifier def notify if should_notify? - recipients.map do |recipient| + # Processing recipients in order avoids deadlocks if notifications overlap. + recipients.sort_by(&:id).map do |recipient| Notification.create! user: recipient, source: source, creator: creator end end diff --git a/app/views/account/join_codes/edit.html.erb b/app/views/account/join_codes/edit.html.erb index 44854c6d0..7f22b9318 100644 --- a/app/views/account/join_codes/edit.html.erb +++ b/app/views/account/join_codes/edit.html.erb @@ -2,8 +2,9 @@ <% content_for :header do %>