Merge pull request #548 from basecamp/change-collection

Change collection

app/assets/images/move.svg

@@ -0,0 +1 @@
+<svg viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><path d="m13.3 18.4c-.3.3-.9.2-1.2-.1-.1-.1-.2-.3-.2-.5v-3.3c-4.2 0-6.7 1.4-7.7 4.4 0 .3-.4.5-.7.5s-.7-.3-.7-.7c.2-6.1 3.2-9.6 9-9.9v-3.4c0-.5.4-.8.8-.8s.4 0 .5.2l7.4 6.1c.3.3.4.8.1 1.2h-.1c0 .1-7.2 6.3-7.2 6.3z"/></svg>

app/assets/stylesheets/card-perma.css

@@ -41,6 +41,17 @@
       max-inline-size: 32ch;
     }
 
+    .collection-picker__button {
+      --btn-border-size: 0;
+    
+      font-size: 0.5em;
+      margin-inline-start: 1ch;
+  
+      + .panel {
+        --panel-size: 18ch;
+      }
+    }
+
     @media (max-width: 800px) {
       --padding-inline: 3cqmin;
 

app/assets/stylesheets/cards.css

@@ -81,7 +81,12 @@
     color: var(--color-ink-inverted);
     font-weight: 800;
     padding: var(--block-space-half) var(--inline-space) var(--block-space-half) var(--inline-space-double);
-    text-transform: uppercase;
+
+    .cards--considering & {
+      background-color: var(--color-canvas);
+      color: color-mix(in srgb, var(--card-color) 40%, var(--color-ink));
+      padding: 0 0 0 var(--inline-space);
+    }
   }
 
   .card__id {
@@ -100,6 +105,12 @@
     display: inline-flex;
     margin-inline-start: var(--inline-space-half);
     padding-inline-start: calc(var(--inline-space) * 0.75);
+    text-transform: uppercase;
+
+    .cards--considering & {
+      border-inline-start: 1px solid color-mix(in srgb, var(--color-ink) 33%, var(--color-canvas));
+      color: color-mix(in srgb, var(--card-color) 40%, var(--color-ink));
+    }
   }
 
   .card__tags {

app/assets/stylesheets/icons.css

@@ -44,6 +44,7 @@
   .icon--menu { --svg: url("menu.svg "); }
   .icon--menu-dots-horizontal { --svg: url("menu-dots-horizontal.svg "); }
   .icon--minus { --svg: url("minus.svg "); }
+  .icon--move { --svg: url("move.svg "); }
   .icon--notification-bell-access-only { --svg: url("notification-bell-access-only.svg "); }
   .icon--notification-bell-everything { --svg: url("notification-bell-everything.svg "); }
   .icon--notification-bell-watching { --svg: url("notification-bell-watching.svg "); }

app/controllers/cards_controller.rb

@@ -28,7 +28,9 @@ class CardsController < ApplicationController
   def update
     @card.update! card_params
 
-    if @card.published?
+    if params[:card][:collection_id].present?
+      redirect_to collection_card_path(@card.collection, @card)
+    elsif @card.published?
       render_card_replacement
     else
       redirect_to @card
@@ -54,7 +56,7 @@ class CardsController < ApplicationController
     end
 
     def card_params
-      params.expect(card: [ :status, :title, :description, :image, tag_ids: [] ])
+      params.expect(card: [ :collection_id, :status, :title, :description, :image, tag_ids: [] ])
     end
 
     def render_card_replacement

app/helpers/events_helper.rb

@@ -96,6 +96,8 @@ module EventsHelper
       "#{event_creator_name(event)} removed the date on <span style='color: var(--card-color)'>#{ title }</span>"
     when "card_title_changed"
       "#{event_creator_name(event)} renamed <span style='color: var(--card-color)'>#{ title }</span> (was: '#{event.particulars.dig('particulars', 'old_title')})'".html_safe
+    when "card_collection_changed"
+      "#{event_creator_name(event)} moved <span style='color: var(--card-color)'>#{ title }</span> to '#{event.particulars.dig('particulars', 'new_collection')}'".html_safe
     end
   end
 
@@ -113,6 +115,8 @@ module EventsHelper
       "comment"
     when "card_title_changed"
       "rename"
+    when "card_collection_changed"
+      "move"
     else
       "person"
     end

app/models/card.rb

@@ -12,6 +12,7 @@ class Card < ApplicationRecord
   has_rich_text :description
 
   before_save :set_default_title, if: :published?
+  after_save :handle_collection_change, if: :saved_change_to_collection_id?
 
   scope :reverse_chronologically, -> { order created_at: :desc, id: :desc }
   scope :chronologically, -> { order created_at: :asc, id: :asc }
@@ -39,4 +40,21 @@ class Card < ApplicationRecord
     def set_default_title
       self.title = "Untitled" if title.blank?
     end
+
+    def handle_collection_change
+      transaction do
+        old_collection = Collection.find_by(id: collection_id_before_last_save)
+        if old_collection.present?
+          track_event "collection_changed", particulars: {
+            old_collection: old_collection.name,
+            new_collection: collection.name
+          }
+        end
+        grant_access_to_assignees unless collection.all_access?
+      end
+    end
+
+    def grant_access_to_assignees
+      collection.accesses.grant_to(assignees)
+    end
 end

app/models/card/eventable/system_commenter.rb

@@ -24,6 +24,8 @@ class Card::Eventable::SystemCommenter
         "Closed as “#{ card.closure.reason }” by #{ event.creator.name }"
       when "card_title_changed"
         "#{event.creator.name} changed the title from '#{event.particulars.dig('particulars', 'old_title')}' to '#{event.particulars.dig('particulars', 'new_title')}'."
+      when "card_collection_changed"
+      "#{event.creator.name} moved this from '#{event.particulars.dig('particulars', 'old_collection')}' to '#{event.particulars.dig('particulars', 'new_collection')}'."
       end
     end
 end

app/views/cards/_container.html.erb

@@ -10,12 +10,7 @@
   <div class="card-perma__bg">
     <%= card_article_tag card, class: "card" do %>
       <header class="card__header">
-        <span class="card__collection">
-          <span class="card__id"><%= card.id %></span>
-          <%= link_to card.collection.name, cards_path(collection_ids: [ card.collection ]),
-                class: "card__collection-name overflow-ellipsis" %>
-        </span>
-
+        <%= render "cards/display/perma/collection", card: card %>
         <%= render "cards/display/perma/tags", card: card %>
       </header>
 

app/views/cards/display/_preview.html.erb

@@ -3,11 +3,7 @@
     <div class="flex gap">
       <div class="flex flex-column flex-item-grow">
         <header class="card__header">
-          <span class="card__collection">
-            <span class="card__id"><%= card.id %></span>
-            <%= link_to card.collection.name, cards_path(collection_ids: [ card.collection ]),
-                  class: "card__collection-name overflow-ellipsis" %>
-          </span>
+          <%= render "cards/display/preview/collection", card: card %>
           <%= render "cards/display/preview/tags", card: card %>
         </header>
 

app/views/cards/display/common/_collection.html.erb

@@ -0,0 +1,6 @@
+<div class="card__collection flex align-start">
+  <span class="card__id"><%= card.id %></span>
+  <%= link_to card.collection.name, cards_path(collection_ids: [ card.collection ]),
+        class: "card__collection-name overflow-ellipsis" %>
+  <%= yield %>
+</div>

app/views/cards/display/perma/_collection.html.erb

@@ -0,0 +1,38 @@
+<%= render "cards/display/common/collection", card: card do %>
+  <div class="position-relative" data-controller="dialog" data-action="keydown.esc->dialog#close:stop click@document->dialog#closeOnClickOutside" <%= "hidden" if card.closed? %>>
+    <button class="collection-picker__button btn btn--reversed card__hide-on-index fill-transparent" data-action="click->dialog#open:stop">
+      <%= icon_tag "caret-down" %>
+      <span class="for-screen-reader">Choose a collection for this card</span>
+    </button>
+
+    <dialog class="popup panel flex-column align-start gap-half fill-white shadow" data-dialog-target="dialog">
+      <%= tag.div class: "max-width full-width", data: {
+          controller: "filter", 
+          dialog_target: "dialog",
+          filter_active_class: "filter--active", 
+          filter_selected_class: "selected"} do %>
+        <strong class="popup__title pad-inline-half txt-nowrap">Put this card in…</strong>
+
+        <% if Current.user.collections.count > 20 %>
+          <input placeholder="Find a collection…" class="input input--transparent txt-small margin-block-half" type="search" autocorrect="off" autocomplete="off" data-1p-ignore="true" data-action="input->filter#filter" autofocus>
+        <% end %>
+
+        <%= form_with(model: card, url: collection_card_path(card.collection, card), method: :patch, class: "full-width", data: { controller: "form" }) do |form| %>
+          <ul class="flex flex-column full-width popup__list margin-block-start-half margin-none-inline margin-none-block-end unpad max-width" data-filter-target="list">
+            <% Current.user.collections.alphabetically.each do |collection| %>
+              <li class="popup__group flex align-center full-width" data-value="<%= collection.name.downcase %>">
+                <label class="btn popup__item min-width flex-item-grow full-width unpad-inline">
+                  <%= form.radio_button :collection_id, collection.id, 
+                      class: "visually-hidden",
+                      data: { action: "change->form#submit" } %>
+                  <span class="overflow-ellipsis flex-item-grow"><%= collection.name %></span>
+                  <%= icon_tag "check", size: 18, class: "checked flex-item-no-shrink flex-item-justify-end", style: "--icon-size: 1em" if card.collection == collection %>
+                </label>
+              </li>
+            <% end %>
+          </ul>
+        <% end %>
+      <% end %>
+    </dialog>
+  </div>
+<% end %>

app/views/cards/display/preview/_collection.html.erb

@@ -0,0 +1 @@
+<%= render "cards/display/common/collection", card: card  %>

test/fixtures/collections.yml

@@ -4,3 +4,9 @@ writebook:
   all_access: true
   auto_close_period: <%= 30.days.to_i %>
   workflow: qa
+
+private:
+  name: Private collection
+  creator: kevin
+  all_access: false
+  auto_close_period: <%= 30.days.to_i %>

test/models/card_test.rb

@@ -128,4 +128,16 @@ class CardTest < ActiveSupport::TestCase
     card.publish
     assert_equal "Untitled", card.reload.title
   end
+
+  test "grants access to assignees when moved to a new collection" do
+    card = cards(:logo)
+    assignee = users(:david)
+    card.toggle_assignment(assignee)
+
+    collection = collections(:private)
+    assert_not_includes collection.users, assignee
+
+    card.update!(collection: collection)
+    assert_includes collection.users.reload, assignee
+  end
 end