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.

Sign up for GitHub

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

Jump to bottom

Maintain Rails 4.2 compatibility #863

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Maintain Rails 4.2 compatibility #863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a122f8
Check for credentials.
astupka Apr 23, 2024
785a072
Fix to make active_record adapter work with 4.2
astupka Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/flipper/adapters/active_record.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ def get_all
rows_query = features.join(gates, ::Arel::Nodes::OuterJoin)
.on(features[:key].eq(gates[:feature_key]))
.project(features[:key].as('feature_key'), gates[:key], gates[:value])
gates = @feature_class.connection.select_rows(rows_query)
select_method = rails_version_over_4_2? ? :select_rows : :select_all
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
select_method = rails_version_over_4_2? ? :select_rows : :select_all
select_method = respond_to?(:select_rows, true) ? :select_rows : :select_all

Instead of version detection, can this use feature detection here?

gates = @feature_class.connection.send(select_method, rows_query)


# group the gates by feature key
grouped_gates = gates.inject({}) do |hash, (feature_key, key, value)|
Expand All @@ -154,6 +156,10 @@ def get_all
end
end

def rails_version_over_4_2?
Rails::VERSION::MAJOR * 10 + Rails::VERSION::MINOR > 42
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Rails::VERSION::MAJOR * 10 + Rails::VERSION::MINOR > 42
Rails.version.to_f > 4.2

If version detection is the only way, this to_f trick is a nice way to get the major/minor version.

end

# Public: Enables a gate for a given thing.
#
# feature - The Flipper::Feature for the gate.
Expand Down
6 changes: 4 additions & 2 deletions lib/flipper/engine.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ def self.default_strict_value

initializer "flipper.default", before: :load_config_initializers do |app|
# Load cloud secrets from Rails credentials
ENV["FLIPPER_CLOUD_TOKEN"] ||= app.credentials.dig(:flipper, :cloud_token)
ENV["FLIPPER_CLOUD_SYNC_SECRET"] ||= app.credentials.dig(:flipper, :cloud_sync_secret)
if defined?(app.credentials)
ENV["FLIPPER_CLOUD_TOKEN"] ||= app.credentials.dig(:flipper, :cloud_token)
ENV["FLIPPER_CLOUD_SYNC_SECRET"] ||= app.credentials.dig(:flipper, :cloud_sync_secret)
end

require 'flipper/cloud' if cloud?

Expand Down
Loading