From cd1e6378d0fc643bb534d69da4234d7f02bd958c Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 7 Jan 2025 17:13:56 +0000 Subject: [PATCH 1/5] Add `drafted` and `published` states --- app/assets/stylesheets/bubbles.css | 4 ++++ .../bubbles/publishes_controller.rb | 8 ++++++++ app/controllers/bubbles_controller.rb | 2 +- app/models/bubble.rb | 3 ++- app/models/bubble/draftable.rb | 9 +++++++++ app/views/bubbles/_bubble.html.erb | 2 +- app/views/bubbles/_publish.html.erb | 4 ++++ app/views/bubbles/show.html.erb | 14 ++++++++----- config/routes.rb | 1 + .../20250107165422_add_status_to_bubbles.rb | 5 +++++ db/schema.rb | 1 + test/fixtures/bubbles.yml | 4 ++++ test/models/bubble/draftable_test.rb | 20 +++++++++++++++++++ 13 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 app/controllers/bubbles/publishes_controller.rb create mode 100644 app/models/bubble/draftable.rb create mode 100644 app/views/bubbles/_publish.html.erb create mode 100644 db/migrate/20250107165422_add_status_to_bubbles.rb create mode 100644 test/models/bubble/draftable_test.rb diff --git a/app/assets/stylesheets/bubbles.css b/app/assets/stylesheets/bubbles.css index 91b65091e..9b7f88d5e 100644 --- a/app/assets/stylesheets/bubbles.css +++ b/app/assets/stylesheets/bubbles.css @@ -443,6 +443,10 @@ border: var(--bubble-border-width) solid var(--bubble-color); } + .bubble.drafted & { + border-style: dashed; + } + .bubble:has(.bubble__link) & { z-index: -1; } diff --git a/app/controllers/bubbles/publishes_controller.rb b/app/controllers/bubbles/publishes_controller.rb new file mode 100644 index 000000000..947311bd6 --- /dev/null +++ b/app/controllers/bubbles/publishes_controller.rb @@ -0,0 +1,8 @@ +class Bubbles::PublishesController < ApplicationController + include BubbleScoped, BucketScoped + + def create + @bubble.published! + redirect_to @bubble + end +end diff --git a/app/controllers/bubbles_controller.rb b/app/controllers/bubbles_controller.rb index 920b8b181..8aa6850d8 100644 --- a/app/controllers/bubbles_controller.rb +++ b/app/controllers/bubbles_controller.rb @@ -7,7 +7,7 @@ class BubblesController < ApplicationController before_action :set_bubble, only: %i[ show edit update destroy ] def index - @bubbles = @filter.bubbles + @bubbles = @filter.bubbles.published_or_drafted_by(Current.user) end def create diff --git a/app/models/bubble.rb b/app/models/bubble.rb index 5b6a0ec93..5313038ed 100644 --- a/app/models/bubble.rb +++ b/app/models/bubble.rb @@ -1,5 +1,6 @@ class Bubble < ApplicationRecord - include Assignable, Boostable, Colored, Commentable, Eventable, Messages, Notifiable, Poppable, Searchable, Staged, Taggable + include Assignable, Boostable, Colored, Commentable, Draftable, + Eventable, Messages, Notifiable, Poppable, Searchable, Staged, Taggable belongs_to :bucket, touch: true belongs_to :creator, class_name: "User", default: -> { Current.user } diff --git a/app/models/bubble/draftable.rb b/app/models/bubble/draftable.rb new file mode 100644 index 000000000..8d50c6be4 --- /dev/null +++ b/app/models/bubble/draftable.rb @@ -0,0 +1,9 @@ +module Bubble::Draftable + extend ActiveSupport::Concern + + included do + enum :status, %w[ drafted published ].index_by(&:itself) + + scope :published_or_drafted_by, ->(user) { where(status: :published).or(where(creator: user)) } + end +end diff --git a/app/views/bubbles/_bubble.html.erb b/app/views/bubbles/_bubble.html.erb index f00d502ff..be53dffda 100644 --- a/app/views/bubbles/_bubble.html.erb +++ b/app/views/bubbles/_bubble.html.erb @@ -1,5 +1,5 @@ <% cache bubble do %> -
" style="view-transition-name: bubble-<%= bubble.id -%>; --bubble-color: <%= bubble.color %>; <%= bubble_rotation(bubble) %> <%= bubble_size(bubble) %>" data-controller="animation upload-preview" data-animation-play-class="bubble--wobble" diff --git a/app/views/bubbles/_publish.html.erb b/app/views/bubbles/_publish.html.erb new file mode 100644 index 000000000..0f7af76c3 --- /dev/null +++ b/app/views/bubbles/_publish.html.erb @@ -0,0 +1,4 @@ +<%= button_to bucket_bubble_publish_path(bubble.bucket, bubble), class: "btn btn--positive full-width justify-start borderless" do %> + <%= image_tag "alert.svg", aria: { hidden: true }, size: 24 %> + Publish +<% end %> diff --git a/app/views/bubbles/show.html.erb b/app/views/bubbles/show.html.erb index 8ecd07f4f..2ce7194fe 100644 --- a/app/views/bubbles/show.html.erb +++ b/app/views/bubbles/show.html.erb @@ -56,12 +56,16 @@ <% end %>
- <%= render "bubbles/pop_toggle", bubble: @bubble %> + <% if @bubble.drafted? %> + <%= render "bubbles/publish", bubble: @bubble %> + <% else %> + <%= render "bubbles/pop_toggle", bubble: @bubble %> + + <% end %> - <%= button_to bucket_bubble_path(@bubble.bucket, @bubble), method: :delete, diff --git a/config/routes.rb b/config/routes.rb index ad0d9a2cf..f122bb209 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -29,6 +29,7 @@ Rails.application.routes.draw do scope module: :bubbles do resource :image resource :pop + resource :publish resource :stage_picker resources :stagings end diff --git a/db/migrate/20250107165422_add_status_to_bubbles.rb b/db/migrate/20250107165422_add_status_to_bubbles.rb new file mode 100644 index 000000000..abe8b481f --- /dev/null +++ b/db/migrate/20250107165422_add_status_to_bubbles.rb @@ -0,0 +1,5 @@ +class AddStatusToBubbles < ActiveRecord::Migration[8.1] + def change + add_column :bubbles, :status, :text, default: :drafted, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index b50200c5c..b10589aaa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -106,6 +106,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do t.integer "stage_id" t.integer "comments_count", default: 0, null: false t.integer "activity_score", default: 0, null: false + t.text "status", default: "drafted", null: false t.index ["bucket_id"], name: "index_bubbles_on_bucket_id" t.index ["stage_id"], name: "index_bubbles_on_stage_id" end diff --git a/test/fixtures/bubbles.yml b/test/fixtures/bubbles.yml index 556637e7c..8bbde128f 100644 --- a/test/fixtures/bubbles.yml +++ b/test/fixtures/bubbles.yml @@ -8,6 +8,7 @@ logo: boosts_count: 5 comments_count: 2 activity_score: 7 + status: published layout: bucket: writebook @@ -17,6 +18,7 @@ layout: created_at: <%= 1.week.ago %> comments_count: 1 activity_score: 1 + status: published text: bucket: writebook @@ -24,6 +26,7 @@ text: title: The text is too small color: "#3B4B59" created_at: <%= 1.week.ago %> + status: published shipping: bucket: writebook @@ -31,3 +34,4 @@ shipping: title: We need to ship the app color: "#ED8008" created_at: <%= 1.week.ago %> + status: published diff --git a/test/models/bubble/draftable_test.rb b/test/models/bubble/draftable_test.rb new file mode 100644 index 000000000..5a13f1f66 --- /dev/null +++ b/test/models/bubble/draftable_test.rb @@ -0,0 +1,20 @@ +require "test_helper" + +class Bubble::DraftableTest < ActiveSupport::TestCase + test "bubbles are only visible to the creator by default" do + bubble = buckets(:writebook).bubbles.create! creator: users(:kevin), title: "Drafted Bubble" + + assert bubble.drafted? + assert_includes Bubble.published_or_drafted_by(users(:kevin)), bubble + assert_not_includes Bubble.published_or_drafted_by(users(:jz)), bubble + end + + test "bubbles are visible to everyone when published" do + bubble = buckets(:writebook).bubbles.create! creator: users(:kevin), title: "Drafted Bubble" + bubble.published! + + assert bubble.published? + assert_includes Bubble.published_or_drafted_by(users(:kevin)), bubble + assert_includes Bubble.published_or_drafted_by(users(:jz)), bubble + end +end From 782259a678de8235a21824f3ed6a6a34b597f422 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 16:52:27 +0000 Subject: [PATCH 2/5] Events should track `published` now Previously when an item was `created`, we'd track the event, update the summary text, and broadcast the notifications. But now that we have a draft state, we shold do all of this when it's published instead. --- app/controllers/bubbles/publishes_controller.rb | 2 +- app/helpers/notifications_helper.rb | 2 +- app/models/bubble/draftable.rb | 7 +++++++ app/models/bubble/eventable.rb | 4 ---- app/models/event_summary.rb | 2 +- app/models/notifier/created.rb | 2 -- app/models/notifier/published.rb | 2 ++ test/controllers/readings_controller_test.rb | 2 +- test/fixtures/events.yml | 16 ++++++++-------- test/fixtures/notifications.yml | 4 ++-- .../{created_test.rb => published_test.rb} | 6 +++--- test/models/notifier_test.rb | 2 +- 12 files changed, 27 insertions(+), 24 deletions(-) delete mode 100644 app/models/notifier/created.rb create mode 100644 app/models/notifier/published.rb rename test/models/notifier/{created_test.rb => published_test.rb} (60%) diff --git a/app/controllers/bubbles/publishes_controller.rb b/app/controllers/bubbles/publishes_controller.rb index 947311bd6..8064e7c68 100644 --- a/app/controllers/bubbles/publishes_controller.rb +++ b/app/controllers/bubbles/publishes_controller.rb @@ -2,7 +2,7 @@ class Bubbles::PublishesController < ApplicationController include BubbleScoped, BucketScoped def create - @bubble.published! + @bubble.publish redirect_to @bubble end end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 7c35a5080..62474229b 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -14,7 +14,7 @@ module NotificationsHelper case notification.event.action when "assigned" then "#{name} assigned to you" - when "created" then "Added by #{name}" + when "published" then "Added by #{name}" when "popped" then "Popped by by #{name}" else name end diff --git a/app/models/bubble/draftable.rb b/app/models/bubble/draftable.rb index 8d50c6be4..1f2879eeb 100644 --- a/app/models/bubble/draftable.rb +++ b/app/models/bubble/draftable.rb @@ -6,4 +6,11 @@ module Bubble::Draftable scope :published_or_drafted_by, ->(user) { where(status: :published).or(where(creator: user)) } end + + def publish + transaction do + published! + track_event :published + end + end end diff --git a/app/models/bubble/eventable.rb b/app/models/bubble/eventable.rb index 64379e360..ccd085870 100644 --- a/app/models/bubble/eventable.rb +++ b/app/models/bubble/eventable.rb @@ -1,10 +1,6 @@ module Bubble::Eventable extend ActiveSupport::Concern - included do - after_create -> { track_event :created, creator: creator } - end - private def track_event(action, creator: Current.user, **particulars) event = find_or_capture_event_summary.events.create! action: action, creator: creator, particulars: particulars diff --git a/app/models/event_summary.rb b/app/models/event_summary.rb index 2c1d23b07..b24818fac 100644 --- a/app/models/event_summary.rb +++ b/app/models/event_summary.rb @@ -17,7 +17,7 @@ class EventSummary < ApplicationRecord def summarize(event) case event.action - when "created" + when "published" "Added by #{event.creator.name} #{time_ago_in_words(event.created_at)} ago." when "assigned" "Assigned to #{event.assignees.pluck(:name).to_sentence} #{time_ago_in_words(event.created_at)} ago." diff --git a/app/models/notifier/created.rb b/app/models/notifier/created.rb deleted file mode 100644 index f2724a931..000000000 --- a/app/models/notifier/created.rb +++ /dev/null @@ -1,2 +0,0 @@ -class Notifier::Created < Notifier -end diff --git a/app/models/notifier/published.rb b/app/models/notifier/published.rb new file mode 100644 index 000000000..d5b50b87b --- /dev/null +++ b/app/models/notifier/published.rb @@ -0,0 +1,2 @@ +class Notifier::Published < Notifier +end diff --git a/test/controllers/readings_controller_test.rb b/test/controllers/readings_controller_test.rb index bb173b4a2..7d124736d 100644 --- a/test/controllers/readings_controller_test.rb +++ b/test/controllers/readings_controller_test.rb @@ -6,7 +6,7 @@ class ReadingsControllerTest < ActionDispatch::IntegrationTest end test "index" do - assert_changes -> { notifications(:logo_created_kevin).reload.read? }, from: false, to: true do + assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do post bucket_bubble_readings_url(bubbles(:logo).bucket, bubbles(:logo)), as: :turbo_stream end diff --git a/test/fixtures/events.yml b/test/fixtures/events.yml index f1232faa6..80f2b5ab1 100644 --- a/test/fixtures/events.yml +++ b/test/fixtures/events.yml @@ -1,6 +1,6 @@ -logo_created: +logo_published: creator: david - action: created + action: published summary: logo_initial_activity created_at: <%= 1.week.ago %> @@ -48,9 +48,9 @@ logo_boost_jz2: summary: logo_third_activity created_at: <%= 1.hour.ago %> -layout_created: +layout_published: creator: david - action: created + action: published summary: layout_initial_activity created_at: <%= 1.week.ago %> @@ -68,15 +68,15 @@ layout_assignment_jz: particulars: <%= { assignee_ids: [ ActiveRecord::FixtureSet.identify(:jz) ] }.to_json %> created_at: <%= 1.hour.ago %> -text_created: +text_published: creator: kevin - action: created + action: published summary: text_initial_activity created_at: <%= 1.week.ago %> -shipping_created: +shipping_published: creator: kevin - action: created + action: published summary: shipping_initial_activity created_at: <%= 1.week.ago %> diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index c73954558..89a8256ee 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -1,6 +1,6 @@ -logo_created_kevin: +logo_published_kevin: user: kevin - event: logo_created + event: logo_published bubble: logo resource: logo (Bubble) read: false diff --git a/test/models/notifier/created_test.rb b/test/models/notifier/published_test.rb similarity index 60% rename from test/models/notifier/created_test.rb rename to test/models/notifier/published_test.rb index 0c7aaa546..79c47d1f7 100644 --- a/test/models/notifier/created_test.rb +++ b/test/models/notifier/published_test.rb @@ -1,14 +1,14 @@ require "test_helper" -class Notifier::CreatedTest < ActiveSupport::TestCase +class Notifier::PublishedTest < ActiveSupport::TestCase test "creates a notification for each recipient" do - notifications = Notifier.for(events(:logo_created)).generate + notifications = Notifier.for(events(:logo_published)).generate assert_equal users(:kevin, :jz).sort, notifications.map(&:user).sort end test "links to the bubble" do - Notifier.for(events(:logo_created)).generate + Notifier.for(events(:logo_published)).generate assert_equal bubbles(:logo), Notification.last.resource end diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb index 62baf0c2d..cb85bf551 100644 --- a/test/models/notifier_test.rb +++ b/test/models/notifier_test.rb @@ -2,7 +2,7 @@ require "test_helper" class NotifierTest < ActiveSupport::TestCase test "for returns the matching notifier class for the event" do - assert_kind_of Notifier::Created, Notifier.for(events(:logo_created)) + assert_kind_of Notifier::Published, Notifier.for(events(:logo_published)) end test "for does not raise an error when the event is not notifiable" do From 731f93b55a052a6857a0150c82062d4d9fdce7c8 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 17:07:00 +0000 Subject: [PATCH 3/5] Add test --- .../bubbles/publishes_controller_test.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 test/controllers/bubbles/publishes_controller_test.rb diff --git a/test/controllers/bubbles/publishes_controller_test.rb b/test/controllers/bubbles/publishes_controller_test.rb new file mode 100644 index 000000000..68b48fa7a --- /dev/null +++ b/test/controllers/bubbles/publishes_controller_test.rb @@ -0,0 +1,18 @@ +require "test_helper" + +class Bubbles::PublishesControllerTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :kevin + end + + test "create" do + bubble = bubbles(:logo) + bubble.drafted! + + assert_changes -> { bubble.reload.published? }, from: false, to: true do + post bucket_bubble_publish_url(bubble.bucket, bubble) + end + + assert_redirected_to bubble + end +end From 056d853cc87c686df0cb4e0f0bb125aa13a1c796 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Wed, 15 Jan 2025 11:42:35 +0000 Subject: [PATCH 4/5] Don't notify while bubbles are still drafted --- app/helpers/notifications_helper.rb | 11 ++++++++++- app/models/notifier.rb | 10 ++++++++-- test/models/notifier_test.rb | 14 ++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 62474229b..96e17ff43 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -14,8 +14,8 @@ module NotificationsHelper case notification.event.action when "assigned" then "#{name} assigned to you" - when "published" then "Added by #{name}" when "popped" then "Popped by by #{name}" + when "published" then notification_bubble_created_message(notification) else name end end @@ -24,4 +24,13 @@ module NotificationsHelper link_to notification.resource, id: dom_id(notification), class: "notification border-radius", data: { turbo_frame: "_top" }, & end + + private + def notification_bubble_created_message(notification) + if notification.bubble.assigned_to?(notification.user) + "#{notification.creator.name} assigned to you" + else + "Added by #{notification.creator.name}" + end + end end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index cb64ec77c..86c07b54d 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -10,8 +10,10 @@ class Notifier end def generate - recipients.map do |recipient| - Notification.create! user: recipient, event: event, bubble: bubble, resource: resource + if should_notify? + recipients.map do |recipient| + Notification.create! user: recipient, event: event, bubble: bubble, resource: resource + end end end @@ -20,6 +22,10 @@ class Notifier @event = event end + def should_notify? + bubble.published? + end + def recipients bubble.bucket.users.without(creator) end diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb index cb85bf551..36cbfca5d 100644 --- a/test/models/notifier_test.rb +++ b/test/models/notifier_test.rb @@ -12,4 +12,18 @@ class NotifierTest < ActiveSupport::TestCase end end end + + test "generate creates notifications for the event recipients" do + assert_difference -> { Notification.count }, +2 do + Notifier.for(events(:logo_published)).generate + end + end + + test "generate does not create notifications if the bubble is not published" do + bubbles(:logo).drafted! + + assert_no_difference -> { Notification.count } do + Notifier.for(events(:logo_published)).generate + end + end end From dbb3fd3b9ca87a3e73d2f61412e5836a6bf8190c Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Wed, 15 Jan 2025 12:03:23 +0000 Subject: [PATCH 5/5] Simplify --- app/helpers/notifications_helper.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 96e17ff43..ddc142efb 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -12,10 +12,10 @@ module NotificationsHelper def notification_body(notification) name = notification.creator.name - case notification.event.action + case notification_event_action(notification) when "assigned" then "#{name} assigned to you" when "popped" then "Popped by by #{name}" - when "published" then notification_bubble_created_message(notification) + when "published" then "Added by #{name}" else name end end @@ -26,11 +26,15 @@ module NotificationsHelper end private - def notification_bubble_created_message(notification) - if notification.bubble.assigned_to?(notification.user) - "#{notification.creator.name} assigned to you" + def notification_event_action(notification) + if notification_is_for_initial_assignement?(notification) + "assigned" else - "Added by #{notification.creator.name}" + notification.event.action end end + + def notification_is_for_initial_assignement?(notification) + notification.event.action == "published" && notification.bubble.assigned_to?(notification.user) + end end