Cleanup customization interfaces #282
Conversation
rafalcymerys
force-pushed
the
feature/cleanup_customization_interfaces
branch
from
06a1b23 to
49fcce9
Compare
There was a problem hiding this comment.
This file needs to be added to spree_rails_frontend (e.g. in this PR).
Without it, when app is being initialized spree_rails_frontend does not find it and throws:
uninitialized constant Spree::Admin::Actions::ProductPreviewActionBuilder (NameError)
Rails.application.config.spree_backend.actions[:product] = Spree::Admin::Actions::ProductPreviewActionBuilder.new.build
There was a problem hiding this comment.
I posted a draft where I moved the creation and details of that button to spree_rails_frontend - since it should know best how to create a button referring to a preview.
https://github.com/spree/spree_rails_frontend/tree/feature/add_preview_button_to_admin_panel
| @@ -37,12 +37,15 @@ class Engine < ::Rails::Engine | |||
| Rails.application.config.spree_backend.actions[:users] = Spree::Admin::Actions::UsersDefaultActionsBuilder.new.build | |||
| Rails.application.config.spree_backend.actions[:user] = Spree::Admin::Actions::UserDefaultActionsBuilder.new.build | |||
| Rails.application.config.spree_backend.actions[:products] = Spree::Admin::Actions::ProductsDefaultActionsBuilder.new.build | |||
| Rails.application.config.spree_backend.actions.include?(:images) ? (Rails.application.config.spree_backend.actions[:images].items << Spree::Admin::Actions::ImagesDefaultActionsBuilder.new.build.items).flatten! : Rails.application.config.spree_backend.actions[:images] = Spree::Admin::Actions::ImagesDefaultActionsBuilder.new.build | |||
| Rails.application.config.spree_backend.actions[:product] = Spree::Admin::Actions::Root.new | |||
There was a problem hiding this comment.
Do we have any guarantee that the config changes from spree_backend will be executed before the config changes from spree_rails_frontend? 🤔
If yes - then this is ok.
If not - then there's a risk tat Spree::Admin::Actions::Root.new would override the contents that were assigned during the after_initialize in the spree_rails_frontend.
Presumably this can be avoided by having the gems being listed in the Gemfile in a specific order but we have no control over how people compose their Gemfiles.
There was a problem hiding this comment.
We can add a check in spree_rails_frontend, that ensures that spree_backend is initialized first, and if not, throws an exception that tells users what to do :)
There was a problem hiding this comment.
Another option I was exploring was to run the initialization process in a regular initializer, but due to how preferences are built right now, in order to load routes Spree needs to establish a database connection. I think if we solve this, we could make it independent on the order - this is however something to be fixed on its own separately, unfortunately this looks like a bigger rewrite.
delete-merged-branch
bot
deleted the
feature/cleanup_customization_interfaces
branch
This is a follow-up PR to #276 and #274. I made the following changes:
completed_checkin tabs withavailability_checksas their semantics seems to be the same