From e1e3c0e05fe71b05c1be0b2b4ab18b4828aad38a Mon Sep 17 00:00:00 2001 From: Stephen Nelson Date: Wed, 20 Sep 2023 13:46:27 +0930 Subject: [PATCH] Add orderable tables First version adds support for drag-and-drop. Styling needs more work. --- Gemfile.lock | 4 +- .../koi/components/_index-table.scss | 11 +- .../koi/components/index-table/_ordinal.scss | 21 +++ app/components/koi/ordinal_table_component.rb | 27 ++++ .../active_record/active_record_generator.rb | 17 +- lib/generators/koi/admin/admin_generator.rb | 1 + lib/koi/version.rb | 2 +- lib/tasks/dummy.thor | 7 + spec/support/capybara.rb | 13 ++ .../controllers/admin/banners_controller.rb | 85 ++++++++++ .../views/admin/banners/_banner.html+row.erb | 4 + .../templates/spec/factories/banners.rb | 8 + .../requests/admin/banners_controller_spec.rb | 152 ++++++++++++++++++ spec/system/index/ordinal_spec.rb | 47 ++++++ 14 files changed, 387 insertions(+), 12 deletions(-) create mode 100644 app/assets/stylesheets/koi/components/index-table/_ordinal.scss create mode 100644 app/components/koi/ordinal_table_component.rb create mode 100644 spec/support/templates/app/controllers/admin/banners_controller.rb create mode 100644 spec/support/templates/app/views/admin/banners/_banner.html+row.erb create mode 100644 spec/support/templates/spec/factories/banners.rb create mode 100644 spec/support/templates/spec/requests/admin/banners_controller_spec.rb create mode 100644 spec/system/index/ordinal_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index b9cab1e77..dbc78d056 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - katalyst-koi (4.0.3) + katalyst-koi (4.1.0) bcrypt importmap-rails katalyst-content @@ -184,7 +184,7 @@ GEM redlock (>= 1.2) katalyst-kpop (2.0.9) katalyst-navigation (1.4.1) - katalyst-tables (2.2.1) + katalyst-tables (2.2.2) html-attributes-utils view_component language_server-protocol (3.17.0.3) diff --git a/app/assets/stylesheets/koi/components/_index-table.scss b/app/assets/stylesheets/koi/components/_index-table.scss index 84cddebcf..d31a01dfc 100644 --- a/app/assets/stylesheets/koi/components/_index-table.scss +++ b/app/assets/stylesheets/koi/components/_index-table.scss @@ -1,3 +1,5 @@ +@use "index-table/ordinal" as *; + @mixin sort-icon { display: inline-block; content: " "; @@ -54,13 +56,10 @@ $row-height: 48px !default; text-decoration: none; } - > img { - width: 6rem; - padding: 1rem 0; - } - + > img, > a > img { - padding-top: 1rem; + max-height: 3rem; + padding: 0; } } diff --git a/app/assets/stylesheets/koi/components/index-table/_ordinal.scss b/app/assets/stylesheets/koi/components/index-table/_ordinal.scss new file mode 100644 index 000000000..59acf0bfb --- /dev/null +++ b/app/assets/stylesheets/koi/components/index-table/_ordinal.scss @@ -0,0 +1,21 @@ +$width: 2rem !default; + +.index-table { + th.ordinal { + width: $width; + padding-left: 0; + a { + width: $width; + height: 3rem; + } + a::after { + right: 0; + } + } + + td.ordinal { + width: $width; + padding-left: 0; + cursor: grab; + } +} diff --git a/app/components/koi/ordinal_table_component.rb b/app/components/koi/ordinal_table_component.rb new file mode 100644 index 000000000..e8c3246ce --- /dev/null +++ b/app/components/koi/ordinal_table_component.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Koi + class OrdinalTableComponent < Katalyst::Turbo::TableComponent + include Katalyst::Tables::Orderable + + def initialize(collection:, + id: "index-table", + url: [:order, :admin, collection], + scope: "order[#{collection.model_name.plural}]", + **options) + super(collection:, id:, class: "index-table", caption: true, **options) + + @url = url + @scope = scope + end + + def before_render + with_orderable(url: @url, scope: @scope) unless orderable? + end + + def call + concat(render_parent) + concat(orderable) + end + end +end diff --git a/lib/generators/koi/active_record/active_record_generator.rb b/lib/generators/koi/active_record/active_record_generator.rb index a1f427541..3590d3d9a 100644 --- a/lib/generators/koi/active_record/active_record_generator.rb +++ b/lib/generators/koi/active_record/active_record_generator.rb @@ -10,6 +10,16 @@ def admin_search "PgSearch::Model".safe_constantize ? pg_search : sql_search end + def ordinal_scope + return unless attributes.any? { |attr| attr.name == "ordinal" } + + insert_into_file "app/models/#{file_name}.rb", before: /end\Z/ do + <<~RUBY + default_scope -> { order(ordinal: :asc) } + RUBY + end + end + private def pg_search @@ -19,7 +29,7 @@ def pg_search RUBY end - insert_into_file "app/models/#{file_name}.rb", before: "end\n" do + insert_into_file "app/models/#{file_name}.rb", before: /end\Z/ do <<~RUBY pg_search_scope :admin_search, against: %i[#{search_fields.join(' ')}], using: { tsearch: { prefix: true } } RUBY @@ -27,10 +37,11 @@ def pg_search end def sql_search - insert_into_file "app/models/#{file_name}.rb", before: "end\n" do + insert_into_file "app/models/#{file_name}.rb", before: /end\Z/ do + clause = search_fields.map { |f| "#{f} LIKE :query" }.join(" OR ") <<~RUBY scope :admin_search, ->(query) do - where("#{search_fields.map { |f| "#{f} LIKE :query" }.join(' OR ')}", query: "%\#{query}%") + where("#{clause}", query: "%\#{query}%") end RUBY end diff --git a/lib/generators/koi/admin/admin_generator.rb b/lib/generators/koi/admin/admin_generator.rb index 53f20a884..871b210b9 100644 --- a/lib/generators/koi/admin/admin_generator.rb +++ b/lib/generators/koi/admin/admin_generator.rb @@ -16,5 +16,6 @@ class AdminGenerator < Rails::Generators::ScaffoldGenerator hook_for :admin_controller, in: :koi, as: :admin, type: :boolean, default: true Rails::Generators::ModelGenerator.hook_for :admin_search, type: :boolean, default: true + Rails::Generators::ModelGenerator.hook_for :ordinal_scope, type: :boolean, default: true end end diff --git a/lib/koi/version.rb b/lib/koi/version.rb index aa9c9aeea..abf0e923d 100644 --- a/lib/koi/version.rb +++ b/lib/koi/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Koi - VERSION = "4.0.3" + VERSION = "4.1.0" end diff --git a/lib/tasks/dummy.thor b/lib/tasks/dummy.thor index 35274f74d..4217c68ab 100644 --- a/lib/tasks/dummy.thor +++ b/lib/tasks/dummy.thor @@ -92,11 +92,18 @@ class Dummy < Thor inside("spec/dummy") do run <<~SH rails g koi:admin Post name:string title:string content:rich_text active:boolean published_on:date + rails g koi:admin Banner name:string image:attachment ordinal:integer SH run "rails db:migrate" end + gsub_file("config/routes/admin.rb", "resources :banners\n", <<~RUBY) + resources :banners do + patch :order, on: :collection + end + RUBY + Dir.glob(File.join(self.class.source_root, "**/*")).each do |file| copy_file(file[(self.class.source_root.size + 1)..], force: true) if File.file?(file) end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index de9d64252..9c04609df 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -2,6 +2,19 @@ require "capybara/rspec" +module WaitForTurbo + def wait_for_form_submission + Timeout.timeout(Capybara.default_max_wait_time) do + loop until form_submission_complete? + end + end + + def form_submission_complete? + page.evaluate_script("Turbo.navigator.formSubmission.state === 5") + end +end + RSpec.configure do |config| config.include Capybara::RSpecMatchers, type: :request + config.include WaitForTurbo, type: :system end diff --git a/spec/support/templates/app/controllers/admin/banners_controller.rb b/spec/support/templates/app/controllers/admin/banners_controller.rb new file mode 100644 index 000000000..d108006cb --- /dev/null +++ b/spec/support/templates/app/controllers/admin/banners_controller.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Admin + class BannersController < ApplicationController + before_action :set_banner, only: %i[show edit update destroy] + + def index + collection = Collection.new.with_params(params).apply(::Banner.strict_loading) + table = Koi::OrdinalTableComponent.new(collection:) + + respond_to do |format| + format.turbo_stream { render table } if self_referred? + format.html { render :index, locals: { table:, collection: } } + end + end + + def show + render locals: { banner: @banner } + end + + def new + render locals: { banner: ::Banner.new } + end + + def edit + render locals: { banner: @banner } + end + + def create + @banner = ::Banner.new(banner_params) + + if @banner.save + redirect_to [:admin, @banner] + else + render :new, locals: { banner: @banner }, status: :unprocessable_entity + end + end + + def update + if @banner.update(banner_params) + redirect_to action: :show + else + render :edit, locals: { banner: @banner }, status: :unprocessable_entity + end + end + + def destroy + @banner.destroy + + redirect_to action: :index + end + + def order + order_params[:banners].each do |id, attrs| + Banner.find(id).update(attrs) + end + + redirect_back(fallback_location: admin_banners_path, status: :see_other) + end + + private + + # Only allow a list of trusted parameters through. + def banner_params + params.require(:banner).permit(:name, :image, :ordinal) + end + + def order_params + params.require(:order).permit(banners: [:ordinal]) + end + + # Use callbacks to share common setup or constraints between actions. + def set_banner + @banner = ::Banner.find(params[:id]) + end + + class Collection < Katalyst::Tables::Collection::Base + attribute :search, :string + + def filter + self.items = items.admin_search(search) if search.present? + end + end + end +end diff --git a/spec/support/templates/app/views/admin/banners/_banner.html+row.erb b/spec/support/templates/app/views/admin/banners/_banner.html+row.erb new file mode 100644 index 000000000..e70c53b25 --- /dev/null +++ b/spec/support/templates/app/views/admin/banners/_banner.html+row.erb @@ -0,0 +1,4 @@ +<% row.ordinal %> +<% row.cell :name do |cell| %> + <%= link_to cell.value, url_for([:admin, banner]) %> +<% end %> diff --git a/spec/support/templates/spec/factories/banners.rb b/spec/support/templates/spec/factories/banners.rb new file mode 100644 index 000000000..688cc4231 --- /dev/null +++ b/spec/support/templates/spec/factories/banners.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :banner do + name { Faker::Kpop.solo } + sequence(:ordinal) + end +end diff --git a/spec/support/templates/spec/requests/admin/banners_controller_spec.rb b/spec/support/templates/spec/requests/admin/banners_controller_spec.rb new file mode 100644 index 000000000..47c579443 --- /dev/null +++ b/spec/support/templates/spec/requests/admin/banners_controller_spec.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Admin::BannersController do + let(:model) { create(:banner) } + + include_context "with admin session" + + describe "GET /admin/banners" do + let(:action) { get polymorphic_path([:admin, Banner]) } + + it_behaves_like "requires admin" + + it "renders successfully" do + action + expect(response).to have_http_status(:success) + end + + context "with a large collection" do + before { create_list(:banner, 25) } + + it "does not paginate the collection" do + action + expect(response.body).to have_selector("tbody tr", count: 25) + end + end + + context "with ordinals" do + let(:action) { get polymorphic_path([:admin, Banner]) } + + before do + create(:banner, name: "first", ordinal: 1) + create(:banner, name: "second", ordinal: 0) + end + + it "finds first in second place" do + action + expect(response.body).to have_selector("tbody tr + tr td", text: "first") + end + end + + context "with search parameter" do + let(:action) { get polymorphic_path([:admin, Banner], search: "first") } + + before do + create(:banner, name: "first") + create(:banner, name: "second") + end + + it "finds the needle" do + action + expect(response.body).to have_selector("table td", text: "first") + end + + it "removes the chaff" do + action + expect(response.body).not_to have_selector("table td", text: "second") + end + end + end + + describe "GET /admin/banners/new" do + let(:action) { get new_polymorphic_path([:admin, Banner]) } + + it_behaves_like "requires admin" + + it "renders successfully" do + action + expect(response).to have_http_status(:success) + end + end + + describe "POST /admin/banners" do + let(:action) { post polymorphic_path([:admin, Banner]), params: { banner: params } } + let(:params) { attributes_for(:banner) } + + it_behaves_like "requires admin" + + it "renders successfully" do + action + expect(response).to redirect_to([:admin, assigns(:banner)]) + end + + it "creates a banner" do + expect { action }.to change(Banner, :count).by(1) + end + end + + describe "GET /admin/banners/:id" do + let(:action) { get polymorphic_path([:admin, model]) } + + it_behaves_like "requires admin" + + it "renders successfully" do + action + expect(response).to have_http_status(:success) + end + end + + describe "GET /admin/banners/:id/edit" do + let(:action) { get edit_polymorphic_path([:admin, model]) } + + it_behaves_like "requires admin" + + it "renders successfully" do + action + expect(response).to have_http_status(:success) + end + end + + describe "PATCH /admin/banners/:id" do + let(:action) { patch polymorphic_path([:admin, model]), params: { banner: params } } + let(:params) { attributes_for(:banner) } + + it_behaves_like "requires admin" + + it "renders successfully" do + action + expect(response).to redirect_to(polymorphic_path([:admin, model])) + end + end + + describe "PATCH /admin/banners/order" do + let(:action) { patch polymorphic_path([:order, :admin, Banner]), params: { order: params } } + let(:params) { { banners: { first.id => { ordinal: 1 }, second.id => { ordinal: 0 } } } } + let(:first) { create(:banner, name: "first", ordinal: 0) } + let(:second) { create(:banner, name: "second", ordinal: 1) } + + it_behaves_like "requires admin" + + it "redirects back" do + action + expect(response).to redirect_to(polymorphic_path([:admin, Banner])) + end + + it "updates ordinals" do + expect { action }.to change(Banner, :first).to have_attributes(name: "second") + end + end + + describe "DELETE /admin/banners/:id" do + let(:action) { delete polymorphic_path([:admin, model]) } + + it_behaves_like "requires admin" + + it "renders successfully" do + action + expect(response).to redirect_to(polymorphic_path([:admin, Banner])) + end + end +end diff --git a/spec/system/index/ordinal_spec.rb b/spec/system/index/ordinal_spec.rb new file mode 100644 index 000000000..883d9d8a7 --- /dev/null +++ b/spec/system/index/ordinal_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "index/ordinal" do + let(:admin) { create(:admin) } + + before do + visit "/admin" + + fill_in "Email", with: admin.email + fill_in "Password", with: admin.password + click_button "Log in" + + %i[first second third].each_with_index do |n, i| + create(:banner, name: n, ordinal: i) + end + end + + it "supports mouse re-ordering", pending: "https://bugs.chromium.org/p/chromium/issues/detail?id=850071" do + visit "/admin/banners" + + within(".index-table tbody") do + first = page.find("tr:first-child td.ordinal") + last = page.find("tr:last-child td.ordinal") + first.drag_to(last) + end + + expect(page).to have_css("tr:last-child td", text: "first") + + wait_for_form_submission + + expect(Banner.all).to contain_exactly( + have_attributes(name: "second", ordinal: 0), + have_attributes(name: "third", ordinal: 1), + have_attributes(name: "first", ordinal: 2), + ) + end + + it "supports keyboard re-ordering", pending: "unimplemented" do + visit "/admin/banners" + + find(".index-table").send_keys("ff", "k", [:shift, "k"]) + + expect(page).to have_css("tr:last-child td", text: "second") + end +end