pboling / flag_shih_tzu Goto Github PK

We use flag_shih_tzu in a number of classes. One of those is a class that uses a tabled shared between two applications. We use "establish_connection" to connect that class with a table in a secondary database.

Since 0.3.0 #check_flag_column method uses ActiveRecord::Base.connection to look for the table - i.e. the "local" database. Previously is just used #connection (i.e. MyClass.connection) which correctly found the "remote" database.

As a result flag_shih_tzu cannot find the expected table and fails to defined the flag methods.

I think having instanсe method that will allow updating single flag in a table without callbacks and validation, similar to AR update_column.

Currently I use

  def update_flag(flag, value)
    self.class.update_all(self.class.set_flag_sql(flag.to_sym, FlagShihTzu::TRUE_VALUES.include?(value)), self.class.primary_key => id) == 1
  end

Probably I should call something to update the state of my model

https://github.com/rails/rails/blob/68307a1ae5fc9e454230629f5cbbbeaf79fd7cec/activerecord/lib/active_record/persistence.rb#L192

Hi I was using the gem today and thought it would be nice if possible to add a functionality like:

Record.flag_names and return an array of strings containing flag names. It would be really useful to iterate over the flag names without having to hard code the names.

Not sure if the feature exists and I haven't seen it but thanks in advance :D

I'm joining a non-Rails project that uses this gem. I just ran the tests and got a backtrace that included this:

...../gems/flag_shih_tzu-0.3.2/lib/flag_shih_tzu.rb:209:in `check_flag_column': 
private method `warn' called for nil:NilClass (NoMethodError)

That goes to a line like this (on master):
https://github.com/pboling/flag_shih_tzu/blob/master/lib/flag_shih_tzu.rb#L269

Turns out that you're trying to give me a helpful error, but I actually got a very confusing one. 😆

This line assumes that the Rails logger is present (right?).

An approach I've seen that gets around that is to have a configurable logger. Eg, when you want to log, call FlagShihTzu.logger.warn('whatever'). That would be a method like:

module FlagShihTzu
  def self.logger
    @logger ||= configuration.logger
  end
end

Then users could configure your log messages to point wherever they want in a configuration file. Eg, configuration.logger = Rails.logger in a Rails app, or configuration.logger = Logger.new(my_location), where my_location could be 'some_file.log' or STDOUT or '/dev/null'.

This is a trick @adamhunter showed me, which we used in Authority: https://github.com/nathanl/authority/blob/c4e55d89389904cda888cbc01a0864728cfde950/lib/authority.rb#L61

Just an idea.

I swapped out some enums I was using for some flags. Especially in tests where I had to set up specific models, I was changing code like this: model.update(enum_field: :enum_value) to model.update(flags: [:flag1, :flag2]) which causes a null value to get sent to the db.

Instead, I ended up doing something like model.update(flags: 3) with a magic number.

It looks like it might be possible to do something similar to enums (https://github.com/rails/rails/blob/a9dc45459abcd9437085f4dd0aa3c9d0e64e062f/activerecord/lib/active_record/enum.rb#L165). Is there any interest in that?

Or any other recommendations to avoid magic numbers when setting multiple flags at once (without having to have one line of code per flag)?

I'm running into an issue where if I try to compile production assets on my local machine this throws an error. I believe this is due to has_flags referencing the connect.

Have you run into this before? Is there a known work-around?

Thanks for the awesome gem!

Anyway, after using it in production for a few months I would like to suggest (with a PR afterwards) to change the default query mode from :in_list to :bit_operator. Even though performance-wise it seems like :in_list is best for MySQL (haven't measured it with Postgres), I would like to argue that :bit_operator is safer.

The reason is that the :in_list query only works if the number of flags is fixed.

# reference state
class User < ApplicationRecord
  has_flags 1 => :warpdrive,
            2 => :shields
end

User.warprive.to_sql
=> "SELECT users.* FROM users WHERE users.flags in (1,3)

However, consider the case where a new flag is added:

# new state
class User < ApplicationRecord
  has_flags 1 => :warpdrive,
            2 => :shields,
            3 => :premium
            :flag_query_mode => :bit_operator
end

And in the same deploy a migration sets the flag for some records:

# db/migrate/...
class AddNewFlagToUsersTable < ApplicationRecord
  def up
    execute <<~SQL
      UPDATE users
        SET flags = flags | 4
        WHERE created_at < '...'
    SQL
  end

  def down
    # noop
  end
end

During the deploy the following query done in the old version of the app becomes invalid:

User.warprive.to_sql
=> "SELECT users.* FROM users WHERE users.flags in (1,3)

However, if the default query mode would be :bit_operator instead of :in_list, this problem wouldn't happen:

User.warprive.to_sql
=> "SELECT users.* FROM users WHERE users.flags & 1 = 1

The :bit_operator works regardless of the number of flags, so there would be no issue during a deploy setting a new flag.

Anyway, I decided to open this issue today because in my company we have been bitten by this a couple of times, and now we try to remember to always add :flag_query_mode => :bit_operator when using has_flags. I wonder if other users have been bitten by that, too. I'm a fan of the principle of least surprise, and think that safety should come before performance. If someone needs more performance he/she can always add the option later.

from flag_shih_tzu.rb:105
# BJM: added test env check. self.columns bombs when !has_ar && Rails.env = 'test'
#if !has_ar || (has_ar && has_table)
if (Rails.env != 'test' && !has_ar) || (has_ar && has_table)

we just had strange errros until we figured out that the :column => :bits must be a string ('bits')

given a rails app which uses flag_shih_tzu.
given an uninitialized db.
when you run rake db:create
then flag_shih_tzu attempts to connect the db that has not yet been created.

Removing existing flags leads to an issue with the class methods. Here is a minimal example ...

Initial state ..

class Spaceship < ActiveRecord::Base
  include FlagShihTzu

  has_flags 
            1 => :warpdrive,
            2 => :shields
end

s = Spaceship.create!
s.warpdrive = true
s.shields = true
s.save
s.flags # => 3

Spaceship.warpdrive.count # => 1, all is well

Now we change the flags to just ..

class Spaceship < ActiveRecord::Base
  include FlagShihTzu

  has_flags 
            1 => :warpdrive
end

s = Spaceship.first
s.flags # => 3
s.warpdrive? # => true

# Here is the problem
Spaceship.warpdrive.count # => 0

When has_flags is used in a model and the corresponding column does not exist yet the plugin raises an exception.

This prevents migrations from running and therefore the required database column can't be created unless the has_flags call is removed or disabled.

this commt solves this issue. Please consider merging it.

Because of this line, the nice new methods are not defined for the default column name:
https://github.com/pboling/flag_shih_tzu/blob/master/lib/flag_shih_tzu.rb#L98
resulting in all_flags, selected_flags, etc.. not to be defined.

Is there a good reason for it?

Hi Peter,

now that flag_shih_tzu lives under your GitHub account, you should enable the build for it on
https://travis-ci.org.

Our existing .travis.yml does not need to be changed.

But you would need to update the build status image url in the README.rdoc from

{<img src="https://secure.travis-ci.org/xing/flag_shih_tzu.png" />}[http://travis-ci.org/xing/flag_shih_tzu]

to

{<img src="https://secure.travis-ci.org/pboling/flag_shih_tzu.png" />}[http://travis-ci.org/pboling/flag_shih_tzu]

It would be neat to be able to access the defined flags.

has_flags 1 => :duty_tax, 2 => :check_price, 3 => :customs_description,
            4 => :selling_price, 5 => :container_allocation, 6 => :freight_charge,
            7 => :handling_charge, 8 => :submitted, #, 9 => :completed,
            :column => 'processing_flags'

To get all the flags, I need to do this

Shipment.flag_mapping['processing_flags'].keys

Which is dangerous because it relies on the fact that implementation do not change.

The best would be to have a method shih_tzu_flags('processing_flags') => [:duty_tax, ..., :submitted]. Even better a method to get a hash => [:duty_tax => true, ... :submitted => false]

What do you think?

I'll try to make a pull request but I'm a bit new to Rails...

Would be great if we could get methods like published_changed? where published is a flag (inline with http://api.rubyonrails.org/v2.3.8/classes/ActiveRecord/Dirty.html)

Using FactoryGirl, I would like to be able to set flags in a constructor, this way:

s = FactoryGirl.create :spaceship, 
    flags: Spaceship.flags_value(:shields, :warpdrive)

This would create an Spaceship instance with shields and wrapdrive set to true.
Is it already possible and I'm not seeing the method?

I'm currently doing something like this to check the DB for an instance with the same flag state:

flags_state = shipping_item.unknown_origin? ? :unknown_origin : :not_unknown_origin
shipment.shipping_items.chained_flags_with('flags', flags_state)

It's quite ugly and doesn't scale well if I need to do it for more than one flag. Is there currently a better way to get a complete array of flag states (either flag or not_flag) for each flag so we can push it to chained_flags_with?

Hello, currently have this kind of problem, i have similar setup:

has_flags 1 => :warpdrive,
          2 => :shields,
          3 => :electrolytes,
          :column => 'features'

has_flags 1 => :spock,
          2 => :scott,
          :column => 'crew'

Here crew column can have 3 values: 0, 1, 2, 3 is it correct ?
But at some moment i notice that field not working as expected anymore, and when i check field value in database it has value of 6. So i then diable all flags and expect it to be 0

enterprise = Spaceship.new
enterprise.spock = false
enterprise.scott = false
enterprise.save!

And crew column now contains 4, so this column now 4,5,6,7, which is happened at some moment for unknown reason.

After upgrading, it seems like there may have been a major change in the flag retrieval because models that were returning "true" for this particular flag were now all uniformly returning "false" for that flag.

Here is the flag setup on the model:

has_flags 1 => :cje,
    2 => :apq,
    3 => :pje,
    4 => :ums,
    5 => :esp,
    6 => :avl,
    7 => :mpop,
    8 => :cvb,
    9 => :cob,
    10 => :cje,
    11 => :sue,
    12 => :lpl,
    13 => :ppl,
    14 => :lsms,
    15 => :lfwp,
    16 => :lfwop,
    17 => :ftp,
    18 => :plu,
    19 => :mpl,
    20 => :mat,
    21 => :skq,
    22 => :tma,
    23 => :rnw,
    flag_query_mode: :bit_operator,
    check_for_column: false

The flag in question is the last one, :rnw - all the others seem to be in the correct state.

has_flags 1=>:xxx, :column=>'asdadsasd' -> no warning/error, but when i try to use xxx it raises, a bit to late ?

I am using RSpec to test my app and when I used the gem, the tests starts failing. It tells me that the methods generated by this gem are undefined.

To make it work, I added the following lines in my spec/rails_helper.rb file:

RSpec.configure do |config|
  config.include FlagShihTzu
end

I would be glad to open a PR for this if you're interested.

given the following scenario.

has_flags 1 => :admin,
2 => :super_admin,
3 => :regional_manager ,
:column => :roles

has_flags 1=> :active, 2=>:inactive, :column => :status

How do I get a list of all set flags as an array of symbols? something like this User.first.role_list

Also how to I select inactive regional managers?

The way the generated scopes currently work is by generating an array of all possible values of flags where the desired flag is set, and then checking array membership in SQL. For example:

has_flags 1 => :foo,
          2 => :bar,
          3 => :baz

User.bar
#=> SELECT "users".* FROM "users" WHERE users.flags in (2, 3, 6, 7)

Here 2, 3, 6, 7 are all the possible values for flags in which bar is set.

This is fine for a small number of flags. However, my application currently uses 19 (!) flags on a single model. You don't want to see the query it generates... (It's long enough to overflow my terminal buffer.)

I wonder whether it wouldn't be faster to use bitwise operators in the generated SQL. Something like this:

User.bar
#=> SELECT "users".* FROM "users" WHERE users.flags & 2 = 2

Not only is this a hell of a lot more readable, it stays true to the spirit of the gem (after all, "Bitwise Operations are fast!"), and it uses a syntax which does not grow in length or complexity as the number of flags on the model increases.

DEPRECATION WARNING: class_inheritable_attribute is deprecated, please use class_attribute method instead. Notice their behavior are slightly different, so refer to class_attribute documentation first.

If we have models Pizza and Topping, where
Topping.has_flag :is_a_veg

and

Pizza.has_many :veggies, :class_name => 'Topping', :conditions => "#{Topping.is_a_veg_condition}"

If a migration prior to the migration which adds column flags to table toppings references a #pizza, that migration will fail with:

undefined method 'is_a_veg_condition' for #<Class:0x4364ecc>

The workaround I am using is to add to the Topping model:
unless Topping.respond_to?(:is_a_veg_condition)
def Topping.is_a_veg_condition
""
end
end

Not all that elegant i know =). I will fork and look for a better way to fix within flag_shih_tzu, Just wanted to drop a note about it.

I'm not sure what a good name for this really is, or the best way for it to work, but I was just writing some tests, and expecting model.attributes to return all my flags as individual Boolean attribute values, which of course, it did not :)

My flag field is called setup_steps so I figured out I could rewrite the test to use selected_setup_steps, but I still think a method to retrieve the flag values in the format of regular attributes (the same way you can set them) would be useful (i.e. { flag1: true, flag2: true, flag3: false })

What I'm not sure about is whether it makes more sense for that to be a model-wide method, like model.attributes_with_flags which would return all the normal attributes, plus additional attributes for each defined flag (regardless of how many flag fields are defined), or to make one method per flag field, like model.setup_steps_as_attributes or something to that effect... that one would be marginally easier to implement at least. Any opinion?

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!

Managed to get this gem up and running under a new rails 5 app I am working on without any troubles. Thanks for all the hard work on it!

I am however seeing a warning that is worth reporting:

DEPRECATION WARNING: #tables currently returns both tables and views. This behavior is deprecated and will be changed with Rails 5.1 to only return tables. Use #data_sources instead.

This occurs within the has_flags call (in my specific case with a custom column name if that helps).

Do let me know if I can offer any more detail.

Hey, guys, i think in this row there is a mistake
https://github.com/pboling/flag_shih_tzu/blob/master/lib/flag_shih_tzu.rb#L340
connection.data_sources
It should be
connection.data_sources.include?(custom_table_name)
otherwise has_table will be always true