[#161102] Adds active_storage_for_images_only feature flag
#3348
Conversation
This turns on ActiveStroage, just for the Image module. This PR also sets an enviroment variable for `facility_tile_list`
`active_storage_for_images_only`
jossim
changed the title
active_storage_for_images feature flagactive_storage_for_images_only feature flag
| @@ -9,7 +9,7 @@ module ActiveStorageFile | |||
| has_one_attached :file | |||
|
|
|||
| def assign_attributes(new_attributes) | |||
| if SettingsHelper.feature_on?(:active_storage) | |||
| if SettingsHelper.active_storage_enabled? | |||
There was a problem hiding this comment.
Do we need this feature flag here? I can't recall if we need it for some reason or perhaps it was left over from a previous iteration of the code, before we separated things into modules
There was a problem hiding this comment.
I also can't recall. It passes CI with it removed though, so I suppose it's fine to leave it out. I recall this method causing a lot of problems, so likely extra stuff was left in it while working up to a solution.
app/lib/settings_helper.rb
Outdated
| @@ -29,6 +29,10 @@ def self.relays_enabled_for_reservation? | |||
| setting "relays.#{Rails.env}.reservation_enabled" | |||
| end | |||
|
|
|||
| def self.active_storage_enabled? | |||
| feature_on?(:active_storage) || feature_on?(:active_storage_for_images_only) | |||
There was a problem hiding this comment.
I think I'm leaning towards letting each feature flag do its own thing - what's your thinking about intertwining here?
If we keep this logic I'd vote for renaming to active_storage_for_images_enabled?
There was a problem hiding this comment.
Because of how the flags are named, it seemed like active_storage could/would be a superset of active_storage_for_images_only. Also doing it this way would mean no change in OSU's settings.yml. Neither of these may be interesting enough reasons, don't have a strong opinion.
`active_storage_for_images_only flag`
Release Notes
This adds the
active_storage_for_images_onlyfeature flag to allow for the use ofActiveStoragejust forDownloadableFiles::Image.Since UMass would like to use images but is using Paperclip otherwise, this will allow them to add images via
ActiveStorageso there's less to migrate over toActiveStoragewhen the time comes.