sorcery icon indicating copy to clipboard operation
sorcery copied to clipboard

Fix Thor warning due to missing :after option

Open Inkybro opened this issue 4 years ago • 13 comments

Hey, I was trying to mess around a little with Sorcery tonight on Rails 6.0.3.2, Ruby 2.7.1p83, and I kept noticing some odd warning message about a flag not being set when I tried to use the generators. I am pretty sure it was actually harmless, and my "fixes" here may ultimately not be necessary or desirable, but I was bored and wanted to see if I could get rid of the warning.

I tried with 0.15.0, as well as the master branch, and still get the warning: File unchanged! The supplied flag value not found! app/models/user.rb (see more below)

rails g sorcery:install
Running via Spring preloader in process 2257
      create  config/initializers/sorcery.rb
    generate  model User --skip-migrationrails g sorcery:install
Running via Spring preloader in process 2257
      create  config/initializers/sorcery.rb
    generate  model User --skip-migration
       rails  generate model User --skip-migration 
Running via Spring preloader in process 2318
      invoke  active_record
      create    app/models/user.rb
      invoke    test_unit
      create      test/models/user_test.rb
      create      test/fixtures/users.yml
      insert  app/models/user.rb
File unchanged! The supplied flag value not found!  app/models/user.rb
      create  db/migrate/20200722041629_sorcery_core.rb
       rails  generate model User --skip-migration 
Running via Spring preloader in process 2318
      invoke  active_record
      create    app/models/user.rb
      invoke    test_unit
      create      test/models/user_test.rb
      create      test/fixtures/users.yml
      insert  app/models/user.rb
File unchanged! The supplied flag value not found!  app/models/user.rb
      create  db/migrate/20200722041629_sorcery_core.rb

It is worth mentioning that the line still did get added to the model, despite the misleading warning. I ended up tracing the warning message back to Thor, and it looks like it's expecting an :after option to let it know where to insert the content.

I also accidentally entered my model name without the --model flag while testing the generator a few times and it was slightly annoying, so I also added the list of available submodules into the generator with a little validation and guidance for users. Other than that I just did a little re-arranging/re-factoring.

I really wasn't sure how one would go about writing specs for a generator. If anyone would like to point me in the right direction, I'd be willing to give it a shot. It looks like none of this code was really tested in the first place, unless I just totally missed it, and looks like the code hadn't been touched too much in a while. I did run the specs before I opened this PR, though.

Anyway, I was just bored. Feel free to throw this away if it's useless or way off base or anything.

Inkybro avatar Jul 22 '20 04:07 Inkybro

It looks like none of this code was really tested in the first place, unless I just totally missed it, and looks like the code hadn't been touched too much in a while.

I don't think you missed any specs, for the most part there haven't been any. Adding tests to all functionality is a major part of the Sorcery v1 rework.

I really wasn't sure how one would go about writing specs for a generator. If anyone would like to point me in the right direction, I'd be willing to give it a shot.

I'm not really sure either yet. Would you be interested in doing some research to see if any other libraries have specs for their generators? Rails might be a good candidate for that. I'm still in the process of grokking the crypto and config, so it'll be a while before I get to reworking the generators for v1.

joshbuker avatar Jul 22 '20 05:07 joshbuker

I will try and find some time to do that :) Thanks for the feedback!

Inkybro avatar Jul 22 '20 16:07 Inkybro

At an absolute stand-still for now. Any help is welcome.

Inkybro avatar Jul 27 '20 23:07 Inkybro

My fellow wizards.. Please forgive me. I got so close.

Ultimately I am too frustrated. I have to chill for a minute.

If the solution is simple here, please do point it out.

Inkybro avatar Jul 27 '20 23:07 Inkybro

It's still going to be a bit before I'm to the generators unfortunately. I recently finished the Rails bootstrapping part (god that was a hurdle), but there's still the config refactor which will be pretty hefty, as well as some other less major refactoring.

@Inkybro how about taking a break from this for now, and once I'm to the generators I can reach out to you and we can look at this together?

joshbuker avatar Jul 27 '20 23:07 joshbuker

@athix Even better, why don't you let me help you with something else? I'm happy to revisit this -- I just know when to cut my losses.

Inkybro avatar Jul 27 '20 23:07 Inkybro

@Inkybro Something that would be helpful is consolidating all the open issues and PRs, along with the old stuff from #32 into essentially one big wishlist of changes. It may save some time if those considerations are made up-front rather than after the refactor is complete, and as it is right now I'm knee deep in trying to grok the existing code.

joshbuker avatar Jul 27 '20 23:07 joshbuker

got u, bb

Inkybro avatar Jul 27 '20 23:07 Inkybro

Somehow I accidentally "liked" my own thing?

Ok, no problem. I will do that but tonight I will relax because i worked all week!!

Inkybro avatar Jul 27 '20 23:07 Inkybro

Sounds good, thanks @Inkybro! Hope you have a relaxing afternoon. :+1:

joshbuker avatar Jul 27 '20 23:07 joshbuker

get some rest my friend

Inkybro avatar Jul 27 '20 23:07 Inkybro

@athix I finally got some very basic generator specs working.

Would love to hear your feedback, let me know what you think.

Going to try and find some time next week to work on the other things we discussed earlier in this thread.

Inkybro avatar Aug 07 '20 14:08 Inkybro

I'm going to have to do some heavy lifting generator-wise for v1 regardless, so I'll just focus on adding generator tests and getting it all fixed up in v1 instead of trying to muddle through the v0 stuff.

joshbuker avatar May 28 '21 20:05 joshbuker