Fix stores with no authentication

[waiting-for-dev]

We deprecated #try_spree_current_user in favor of
#spree_current_user in #3923. The former defaulted to nil when
no authentication library was present, but the latter was simply not
defined under those circumstances.

This commit restores the old behavior, as the code base works with the
assumption that current user is nil when no extension has defined it
(see one example or another).

The issue was raised in the support channel.

[kennyadsl]

Left a small comment, thanks!

[kennyadsl]

Some lines later, in try_spree_current_user, we have:

        def try_spree_current_user
          # This one will be defined by apps looking to hook into Spree
          # As per authentication_helpers.rb
          if respond_to?(:spree_current_user, true)
            spree_current_user
          # This one will be defined by Devise
          elsif respond_to?(:current_spree_user, true)
            current_spree_user
          end
        end

With the proposed change, will the respond_to?(:spree_current_user, true) always be true?

[waiting-for-dev]

Yes, but I think that's ok. Then spree_current_user will delegate to the auth library, and solidus_auth_devise is already implementing it as noted in the linked PR.

My understanding is that #current_spree_user is never defined (solidus_auth_devise delegates to #current_user instead), so it's safe to forgo its support.

[kennyadsl]

I'm not sure to fully understand here. If we are 100% sure that the elsif will never be executed, why keep it?

[waiting-for-dev]

Yes, I guess we can remove it. Pinging @elia to be 100% sure.

[elia]

@waiting-for-dev you're right, we shouldn't care about current_spree_user and this will work.
I'm just a bit hesitant on having a cover for a missing authentication implementation. Thoughts on having a sort of NoAuth module that we inject from within an initializer like we do for Spree::AuthenticationHelpers in auth-devise?

solidus_auth_devise/lib/spree/auth/engine.rb

module Spree
  module Auth
    class Engine < Rails::Engine
      …
      config.to_prepare do
        Spree::Auth::Engine.prepare_backend if SolidusSupport.backend_available?
        Spree::Auth::Engine.prepare_frontend if SolidusSupport.frontend_available?

        ApplicationController.include Spree::AuthenticationHelpers
      end

in the no-auth case would be something like:

my-app/config/initializers/solidus_auth.rb

Rails.application.reloader.to_prepare do
  ApplicationController.include Spree::NoAuth
end

[aldesantis]

@elia our new storefront customization strategy is never to inject anything, but rather to copy-paste code into the host application. We don't want to do anything weird with the storefront as that makes customization harder.

[waiting-for-dev]

Thanks for looking into it, @elia! On top of what @aldesantis said, I think it's not worth it to make things complex. I agree that we don't have a clear separation of concerns for the authentication system. But extracting things to separated modules would give an apparent impression of orthogonality, but the truth is that all the components expect something to be there. Until we don't revisit it, I think it's better to have the no-op behavior as visible as possible.

[waiting-for-dev]

cc @elia