-
-
Notifications
You must be signed in to change notification settings - Fork 318
Convert auth accessors to configuration object #3418
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
base: main
Are you sure you want to change the base?
Conversation
d5c9603 to
954e985
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3418 +/- ##
==========================================
+ Coverage 97.25% 97.27% +0.02%
==========================================
Files 291 291
Lines 7682 7706 +24
==========================================
+ Hits 7471 7496 +25
+ Misses 211 210 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
954e985 to
8645785
Compare
c6d73b9 to
74d6166
Compare
|
Rebased on #3424 in order to get the |
74d6166 to
a85e1b4
Compare
|
|
||
| alias_method :get, :send | ||
| alias_method :[], :get | ||
| alias_method :configure, :tap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s.m.a.r.t.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the example in the commit message is weird though.
README.md
Outdated
| Alchemy.unauthorized_path = '/some/public/page' # Defaults to '/' | ||
| Alchemy.configure do |config| | ||
| config.auth.configure do |auth| | ||
| auth.user_class_name = 'YourUserClass' # Defaults to 'User' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auth.user_class_name = 'YourUserClass' # Defaults to nil
lib/alchemy/configurations/auth.rb
Outdated
| # | ||
| # Alchemy has some defaults for user model name and login logout path names: | ||
| # | ||
| # +Alchemy.config.auth.user_class_name+ defaults to +'User'+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# +Alchemy.config.auth.user_class_name+ defaults to +nil+
lib/alchemy/configurations/auth.rb
Outdated
| Or add the `alchemy-devise` gem to your Gemfile: | ||
| bundle add alchemy-devise | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe find a better way for this warning
e1adfda to
43681c3
Compare
|
This pull request has not seen any activiy in a long time. |
|
@mamhoff this needs a rebase and conflict resolution. |
d13264a to
20a4bdb
Compare
tvdeyen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for rebasing. I think we should get rid of the extra auth class. It is unnecessary imo.
|
|
||
| alias_method :get, :send | ||
| alias_method :[], :get | ||
| alias_method :configure, :tap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the example in the commit message is weird though.
lib/alchemy/configurations/auth.rb
Outdated
|
|
||
| module Alchemy | ||
| module Configurations | ||
| class Auth < Alchemy::Configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should add another configuration class just for auth. The methods have conflict free names and it only makes it harder to read and use. Alchemy.config.user_class is perfect. Alchemy.config.auth.user_class is to verbose and does not provide anything useful.
a4034bf to
e80ebb2
Compare
If `Alchemy.user_class` is `nil`, which it is in case of an invalid configuration, it's not a class, and we can't use the `<` operator. Let's use duck typing instead.
This allows us to do ``` Alchemy.configure do |config| config.user_class = "Spree::User" end ```
Creates a configuration object for authentication that can be initialized outside of a `config.to_prepare` block. Configuration is now in `Alchemy.config.auth`.
e80ebb2 to
1b0afb4
Compare
What is this pull request for?
This converts the
auth_accessors.rbmodule, which is purely configuration, to a configuration object.All methods are still there, but deprecated, and a few accessors stay in
lib/alchemy.rbas shortcuts.Notable changes (remove if none)
We can now configure Alchemy's auth system entirely outside of a
config.to_prepareblock.Checklist