rails icon indicating copy to clipboard operation
rails copied to clipboard

[Bug] [ActiveSupport] "rails destroy model Adress" [sic] does not check first if the model exists

Open rossme opened this issue 1 year ago • 4 comments

Steps to reproduce

When I run the script rails destroy model Adress [sic] in the CLI, it prints that the model and associated files were removed, even though no such files existed. This behavior could cause confusion when a user makes a typo, as it may be misleading. While there is a robust check_class_collision callback when generating objects, a similar approach could be used when reverting the generate command.

Perhaps the rails destroy ... command is designed intentionally to assume the developer's responsibility and avoid complex checks, but I wanted to raise it as an issue nonetheless.

my-code/my-repo (main*) $ rails destroy model Adress      
      invoke  active_record
      remove    db/migrate/20240910091915_create_adresses.rb
      remove    app/models/adress.rb
      invoke    rspec
      remove      spec/models/adress_spec.rb
      invoke      factory_bot
      remove        spec/factories/adresses.rb
my-code/my-repo (main*) $ 

Expected behavior

If there is a typo in the model name, I should be alerted that no files were removed with that name, or an error should be returned, such as "Did you mean model name Address?"

Actual behavior

When I run the script rails destroy model Adress [sic] in the CLI, it prints that the model and associated files were removed, even though no files existed.

System configuration

Rails version: 7.2.1 Ruby version: 3.1.2p20

rossme avatar Sep 10 '24 09:09 rossme

This seems to be addressed in #53024

MatElGran avatar Sep 26 '24 09:09 MatElGran

Thanks @MatElGran. Out of interest, in future would it be better for me just to create a PR instead of first raising the issue? I was hoping by raising the issue last month that I might be asked to create the PR with the fix if it was needed.

rossme avatar Sep 26 '24 10:09 rossme

@rossme Yeah if you want to just open a PR first next time, that is preferred. Using issues to ask "is this a bug?" is fine but if you're already planning to try and fix it go for it!

zzak avatar Oct 03 '24 04:10 zzak

Since this has been worked on here, https://github.com/rails/rails/pull/53024 time to close this or at least say in this description that a merge of https://github.com/rails/rails/pull/53024 will solve this possibly linking them and letting github close this automatically once https://github.com/rails/rails/pull/53024 is merge

rasheedmhd avatar Oct 14 '24 20:10 rasheedmhd