bullet-train-co / bullet_train

The Open Source Ruby on Rails SaaS Template
MIT License
1.68k stars 269 forks source link

Adding multiple documents breaks things #1721

Closed awortham closed 2 weeks ago

awortham commented 1 month ago

I followed the documentation for adding a new field in order to upload documents to a model. https://bullettrain.co/docs/field-partials/file-field#example

Specifically this part: CleanShot 2024-10-23 at 16 00 49@2x

The only part I changed is to put it on a Staff model instead of a Post model. rails generate super_scaffold:field Staff documents:file_field{multiple}

This works initially:

Yay!

But then I re-open the form (edit action) and try to update again. CleanShot 2024-10-23 at 16 02 39@2x

Was I supposed to add another column? If so - what type of column should I add?

Expected behavior: I would expect that when following step by step in the documentation that I should be able to save the object.

What really happened: It blew up looking for a column I wasn't aware of :/

Can anyone fill me in about this documents_removal field? What kind of field is expected to be there? Did I miss a step somewhere?

jagthedrummer commented 4 weeks ago

@awortham I just tested this and it's working like I'd expect. (Though we do need to clean up the docs a little to remove some unnecessary bits.)

I did this:

rails generate super_scaffold Project Team name:text_field
rails db:migrate
rails generate super_scaffold:field Project documents:file_field{multiple}
rails db:migrate

That left me with a Project model that looks like this:

class Project < ApplicationRecord
  # 🚅 add concerns above.

  attr_accessor :documents_removal
  # 🚅 add attribute accessors above.

  belongs_to :team
  # 🚅 add belongs_to associations above.

  has_many_attached :documents
  # 🚅 add has_many associations above.

  # 🚅 add has_one associations above.

  # 🚅 add scopes above.

  validates :name, presence: true
  # 🚅 add validations above.

  after_validation :remove_documents, if: :documents_removal?
  # 🚅 add callbacks above.

  # 🚅 add delegations above.

  def documents_removal?
    documents_removal&.any?
  end

  def remove_documents
    documents_attachments.where(id: documents_removal).map(&:purge)
  end

  def documents=(attachables)
    attachables = Array(attachables).compact_blank

    if attachables.any?
      attachment_changes["documents"] =
        ActiveStorage::Attached::Changes::CreateMany.new("documents", self, documents.blobs + attachables)
    end
  end

  # 🚅 add methods above.
end

The scaffolding command added a line for attr_accessor :documents_removal as well as a few methods related to setting and removing documents.

Can you give this another try and see if it does the same thing for you? If it doesn't seem to work, can you check the output of the super scaffolding commands to see if it throws some kind of error about not being able to make the changes to your model?

awortham commented 2 weeks ago

Ok - I think what happened is that I had removed some of those commented lines. I didn't realize that bullettrain looks for those to know where to place them. So it seems to silently not add them if it doesn't find what it's looking for.

I should have left those in place.

jagthedrummer commented 2 weeks ago

Thanks for following up, @awortham! Those comments are definitely important. It looks like we should handle this situation better. I'd expect that we should throw an error if we can't find the target comments, but we currently just fail silently. I've opened #1753 to track that particular issue, and I'm going to close this one.