RFC: Support Backend-Only Stores #96

stewart · 2017-05-22T15:48:15Z

This PR introduces some changes to solidus_auth_devise supporting Solidus stores that use solidus_backend, but have custom front-ends (read: do not use solidus_frontend).

The routes for password reset have also been added to the admin scope to allow the full Devise recovery flow to be followed. A corresponding change to the Devise initialiser is required to allow Devise to pick up on the Spree routes. Additionally, the configuration of the admin routes has been updated.

dgra · 2017-05-22T16:05:59Z

@stewart You may be able to continue to use the authenticate_spree_user, admin_spree_user, etc if you add singular: :spree_user to the devise_for like

      devise_for(:spree_user, {
        class_name: 'Spree::User',
        controllers: {
          sessions: 'spree/admin/user_sessions',
          passwords: 'spree/admin/user_passwords'
        },
        skip: [:unlocks, :omniauth_callbacks, :registrations],
        path_names: { sign_out: 'logout' },
        path_prefix: :user,
        singular: :spree_user # This line was added.
      })

Also, you may not, I didn't get too far into it.

Nice work!

tvdeyen

I think this is a great improvement. Admin user management should be separated from the frontend.

There is a failing spec, that might be because we did not updated devise_for in the admin scope. But I'm not sure.

Could you please add a CHANGELOG for this?

tvdeyen · 2017-05-23T09:25:51Z

config/routes.rb

@@ -55,11 +55,16 @@
        path_prefix: :user
      })

-      devise_scope :spree_user do
+      devise_scope :admin_spree_user do


I'm asking myself if we also need to change the devise_for above to admin_spree_user?

stewart · 2017-05-23T10:19:45Z

I've updated this per @dgra's suggestion - now uses the singular option to use the same resource name for both frontend and backend.

Additionally, I've removed the auto-generated routes, as we already generate pretty alternatives.

This is the routes diff from an example application, between the last release and this PR:

diff --git a/before b/after
index d49ee08..7842ffa 100644
--- a/before
+++ b/after
@@ -2,18 +2,14 @@ Prefix Verb URI Pattern Controller#Action
  spree      /           Spree::Core::Engine
 
 Routes for Spree::Core::Engine:
-                               new_admin_spree_user_session GET    /user/spree_user/sign_in(.:format)                                                                 spree/admin/user_sessions#new
-                                   admin_spree_user_session POST   /user/spree_user/sign_in(.:format)                                                                 spree/admin/user_sessions#create
-                           destroy_admin_spree_user_session GET    /user/spree_user/logout(.:format)                                                                  spree/admin/user_sessions#destroy
-                              new_admin_spree_user_password GET    /user/spree_user/password/new(.:format)                                                            spree/admin/user_passwords#new
-                             edit_admin_spree_user_password GET    /user/spree_user/password/edit(.:format)                                                           spree/admin/user_passwords#edit
-                                  admin_spree_user_password PATCH  /user/spree_user/password(.:format)                                                                spree/admin/user_passwords#update
-                                                            PUT    /user/spree_user/password(.:format)                                                                spree/admin/user_passwords#update
-                                                            POST   /user/spree_user/password(.:format)                                                                spree/admin/user_passwords#create
                                          admin_unauthorized GET    /admin/authorization_failure(.:format)                                                             spree/admin/user_sessions#authorization_failure
                                                 admin_login GET    /admin/login(.:format)                                                                             spree/admin/user_sessions#new
                                    admin_create_new_session POST   /admin/login(.:format)                                                                             spree/admin/user_sessions#create
                                                admin_logout GET    /admin/logout(.:format)                                                                            spree/admin/user_sessions#destroy
+                                     admin_recover_password GET    /admin/password/recover(.:format)                                                                  spree/admin/user_passwords#new
+                                       admin_reset_password POST   /admin/password/recover(.:format)                                                                  spree/admin/user_passwords#create
+                                        admin_edit_password GET    /admin/password/change(.:format)                                                                   spree/admin/user_passwords#edit
+                                      admin_update_password PUT    /admin/password/change(.:format)                                                                   spree/admin/user_passwords#update
                                          admin_search_users GET    /admin/search/users(.:format)                                                                      spree/admin/search#users
                                       admin_search_products GET    /admin/search/products(.:format)                                                                   spree/admin/search#products
                                       home_admin_dashboards GET    /admin/dashboards/home(.:format)                                                                   spree/admin/dashboards#home

tvdeyen

👍 💯

bbuchalter · 2017-05-24T07:53:03Z

👍 but I think we could use a rebase of the commits which toggle the name of the routes.

acreilly · 2017-06-16T01:58:50Z

Has this been approved to merge?

tvdeyen · 2017-10-04T14:19:35Z

@stewart I'm still a huge fan of this change! Would you mind to rebase and have a look into the failing specs?

stewart · 2017-10-04T16:39:06Z

Rebased again - there are a couple of outstanding failures when paired with Solidus' master branch (these are present on this repo's master as well), but it passes with all released versions.

If there's anything more that needs to be done to get this PR ready-to-go, please let me know.

tvdeyen · 2017-10-04T18:51:12Z

@stewart thanks. #113 should fix the current master builds. Would you mind to review, merge if satisfied and rebase your branch after that?

This commit introduces some breaking changes to solidus_auth_devise supporting Solidus stores that use solidus_backend, but have custom front-ends (read: do not use solidus_frontend). These changes mainly take the form of scope corrections to the admin scoped controllers, moving from `spree_user` to `admin_spree_user`, which Devise is expecting. This includes changes to the routes, controllers, and views. Additionally, a small change has been made to `Spree::AuthenticationHelpers` to allow it to support both `spree_user` and `admin_spree_user` Devise scopes. The routes for password reset have also been added to the admin scope to allow the full Devise recovery flow to be followed. A corresponding change to the Devise initialiser is required to allow Devise to pick up on the Spree routes.

This commit updates the backend route configuration for Devise to remove the `admin_` prefix from resource references. This ensures we can always use `spree_user` to refer to the User model from controllers, and the authentication helpers.

This commit updates the backend route configuration for Devise, to skip generating the default route set. We already specify these routes with much more convenient names (`/admin/login` vs `/user/spree_user/sign_in`).

stewart · 2017-10-04T20:06:22Z

Rebased once more and green across the board. Based on the two 👍s already given, I'm going to merge this.

tvdeyen · 2017-10-04T21:22:42Z

👍

mamhoff assigned tvdeyen May 23, 2017

tvdeyen requested changes May 23, 2017

View reviewed changes

tvdeyen added the SolidusConf Hack Day label May 23, 2017

tvdeyen approved these changes May 23, 2017

View reviewed changes

stewart requested a review from jhawthorn May 23, 2017 14:08

acreilly mentioned this pull request May 24, 2017

Allow Spree::Admin::UserPasswordsController to be accessed from admin… #97

Closed

stewart force-pushed the rfc/support-backend-only-stores branch 2 times, most recently from f9434e5 to ae77ed3 Compare September 12, 2017 22:06

gmacdougall approved these changes Oct 4, 2017

View reviewed changes

stewart force-pushed the rfc/support-backend-only-stores branch from ae77ed3 to fc1504d Compare October 4, 2017 16:03

stewart added 3 commits October 4, 2017 12:45

Skip generation of default Devise routes for admin

b2018ea

This commit updates the backend route configuration for Devise, to skip generating the default route set. We already specify these routes with much more convenient names (`/admin/login` vs `/user/spree_user/sign_in`).

stewart force-pushed the rfc/support-backend-only-stores branch from fc1504d to b2018ea Compare October 4, 2017 19:45

stewart merged commit 47b2f54 into solidusio:master Oct 4, 2017

stewart deleted the rfc/support-backend-only-stores branch October 4, 2017 20:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Support Backend-Only Stores #96

RFC: Support Backend-Only Stores #96

stewart commented May 22, 2017 •

edited

Loading

dgra commented May 22, 2017

tvdeyen left a comment

tvdeyen May 23, 2017

stewart commented May 23, 2017

tvdeyen left a comment

bbuchalter commented May 24, 2017 •

edited

Loading

acreilly commented Jun 16, 2017

tvdeyen commented Oct 4, 2017

stewart commented Oct 4, 2017

tvdeyen commented Oct 4, 2017

stewart commented Oct 4, 2017

tvdeyen commented Oct 4, 2017

RFC: Support Backend-Only Stores #96

RFC: Support Backend-Only Stores #96

Conversation

stewart commented May 22, 2017 • edited Loading

dgra commented May 22, 2017

tvdeyen left a comment

Choose a reason for hiding this comment

tvdeyen May 23, 2017

Choose a reason for hiding this comment

stewart commented May 23, 2017

tvdeyen left a comment

Choose a reason for hiding this comment

bbuchalter commented May 24, 2017 • edited Loading

acreilly commented Jun 16, 2017

tvdeyen commented Oct 4, 2017

stewart commented Oct 4, 2017

tvdeyen commented Oct 4, 2017

stewart commented Oct 4, 2017

tvdeyen commented Oct 4, 2017

stewart commented May 22, 2017 •

edited

Loading

bbuchalter commented May 24, 2017 •

edited

Loading