diff --git a/AGENTS.md b/AGENTS.md index e8d1f5d63..51407a6fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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`. diff --git a/app/assets/stylesheets/web/header.css b/app/assets/stylesheets/web/header.css index 8ef1b4094..c2471e522 100644 --- a/app/assets/stylesheets/web/header.css +++ b/app/assets/stylesheets/web/header.css @@ -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 { diff --git a/app/controllers/account/settings_controller.rb b/app/controllers/account/settings_controller.rb index a076479cb..27f47ab99 100644 --- a/app/controllers/account/settings_controller.rb +++ b/app/controllers/account/settings_controller.rb @@ -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 diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb index 258bffc1a..1e4233696 100644 --- a/app/controllers/boards_controller.rb +++ b/app/controllers/boards_controller.rb @@ -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 diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 863522893..7e7ded229 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -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 diff --git a/app/controllers/notifications/trays_controller.rb b/app/controllers/notifications/trays_controller.rb index 26a1f06e5..c2bac662a 100644 --- a/app/controllers/notifications/trays_controller.rb +++ b/app/controllers/notifications/trays_controller.rb @@ -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. diff --git a/app/javascript/controllers/navigable_list_controller.js b/app/javascript/controllers/navigable_list_controller.js index f1d22e92d..4af794c4b 100644 --- a/app/javascript/controllers/navigable_list_controller.js +++ b/app/javascript/controllers/navigable_list_controller.js @@ -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) } } diff --git a/app/models/account/seeder.rb b/app/models/account/seeder.rb index f7f7cf60b..9858cff6b 100644 --- a/app/models/account/seeder.rb +++ b/app/models/account/seeder.rb @@ -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 diff --git a/app/models/card.rb b/app/models/card.rb index 33a993f71..0a589d0b7 100644 --- a/app/models/card.rb +++ b/app/models/card.rb @@ -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 diff --git a/app/models/card/assignable.rb b/app/models/card/assignable.rb index c43c50526..be0dd257e 100644 --- a/app/models/card/assignable.rb +++ b/app/models/card/assignable.rb @@ -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? diff --git a/app/models/comment.rb b/app/models/comment.rb index a42b205fc..f81c465ad 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -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" }) } diff --git a/app/models/event.rb b/app/models/event.rb index c759c18ee..bbbf2a60c 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -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 diff --git a/app/models/filter.rb b/app/models/filter.rb index 7a398d03d..9dac977bd 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -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? diff --git a/app/models/notification.rb b/app/models/notification.rb index 416506783..545f6ab75 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index 976f1bdc0..90aab0338 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,4 +25,8 @@ class User < ApplicationRecord update! active: false, identity: nil end end + + def setup? + name != identity.email_address + end end diff --git a/app/models/user/day_timeline.rb b/app/models/user/day_timeline.rb index b9ae4941a..1fe4ed36e 100644 --- a/app/models/user/day_timeline.rb +++ b/app/models/user/day_timeline.rb @@ -71,7 +71,7 @@ class User::DayTimeline def timelineable_events Event - .includes(:creator, :board, :eventable) + .preloaded .where(board: boards) .where(action: TIMELINEABLE_ACTIONS) end diff --git a/app/models/user/day_timeline/column.rb b/app/models/user/day_timeline/column.rb index 0331a83ff..ce87873fa 100644 --- a/app/models/user/day_timeline/column.rb +++ b/app/models/user/day_timeline/column.rb @@ -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 diff --git a/app/views/cards/_messages.html.erb b/app/views/cards/_messages.html.erb index c92c94f7c..402a39ec0 100644 --- a/app/views/cards/_messages.html.erb +++ b/app/views/cards/_messages.html.erb @@ -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 %> diff --git a/app/views/cards/comments/reactions/_reactions.html.erb b/app/views/cards/comments/reactions/_reactions.html.erb index 6de217842..ba3d491eb 100644 --- a/app/views/cards/comments/reactions/_reactions.html.erb +++ b/app/views/cards/comments/reactions/_reactions.html.erb @@ -1,7 +1,7 @@ <%= turbo_frame_tag comment, :reacting do %>
- <%= render partial: "cards/comments/reactions/reaction", collection: comment.reactions.includes(:reacter).ordered %> + <%= render partial: "cards/comments/reactions/reaction", collection: comment.reactions %>
<%= turbo_frame_tag comment, :new_reaction do %> diff --git a/app/views/events/day_timeline/_column.html.erb b/app/views/events/day_timeline/_column.html.erb index 23bf7e34e..509efd69d 100644 --- a/app/views/events/day_timeline/_column.html.erb +++ b/app/views/events/day_timeline/_column.html.erb @@ -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" %> Expand column <% end %> diff --git a/db/seeds.rb b/db/seeds.rb index 5e2ab12ac..80523e86e 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -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) diff --git a/test/controllers/cards/assignments_controller_test.rb b/test/controllers/cards/assignments_controller_test.rb index 9cfcc469f..c823174a8 100644 --- a/test/controllers/cards/assignments_controller_test.rb +++ b/test/controllers/cards/assignments_controller_test.rb @@ -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 diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index 9b386cb8f..74613c724 100644 --- a/test/controllers/join_codes_controller_test.rb +++ b/test/controllers/join_codes_controller_test.rb @@ -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 diff --git a/test/models/card_test.rb b/test/models/card_test.rb index 4858e7203..e18ce43f8 100644 --- a/test/models/card_test.rb +++ b/test/models/card_test.rb @@ -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 diff --git a/test/models/identity/joinable_test.rb b/test/models/identity/joinable_test.rb new file mode 100644 index 000000000..8a8d40e1c --- /dev/null +++ b/test/models/identity/joinable_test.rb @@ -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 diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 6f5028bdb..5ffc5db56 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -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