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

Change flag column check to be opt-in #52

Open
modullar opened this issue Jul 28, 2016 · 6 comments
Open

Change flag column check to be opt-in #52

modullar opened this issue Jul 28, 2016 · 6 comments
Assignees
Milestone

Comments

@modullar
Copy link

modullar commented Jul 28, 2016

Let's say I have a flag in User model. When running rake db:migrate or rake db:setup for the first time.
You'll get this error and migration
Also rake db:seed after that would fail !!

Even running test using rspec cause sometimes the same result !

FlagShihTzu#has_flags: Table "users" doesn't exist.  Have all migrations been run?
FlagShihTzu says: Flag column roles appears to be missing!
@modullar modullar changed the title Running migration for the first time is causing an error Running migration for the first time or running test is causing an error Jul 28, 2016
@pboling
Copy link
Owner

pboling commented Aug 2, 2016

@mouazq
I know the readme is ridiculously long, but it is in there! https://github.com/pboling/flag_shih_tzu#skipping-flag-column-check

the flag column check can be turned off, even by a command line ENV param if you like:

has_flags 1 => :warpdrive,
          2 => :shields,
          :check_for_column => false # or ENV['FLAG_CHECK'] == "true" if you want

@pboling pboling closed this as completed Aug 2, 2016
@modullar
Copy link
Author

modullar commented Aug 4, 2016

hey pboling.

But why checking the database column should be enabled by default if it's going to lead to this behaviour ? and would turning off the checker notify you too if somone missed to add the flag field to the db table ?

Thanks for the reply.

@pboling
Copy link
Owner

pboling commented Aug 7, 2016

Perhaps it should be opt-in. It was added many years ago, and my understanding of best practice has evolved considerably since then. This gem needs an overhaul!

would turning off the checker notify you too if somone missed to add the flag field to the db table.

I think it will not warn you if turned off, but your project won't work, obviously, if the column is missing (aside from migrations). You can test it out if you like.

@pboling
Copy link
Owner

pboling commented Aug 7, 2016

I am going to re-open this as an issue to change the flag column check to opt-in.

@pboling pboling reopened this Aug 7, 2016
@pboling pboling changed the title Running migration for the first time or running test is causing an error Change flag column check to be opt-in Aug 7, 2016
@pboling pboling added this to the 0.4 milestone May 2, 2017
@pboling pboling self-assigned this May 2, 2017
@tobypinder
Copy link

Would it not be possible to maintain the functionality at least as a callback that occurs once ActiveRecord is being used? The main problem as I can see it is that this code executes in the class context, as models are being loaded and defined, well before usual initialisation steps. As such this breaks commands like rake assets:precompile where a database is not needed, and many other contexts as discussed above.

I get that flipping defaults etc is difficult (especially in SemVer) but Rails.application.config.after_initialize { ... } or similar callback hook blocks might be more appropriate for these checks without breaking existing compatibility?

@pboling
Copy link
Owner

pboling commented Oct 11, 2019

I am sure there is a way to get this check hooked in at an appropriate point during initialization. I just haven't had time to look into it. It can/should probably be done with a Railtie?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants