Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting a record with validation errors within has_many view hangs on reloading view #1918

Closed
5 of 10 tasks
dhnaranjo opened this issue Aug 17, 2023 · 6 comments · Fixed by #2016
Closed
5 of 10 tasks
Assignees
Labels
Bug Something isn't working

Comments

@dhnaranjo
Copy link
Contributor

dhnaranjo commented Aug 17, 2023

Describe the bug

When an attempt is made to destroy a resource within a Parent resources field :children, as: :has_many, if the deletion fails because of a validation error the turbo frame fails to update, and is stuck on the loading view. The response is passed and looks like a proper turbo frame update to me, I couldn't identify the flaw.

Steps to Reproduce

Drop the following single-file app into a file called app.ru and run rackup app.ru. I found the issue in my own app, then verified it in a rails new --minimal, and finally knocked this thing together to simplify reproduction.

  • Navigate to /resources/parents/1
  • Click on the delete button next to Child
  • See that children has_many view hangs on loading
  • Reload page, click on the delete button next to Grandchildless Child
  • See that it behaves as expected
require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  gem 'rails', '~> 7.0.4'
  gem 'sqlite3'
  gem 'avo', '2.38.0'
  gem 'puma'
  gem 'propshaft'
end

require 'rails/all'

database = 'development.sqlite3'
ENV['DATABASE_URL'] = "sqlite3:#{database}"
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: database)
ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Schema.define do
  create_table :parents, if_not_exists: true do |t|
    t.string :name
  end
  create_table :children, if_not_exists: true do |t|
    t.references :parent, null: false, foreign_key: true
    t.string :name
  end
  create_table :grandchildren, if_not_exists: true do |t|
    t.references :child, null: false, foreign_key: true
    t.string :name
  end
end

class App < Rails::Application
  config.root = __dir__
  config.eager_load = true
  config.consider_all_requests_local = true
  config.secret_key_base = 'Aswertyuioasdfghjkqwertyuiqwerty'
  config.active_storage.service_configurations = { 'local' => { 'service' => 'Disk', 'root' => './storage' } }
  config.active_record.legacy_connection_handling = false


  routes.append do
    mount Avo::Engine, at: "/"
  end
end

App.initialize!

class Parent < ActiveRecord::Base
  has_many :children
end

class Child < ActiveRecord::Base
  belongs_to :parent
  has_many :grandchildren, dependent: :restrict_with_error
end

class Grandchild < ActiveRecord::Base
  belongs_to :child
end

class ParentResource < Avo::BaseResource
  field :name, as: :text
  field :children, as: :has_many
end

class ChildResource < Avo::BaseResource
  field :parent, as: :belongs_to
  field :name, as: :text
  field :grandchildren, as: :has_many
end

class GrandchildResource < Avo::BaseResource
  field :child, as: :belongs_to
  field :name, as: :text
end

class Avo::ParentsController < Avo::ResourcesController
end

class Avo::ChildrenController < Avo::ResourcesController
end

class Avo::GrandchildrenController < Avo::ResourcesController
end

Rails.application.reload_routes!

parent = Parent.find_or_create_by! name: 'Parent'
child = Child.find_or_create_by! name: 'Child', parent: parent
_grandchild = Grandchild.find_or_create_by! name: 'Grandchild', child: child
_grandchildless_child = Child.find_or_create_by! name: 'Grandchildless Child', parent: parent

run App

Expected behavior & Actual behavior

View should refresh and error flash message should appear.

Models and resource files

System configuration

Avo version: 2.38.0

Rails version: 7.0.4

Ruby version: 3.2.2

License type:

  • Community
  • Pro

Are you using Avo monkey patches, overriding views or view components?

  • Yes. If so, please post code samples.
  • No

Screenshots or screen recordings

image

Additional context

The single file is handy for IDing bugs, but It is necessary to run App.initialize! prior to defining controllers so that Avo::ResourcesController is available, then you gotta Rails.application.reload_routes! so (I believe) the Avo dynamic router can pick them up. Just a note for anyone who might find that useful.

Impact

  • High impact (It makes my app un-usable.)
  • Medium impact (I'm annoyed, but I'll live.)
  • Low impact (It's really a tiny thing that I could live with.)

Urgency

I haven't found a workaround but it isn't exactly a showstopper. It will require explanation to my internal users which is a bummer.

  • High urgency (I can't continue development without it.)
  • Medium urgency (I found a workaround, but I'd love to have it fixed.)
  • Low urgency (It can wait. I just wanted you to know about it.)
@dhnaranjo
Copy link
Contributor Author

dhnaranjo commented Aug 17, 2023

Also present in Avo v3.

Repro script
# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  gem 'rails', '~> 7.0.4'
  gem 'sqlite3'
  gem 'avo', '>= 3.0.1.beta5', source: 'https://packager.dev/avo-hq/'
  gem 'puma'
  gem 'propshaft'
end

require 'rails/all'

database = 'development.sqlite3'
ENV['DATABASE_URL'] = "sqlite3:#{database}"
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: database)
ActiveRecord::Base.logger = Logger.new($stdout)
ActiveRecord::Schema.define do
  create_table :parents, if_not_exists: true do |t|
    t.string :name
  end
  create_table :children, if_not_exists: true do |t|
    t.references :parent, null: false, foreign_key: true
    t.string :name
  end
  create_table :grandchildren, if_not_exists: true do |t|
    t.references :child, null: false, foreign_key: true
    t.string :name
  end
end

class App < Rails::Application
  config.root = __dir__
  config.eager_load = true
  config.consider_all_requests_local = true
  config.secret_key_base = 'Aswertyuioasdfghjkqwertyuiqwerty'
  config.active_storage.service_configurations = { 'local' => { 'service' => 'Disk', 'root' => './storage' } }
  config.active_record.legacy_connection_handling = false

  routes.append do
    mount Avo::Engine, at: '/'
  end
end

App.initialize!

class Parent < ActiveRecord::Base
  has_many :children
end

class Child < ActiveRecord::Base
  belongs_to :parent
  has_many :grandchildren, dependent: :restrict_with_error
end

class Grandchild < ActiveRecord::Base
  belongs_to :child
end

module Avo
  module Resources
    class Parent < Avo::BaseResource
      def fields
        field :name, as: :text
        field :children, as: :has_many
      end
    end
  end
end

module Avo
  module Resources
    class Child < Avo::BaseResource
      def fields
        field :parent, as: :belongs_to
        field :name, as: :text
        field :grandchildren, as: :has_many
      end
    end
  end
end

module Avo
  module Resources
    class Grandchild < Avo::BaseResource
      def fields
        field :child, as: :belongs_to
        field :name, as: :text
      end
    end
  end
end

module Avo
  class ParentsController < Avo::ResourcesController
  end
end

module Avo
  class ChildrenController < Avo::ResourcesController
  end
end

module Avo
  class GrandchildrenController < Avo::ResourcesController
  end
end

Rails.application.reload_routes!

parent = Parent.find_or_create_by! name: 'Parent'
child = Child.find_or_create_by! name: 'Child', parent: parent
_grandchild = Grandchild.find_or_create_by! name: 'Grandchild', child: child
_grandchildless_child = Child.find_or_create_by! name: 'Grandchildless Child', parent: parent

run App

# Navigate to /resources/parents/1
# Click on the delete button next to Child
# See that children has_many view hangs on loading
# Reload page, click on the delete button next to Grandchildless Child
# See that it behaves as expected

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2023

This issue has been marked as stale because there was no activity for the past 15 days.

@github-actions github-actions bot added the Stale label Sep 2, 2023
@dhnaranjo
Copy link
Contributor Author

Bump.

@github-actions github-actions bot removed the Stale label Sep 4, 2023
@github-actions
Copy link
Contributor

This issue has been marked as stale because there was no activity for the past 15 days.

@github-actions github-actions bot added the Stale label Sep 19, 2023
@adrianthedev adrianthedev added Bug Something isn't working and removed Stale labels Sep 19, 2023
@adrianthedev
Copy link
Collaborator

@Paul-Bob can you please have a look at this?

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Nov 9, 2023

@dhnaranjo thank you for the detailed report and the accurate reproduction steps. There is a PR fixing this issue by streaming the flash errors. It should be included on the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants