Merge branch 'main' into mobile-app/scoped-stylesheets

* main: (24 commits)
  style: rubocop fix
  Fix: scrolling to parent navigable list despite of setting
  Fix join codes skipping user setup
  Avoid N+1 on account and board settings pages
  Remove N+1 on the cards board.
  Preload Event to avoid N+1s on the timeline
  Remove a bunch of N+1s
  Make db/seeds.rb rerunnable when I want to add more cards.
  Fix that board button was leaking to the right of columns
  Set entropy to 1 year so cards don't close before they can play
  Improve AGENTS.md with some loki suggestions
  Prevent Fizzy menu from closing on page refreshes
  Rename bin/gitleaks to gitleaks-audit
  Set entropy to 1 year so cards don't close before they can play
  Improve AGENTS.md with some loki suggestions
  Prevent Fizzy menu from closing on page refreshes
  Rename bin/gitleaks to gitleaks-audit
  Improve AGENTS.md with some loki suggestions
  Prevent Fizzy menu from closing on page refreshes
  Rename bin/gitleaks to gitleaks-audit
  ...
This commit is contained in:
Adrien Maston
2025-11-28 12:20:03 +01:00
26 changed files with 144 additions and 32 deletions
+33 -3
View File
@@ -152,10 +152,40 @@ Grafana MCP tools provide access to production metrics and logs for performance
- `rails_request_total:rate1m:sum_by_controller_action{app="fizzy"}` - Request rates by endpoint
- `fizzy_replica_wait_seconds` - Database replica consistency wait times
### Loki Log Labels
Query: `{service_namespace="fizzy", deployment_environment_name="production", service_name="rails"}`
### Loki Log Labels and Query Patterns
Useful fields: `event_duration_ms`, `performance_time_db_ms`, `performance_time_cpu_ms`, `rails_endpoint`, `rails_controller`
**Base label selector:**
```logql
{service_namespace="fizzy", deployment_environment_name="production", service_name="rails"}
```
**Useful JSON fields:** `event_duration_ms`, `performance_time_db_ms`, `performance_time_cpu_ms`, `rails_endpoint`, `rails_controller`, `url_path`, `authentication_identity_id`, `http_response_status_code`
**Query patterns:**
- Filter by fields: `{labels} | field_name = "value"`
- Multiple field filters: `{labels} | field1 = "value1" | field2 = "value2"`
- Reduce returned labels: `{labels} | filters | keep field1,field2,field3` (reduces label payload)
- Minimize log line content: `{labels} | filters | line_format "{{.field_name}}"` (replaces raw log line)
- Combine both for minimal tokens: `{labels} | filters | keep field1,field2 | line_format "{{.field1}}"`
- **Important:** Fields are pre-parsed by the OTel collector. Don't use string search (`|=`) when filtering structured fields
- **Important:** Do NOT use `| json` - it will cause JSONParserErr since fields are already parsed as labels
**Token management (CRITICAL):**
- Always probe with `limit: 3` first to check response size before running larger queries
- Aggregations return time series (many data points), not single values - can explode token usage
- NEVER use `sum by (field)` - returns a time series per unique value, easily exceeds token limits
- For breakdowns by field: fetch raw logs with `| keep field | line_format "{{.field}}"` and count client-side
**Aggregations for statistics (use instead of fetching raw logs):**
- `mcp__grafana__query_loki_logs` returns limited results (default 10, max ~100) and large responses get truncated; use aggregations for statistics on large datasets
- Count: `sum(count_over_time({labels} | filters [12h]))`
- Percentiles: `quantile_over_time(0.95, {labels} | filters | unwrap field_name | __error__="" [12h]) by ()`
- Average: `avg_over_time({labels} | filters | unwrap field_name | __error__="" [12h]) by ()`
- Min/Max: `min_over_time(...)` / `max_over_time(...)`
- The `| unwrap field_name | __error__=""` pattern extracts numeric values from pre-parsed labels
- Use `by ()` or wrap in `sum()` to avoid cardinality limits
**Documentation:** For advanced LogQL syntax (aggregations, pattern matching, etc.), consult https://grafana.com/docs/loki/latest/query/
### Instrumentation
Yabeda-based metrics exported at `:9394/metrics`. Config in `config/initializers/yabeda.rb`.
-1
View File
@@ -43,7 +43,6 @@
font-size: var(--text-x-small);
gap: var(--header-gap);
inline-size: var(--header-actions-width);
min-inline-size: fit-content;
}
.header__actions--start {
@@ -3,7 +3,7 @@ class Account::SettingsController < ApplicationController
before_action :set_account
def show
@users = @account.users.active.alphabetically
@users = @account.users.active.alphabetically.includes(:identity)
end
def update
+1 -1
View File
@@ -24,7 +24,7 @@ class BoardsController < ApplicationController
def edit
selected_user_ids = @board.users.pluck :id
@selected_users, @unselected_users = \
@board.account.users.active.alphabetically.partition { |user| selected_user_ids.include? user.id }
@board.account.users.active.alphabetically.includes(:identity).partition { |user| selected_user_ids.include? user.id }
end
def update
+4 -1
View File
@@ -13,9 +13,12 @@ class JoinCodesController < ApplicationController
identity = Identity.find_or_create_by!(email_address: params.expect(:email_address))
@join_code.redeem { |account| identity.join(account) } unless identity.member_of?(@join_code.account)
user = User.active.find_by!(account: @join_code.account, identity: identity)
if identity == Current.identity
if identity == Current.identity && user.setup?
redirect_to landing_url(script_name: @join_code.account.slug)
elsif identity == Current.identity
redirect_to new_users_join_url(script_name: @join_code.account.slug)
else
terminate_session if Current.identity
@@ -1,6 +1,6 @@
class Notifications::TraysController < ApplicationController
def show
@notifications = Current.user.notifications.unread.ordered.limit(100)
@notifications = Current.user.notifications.preloaded.unread.ordered.limit(100)
# Invalidate on the whole set instead of the unread set since the max updated at in the unread set
# can stay the same when reading old notifications.
@@ -161,7 +161,7 @@ export default class extends Controller {
#relayNavigationToParentNavigableList(event) {
const parentController = this.#parentNavigableListController
if (parentController) {
parentController.element.focus({ preventScroll: !this.autoScrollValue })
parentController.element.focus({ preventScroll: !parentController.autoScrollValue })
parentController.navigate(event)
}
}
+1
View File
@@ -25,6 +25,7 @@ class Account::Seeder
# Playground Board
# ---------------
playground = account.boards.create! name: "Playground", creator: creator, all_access: true
playground.update! auto_postpone_period: 365.days
# Cards
playground.cards.create! creator: creator, title: "Finally, watch this Fizzy orientation video", status: "published", description: <<~HTML
+1 -1
View File
@@ -22,7 +22,7 @@ class Card < ApplicationRecord
scope :chronologically, -> { order created_at: :asc, id: :asc }
scope :latest, -> { order last_active_at: :desc, id: :desc }
scope :with_users, -> { preload(creator: [ :avatar_attachment, :account ], assignees: [ :avatar_attachment, :account ]) }
scope :preloaded, -> { with_users.preload(:column, :tags, :steps, :closure, :goldness, :activity_spike, :image_attachment, board: [ :entropy ], not_now: [ :user ]).with_rich_text_description_and_embeds }
scope :preloaded, -> { with_users.preload(:column, :tags, :steps, :closure, :goldness, :activity_spike, :image_attachment, board: [ :entropy, :columns ], not_now: [ :user ]).with_rich_text_description_and_embeds }
scope :indexed_by, ->(index) do
case index
+1 -1
View File
@@ -15,7 +15,7 @@ module Card::Assignable
end
def assigned_to?(user)
assignments.exists? assignee: user
assignments.any? { |a| a.assignee_id == user.id }
end
def assigned?
+2 -1
View File
@@ -4,11 +4,12 @@ class Comment < ApplicationRecord
belongs_to :account, default: -> { card.account }
belongs_to :card, touch: true
belongs_to :creator, class_name: "User", default: -> { Current.user }
has_many :reactions, dependent: :delete_all
has_many :reactions, -> { order(:created_at) }, dependent: :delete_all
has_rich_text :body
scope :chronologically, -> { order created_at: :asc, id: :desc }
scope :preloaded, -> { with_rich_text_body.includes(reactions: :reacter) }
scope :by_system, -> { joins(:creator).where(creator: { role: "system" }) }
scope :by_user, -> { joins(:creator).where.not(creator: { role: "system" }) }
+10
View File
@@ -9,6 +9,16 @@ class Event < ApplicationRecord
has_many :webhook_deliveries, class_name: "Webhook::Delivery", dependent: :delete_all
scope :chronologically, -> { order created_at: :asc, id: :desc }
scope :preloaded, -> {
includes(:creator, :board, {
eventable: [
:goldness, :closure, :image_attachment,
{ rich_text_body: :embeds_attachments },
{ rich_text_description: :embeds_attachments },
{ card: [ :goldness, :closure, :image_attachment ] }
]
})
}
after_create -> { eventable.event_was_created(self) }
after_create_commit :dispatch_webhooks
+1 -1
View File
@@ -18,7 +18,7 @@ class Filter < ApplicationRecord
def cards
@cards ||= begin
result = creator.accessible_cards.published
result = creator.accessible_cards.preloaded.published
result = result.indexed_by(indexed_by)
result = result.sorted_by(sorted_by)
result = result.where(id: card_ids) if card_ids.present?
+1 -1
View File
@@ -14,7 +14,7 @@ class Notification < ApplicationRecord
after_destroy_commit :broadcast_read
after_create :bundle
scope :preloaded, -> { preload(:creator, :account, source: [ :board, :creator ]) }
scope :preloaded, -> { preload(:creator, :account, source: [ :board, :creator, { eventable: [ :closure, :board, :assignments ] } ]) }
delegate :notifiable_target, to: :source
delegate :card, to: :source
+4
View File
@@ -25,4 +25,8 @@ class User < ApplicationRecord
update! active: false, identity: nil
end
end
def setup?
name != identity.email_address
end
end
+1 -1
View File
@@ -71,7 +71,7 @@ class User::DayTimeline
def timelineable_events
Event
.includes(:creator, :board, :eventable)
.preloaded
.where(board: boards)
.where(action: TIMELINEABLE_ACTIONS)
end
+9
View File
@@ -17,6 +17,15 @@ class User::DayTimeline::Column
safe_join(parts, " ")
end
def base_title_for_route
case base_title
when "Done"
"closed"
else
base_title.downcase
end
end
def events_by_hour
limited_events.group_by { it.created_at.hour }
end
+1 -1
View File
@@ -1,6 +1,6 @@
<%= messages_tag(card) do %>
<% if card.published? %>
<%= render partial: "cards/comments/comment", collection: card.comments.chronologically, cached: true %>
<%= render partial: "cards/comments/comment", collection: card.comments.preloaded.chronologically, cached: true %>
<%= render "cards/comments/new", card: card %>
<%= render "cards/comments/watchers", card: card %>
@@ -1,7 +1,7 @@
<%= turbo_frame_tag comment, :reacting do %>
<div class="reactions">
<div id="<%= dom_id(comment, :reactions) %>" class="reactions__list">
<%= render partial: "cards/comments/reactions/reaction", collection: comment.reactions.includes(:reacter).ordered %>
<%= render partial: "cards/comments/reactions/reaction", collection: comment.reactions %>
</div>
<%= turbo_frame_tag comment, :new_reaction do %>
@@ -3,7 +3,7 @@
<%= column.title %>
<% if column.events_by_hour.any? %>
<%= link_to events_day_timeline_column_path(id: column.base_title.downcase, day: column.day_timeline.day.to_date), class: "events__maximize-button btn btn--circle txt-x-small borderless", data: { turbo_frame: "_top" } do %>
<%= link_to events_day_timeline_column_path(id: column.base_title_for_route, day: column.day_timeline.day.to_date), class: "events__maximize-button btn btn--circle txt-x-small borderless", data: { turbo_frame: "_top" } do %>
<%= icon_tag "grid", class: "translucent" %>
<span class="for-screen-reader">Expand column</span>
<% end %>
+13 -11
View File
@@ -16,16 +16,18 @@ else
email_address = "david@37signals.com"
identity = Identity.find_or_create_by!(email_address: email_address)
account = Account.create_with_admin_user(
account: {
external_account_id: tenant_id,
name: signal_account_name
},
owner: {
name: "David Heinemeier Hansson",
identity: identity
}
)
unless account = Account.find_by(external_account_id: tenant_id)
account = Account.create_with_admin_user(
account: {
external_account_id: tenant_id,
name: signal_account_name
},
owner: {
name: "David Heinemeier Hansson",
identity: identity
}
)
end
Current.account = account
end
@@ -43,7 +45,7 @@ else
end
def create_board(name, creator: Current.user, all_access: true, access_to: [])
Board.create!(name:, creator:, all_access:).tap { it.accesses.grant_to(access_to) }
Board.find_or_create_by!(name:, creator:, all_access:).tap { it.accesses.grant_to(access_to) }
end
def create_card(title, board:, description: nil, status: :published, creator: Current.user)
@@ -11,12 +11,12 @@ class Cards::AssignmentsControllerTest < ActionDispatch::IntegrationTest
end
test "create" do
assert_changes "cards(:logo).assigned_to?(users(:david))", from: false, to: true do
assert_changes "cards(:logo).reload.assigned_to?(users(:david))", from: false, to: true do
post card_assignments_path(cards(:logo)), params: { assignee_id: users(:david).id }, as: :turbo_stream
assert_meta_replaced(cards(:logo))
end
assert_changes "cards(:logo).assigned_to?(users(:david))", from: true, to: false do
assert_changes "cards(:logo).reload.assigned_to?(users(:david))", from: true, to: false do
post card_assignments_path(cards(:logo)), params: { assignee_id: users(:david).id }, as: :turbo_stream
assert_meta_replaced(cards(:logo))
end
@@ -43,6 +43,9 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest
identity = identities(:jz)
sign_in_as :jz
assert identity.member_of?(@account), "JZ should be a member of 37s for this test"
assert identity.users.find_by!(account: @account).setup?, "JZ's user should be setup for this test"
assert_no_difference -> { Identity.count } do
assert_no_difference -> { User.count } do
post join_path(code: @join_code.code, script_name: @account.slug), params: { email_address: identity.email_address }
@@ -51,4 +54,19 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to landing_url(script_name: @account.slug)
end
test "create for signed-in identity without a user in the account redirects to user setup" do
identity = identities(:mike)
sign_in_as :mike
assert_not identity.member_of?(@account), "Mike should not be a member of 37s for this test"
assert_no_difference -> { Identity.count } do
assert_difference -> { User.count }, 1 do
post join_path(code: @join_code.code, script_name: @account.slug), params: { email_address: identity.email_address }
end
end
assert_redirected_to new_users_join_url(script_name: @account.slug)
end
end
+1 -1
View File
@@ -37,7 +37,7 @@ class CardTest < ActiveSupport::TestCase
assert_difference({ -> { cards(:logo).assignees.count } => -1, -> { Event.count } => +1 }) do
cards(:logo).toggle_assignment users(:kevin)
end
assert_not cards(:logo).assigned_to?(users(:kevin))
assert_not cards(:logo).reload.assigned_to?(users(:kevin))
unassign_event = Event.last
assert_equal "card_unassigned", unassign_event.action
assert_equal [ users(:kevin) ], unassign_event.assignees
+25
View File
@@ -0,0 +1,25 @@
require "test_helper"
class Identity::JoinableTest < ActiveSupport::TestCase
test "join" do
identity = identities(:david)
user = identity.join(accounts(:initech))
assert_kind_of User, user
assert_equal accounts(:initech), user.account
assert_equal identity.email_address, user.name
identity = identities(:mike)
user = identity.join(accounts("37s"), name: "Mike")
assert_kind_of User, user
assert_equal accounts("37s"), user.account
assert_equal "Mike", user.name
end
test "member_of?" do
identity = identities(:david)
assert identity.member_of?(accounts("37s"))
assert_not identity.member_of?(accounts(:initech))
end
end
+10
View File
@@ -35,4 +35,14 @@ class UserTest < ActiveSupport::TestCase
assert_equal "DHH", User.new(name: "David Heinemeier Hansson").initials
assert_equal "ÉLH", User.new(name: "Éva-Louise Hernández").initials
end
test "setup?" do
user = users(:kevin)
user.update!(name: user.identity.email_address)
assert_not user.setup?
user.update!(name: "Kevin")
assert user.setup?
end
end