diff --git a/app/assets/stylesheets/components/notes.css b/app/assets/stylesheets/components/notes.css index a9c1e1f..0ef0061 100644 --- a/app/assets/stylesheets/components/notes.css +++ b/app/assets/stylesheets/components/notes.css @@ -107,6 +107,25 @@ margin-bottom: var(--spacing-2); } +.note-form-fields { + display: flex; + flex-direction: column; + gap: var(--spacing-2); +} + +.note-meta-fields { + display: grid; + grid-template-columns: repeat(auto-fit, minmax(240px, 1fr)); + gap: var(--spacing-3); +} + +.note-field label { + display: block; + font-weight: var(--font-weight-medium); + margin-bottom: var(--spacing-1); + color: var(--color-text-primary); +} + .note-textarea { width: 100%; border-radius: var(--border-radius-sm); @@ -116,6 +135,21 @@ font-size: var(--font-size-base); } +.note-text-field { + width: 100%; + border-radius: var(--border-radius-sm); + border: var(--border-width) solid var(--color-border); + padding: var(--spacing-2); + font-family: var(--font-family-base); + font-size: var(--font-size-base); +} + +.note-hint { + margin-top: 4px; + color: var(--color-text-muted); + font-size: var(--font-size-xs); +} + .note-actions { display: flex; justify-content: flex-end; diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 7f79833..c9e59b5 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -7,7 +7,13 @@ class NotesController < ApplicationController def create topic = Topic.find(note_params[:topic_id]) message = resolve_message(topic) - note = NoteBuilder.new(author: current_user).create!(topic:, message:, body: note_params[:body]) + note = NoteBuilder.new(author: current_user).create!( + topic:, + message:, + body: note_params[:body], + tags: parsed_tags, + mention_names: parsed_mentions + ) redirect_to topic_path(topic, anchor: note_anchor(note)), notice: "Note added" rescue NoteBuilder::Error, ActiveRecord::RecordInvalid => e @@ -16,6 +22,8 @@ def create body: note_params[:body], message_id: note_params[:message_id].presence, topic_id: note_params[:topic_id], + tags_input: note_params[:tags_input], + mentions_input: note_params[:mentions_input], error: e.message } redirect_back fallback_location: topic_path(topic) @@ -24,7 +32,12 @@ def create def update return if performed? - NoteBuilder.new(author: current_user).update!(note: @note, body: note_params[:body]) + NoteBuilder.new(author: current_user).update!( + note: @note, + body: note_params[:body], + tags: parsed_tags, + mention_names: parsed_mentions + ) redirect_to topic_path(@note.topic, anchor: note_anchor(@note)), notice: "Note updated" rescue NoteBuilder::Error, ActiveRecord::RecordInvalid => e @@ -34,6 +47,8 @@ def update message_id: note_params[:message_id].presence, topic_id: note_params[:topic_id], note_id: @note.id, + tags_input: note_params[:tags_input], + mentions_input: note_params[:mentions_input], error: e.message } redirect_back fallback_location: topic_path(@note.topic) @@ -67,7 +82,7 @@ def set_note end def note_params - params.require(:note).permit(:body, :topic_id, :message_id) + params.require(:note).permit(:body, :topic_id, :message_id, :tags_input, :mentions_input) end def resolve_message(topic) @@ -82,4 +97,20 @@ def note_anchor(note) "thread-notes" end end + + def parsed_tags + split_tokens(note_params[:tags_input]) { |token| token.delete_prefix("#") } + end + + def parsed_mentions + split_tokens(note_params[:mentions_input]) { |token| token.delete_prefix("@") } + end + + def split_tokens(raw, &block) + Array(raw.to_s.split(/[,\s]+/)) + .map { |token| token.strip } + .reject(&:blank?) + .map(&block) + .reject(&:blank?) + end end diff --git a/app/services/note_builder.rb b/app/services/note_builder.rb index 51f5fbf..0ea3eaa 100644 --- a/app/services/note_builder.rb +++ b/app/services/note_builder.rb @@ -7,7 +7,7 @@ def initialize(author:) @author = author end - def create!(topic:, message: nil, body:) + def create!(topic:, message: nil, body:, tags: [], mention_names: []) raise Error, "You must be signed in to add a note" unless author body_text = body.to_s.strip raise Error, "Note cannot be blank" if body_text.blank? @@ -15,13 +15,13 @@ def create!(topic:, message: nil, body:) ActiveRecord::Base.transaction do note = Note.new(topic:, message:, author:, last_editor: author, body: body_text) note.save! - mentionables, tags = rebuild_mentions_and_tags!(note, body_text) + mentionables, tags = rebuild_mentions_and_tags!(note, tags:, mention_names:) fan_out!(note:, mentionables:, mark_author_read: true) note end end - def update!(note:, body:) + def update!(note:, body:, tags: [], mention_names: []) raise Error, "You do not have permission to edit this note" unless note.author_id == author.id body_text = body.to_s.strip raise Error, "Note cannot be blank" if body_text.blank? @@ -30,7 +30,7 @@ def update!(note:, body:) note.note_edits.create!(editor: author, body: note.body) if note.body != body_text note.update!(body: body_text, last_editor: author) - mentionables, _tags = rebuild_mentions_and_tags!(note, body_text) + mentionables, _tags = rebuild_mentions_and_tags!(note, tags:, mention_names:) fan_out!(note:, mentionables:, mark_author_read: false) note end @@ -40,10 +40,10 @@ def update!(note:, body:) attr_reader :author - def rebuild_mentions_and_tags!(note, body_text) - mention_names = extract_mentions(body_text) - mentionables = resolve_mentions(mention_names) - tags = extract_tags(body_text) + def rebuild_mentions_and_tags!(note, tags:, mention_names:) + normalized_mentions = normalize_mentions(mention_names) + mentionables = resolve_mentions(normalized_mentions) + normalized_tags = normalize_tags(tags) note.note_mentions.delete_all mentionables.each do |mentionable| @@ -51,27 +51,26 @@ def rebuild_mentions_and_tags!(note, body_text) end note.note_tags.delete_all - tags.each do |tag| + normalized_tags.each do |tag| note.note_tags.create!(tag:) end - [mentionables, tags] + [mentionables, normalized_tags] end - def extract_mentions(text) - text.to_s.scan(/(?:^|[^@\w])@([A-Za-z0-9_.-]+)/) - .flatten - .map { |m| NameReservation.normalize(m) } - .reject(&:blank?) - .uniq + def normalize_mentions(mention_names) + Array(mention_names) + .map { |m| m.to_s.delete_prefix("@") } + .map { |m| NameReservation.normalize(m) } + .reject(&:blank?) + .uniq end - def extract_tags(text) - text.to_s.scan(/(?:^|[^#\w])#([A-Za-z0-9_.-]+)/) - .flatten - .map { |t| t.to_s.strip.downcase } - .select { |tag| tag.match?(NoteTag::TAG_FORMAT) } - .uniq + def normalize_tags(tags) + Array(tags) + .map { |t| t.to_s.delete_prefix("#").strip.downcase } + .select { |tag| tag.match?(NoteTag::TAG_FORMAT) } + .uniq end def resolve_mentions(names) diff --git a/app/views/notes/_form.html.slim b/app/views/notes/_form.html.slim index 1fea4f3..99685f3 100644 --- a/app/views/notes/_form.html.slim +++ b/app/views/notes/_form.html.slim @@ -3,18 +3,33 @@ - note_error = flash[:note_error]&.with_indifferent_access - prefill_body = form_note.body +- prefill_tags = form_note.note_tags.map(&:tag).join(", ") +- prefill_mentions = form_note.note_mentions.includes(:mentionable).map { |mention| note_mention_label(mention) }.join(", ") - if note_error - matches_note = note_error[:note_id].present? && form_note.persisted? && form_note.id == note_error[:note_id].to_i - matches_new_note = !form_note.persisted? && note_error[:topic_id].to_i == topic.id && note_error[:message_id].to_s == message&.id.to_s - if matches_note || matches_new_note - prefill_body = note_error[:body] + - prefill_tags = note_error[:tags_input].to_s + - prefill_mentions = note_error[:mentions_input].to_s = form_with model: form_note, url: form_note.persisted? ? note_path(form_note) : notes_path, method: form_note.persisted? ? :patch : :post, data: { turbo: true } do |f| = hidden_field_tag "note[topic_id]", topic.id = hidden_field_tag "note[message_id]", message&.id - .note-field - = f.text_area :body, rows: 3, placeholder: "Add a note with @mentions and #tags…", required: true, class: "note-textarea", value: prefill_body - - if note_error && prefill_body == note_error[:body] - .note-error = note_error[:error] + .note-form-fields + .note-field + = f.label :body, "Note" + = f.text_area :body, rows: 4, placeholder: "Write your note", required: true, class: "note-textarea", value: prefill_body + - if note_error && prefill_body == note_error[:body] + .note-error = note_error[:error] + .note-meta-fields + .note-field + = label_tag :note_mentions_input, "Mentions" + = text_field_tag "note[mentions_input]", prefill_mentions, placeholder: "@alice, @team", class: "note-text-field" + .note-hint "Notify people or teams. Separate with commas or spaces." + .note-field + = label_tag :note_tags_input, "Tags" + = text_field_tag "note[tags_input]", prefill_tags, placeholder: "release, docs, follow-up", class: "note-text-field" + .note-hint "Optional tags for filtering. Lowercase letters, numbers, dash, underscore, or dot." .note-actions = f.submit submit_label, class: "button-secondary" diff --git a/db/seeds.rb b/db/seeds.rb index 6b1dc3f..f526ff0 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -913,48 +913,56 @@ def mark_aware_until(user:, topic:, message:, timestamp:) alice_notes.create!( topic: patch_topic, message: msg3, - body: "- Add WAL position to progress report\n- Split heap vs index counters\n- Autovacuum should emit too\n@ExampleCompany please sync on scope" + body: "- Add WAL position to progress report\n- Split heap vs index counters\n- Autovacuum should emit too\nExampleCompany please sync on scope", + mention_names: [example_team.name] ) bob_notes.create!( topic: patch_topic, message: msg1, - body: "Queued for next CF round; tracking in CF app.\n@ExampleCompany heads-up for review bandwidth" + body: "Queued for next CF round; tracking in CF app.\nExampleCompany heads-up for review bandwidth", + mention_names: [example_team.name] ) # RFC thread notes (thread + message, different authors) carol_notes.create!( topic: rfc_topic, - body: "Thread summary: add index AM hooks, include sample AM + docs.\n@ExampleCompany track follow-ups" + body: "Thread summary: add index AM hooks, include sample AM + docs.\nExampleCompany track follow-ups", + mention_names: [example_team.name] ) alice_notes.create!( topic: rfc_topic, message: rfc_msg2, - body: "Docs + sample AM needed before commit. Add SGML + README.\n@ExampleCompany can we help draft?" + body: "Docs + sample AM needed before commit. Add SGML + README.\nExampleCompany can we help draft?", + mention_names: [example_team.name] ) # Discussion thread: mix of thread/message notes from different people carol_notes.create!( topic: discussion_topic, - body: "Thread note: align logical slots with failover, add standby-safe flag.\n@ExampleCompany capture design risks" + body: "Thread note: align logical slots with failover, add standby-safe flag.\nExampleCompany capture design risks", + mention_names: [example_team.name] ) alice_notes.create!( topic: discussion_topic, message: disc_msg4, - body: "Message note: prototype slot fencing, watch cascading setups; prefer heartbeat piggyback.\n@ExampleCompany please review" + body: "Message note: prototype slot fencing, watch cascading setups; prefer heartbeat piggyback.\nExampleCompany please review", + mention_names: [example_team.name] ) # Committer topic: partially read with a note bob_notes.create!( topic: committer_topic, message: committer_messages[3], - body: "Action: test lower freeze_age under heavy autovacuum.\n@ExampleCompany check lab capacity" + body: "Action: test lower freeze_age under heavy autovacuum.\nExampleCompany check lab capacity", + mention_names: [example_team.name] ) # Moderate topic note for visibility carol_notes.create!( topic: moderate_topic_1, message: moderate_msgs_1[5], - body: "Pooling experiments look good; consider adaptive target.\n@ExampleCompany let's benchmark with PG16" + body: "Pooling experiments look good; consider adaptive target.\nExampleCompany let's benchmark with PG16", + mention_names: [example_team.name] ) # Notes on extra sampler topics for visibility across pages diff --git a/spec/services/note_builder_spec.rb b/spec/services/note_builder_spec.rb index f4687ae..3deb2f9 100644 --- a/spec/services/note_builder_spec.rb +++ b/spec/services/note_builder_spec.rb @@ -11,7 +11,13 @@ team_member = create(:user, username: "carl") create(:team_member, team:, user: team_member) - note = described_class.new(author: author).create!(topic:, message:, body: "Ping @bob and @team-two #Foo #bar") + note = described_class.new(author: author).create!( + topic:, + message:, + body: "Ping for review", + tags: %w[foo bar], + mention_names: %w[bob team-two] + ) expect(note.note_tags.pluck(:tag)).to match_array(%w[foo bar]) expect(note.note_mentions.map(&:mentionable)).to match_array([mentioned_user, team]) @@ -30,8 +36,8 @@ carol = create(:user, username: "carol") builder = described_class.new(author: author) - note = builder.create!(topic:, message:, body: "Hi @bob and @devs") - builder.update!(note:, body: "Hi @bob and @carol") + note = builder.create!(topic:, message:, body: "Hi team", mention_names: %w[bob devs]) + builder.update!(note:, body: "Hi friends", mention_names: %w[bob carol]) old_activity = Activity.find_by(user: dev_member, subject: note) new_activity = Activity.find_by(user: carol, subject: note)