stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.28k stars 172 forks source link

Using `morph` gives error on missing Warden::Manager midddleware? #304

Closed nathanvda closed 4 years ago

nathanvda commented 4 years ago

Bug Report

I am starting to play with stimulus reflex and see how we can implement in our project. I have a comments-section and using SR seemed pretty straight-forward and using complete page morphs is working incredibly easy, however ... in our specific application we have a map-view and a splitter dividing the "admin" content and the map content ... upon a complete page morph both the map and splitter are removed/uninitialised.

I was able to "protect" the map using the data-reflex-permanent but still lost the splitter. So .. I tried using a morph which seemed ideal in this case: we just need to re-render and replace the comment section.

However when doing so, I got the weirdest error:

StimulusReflex::Channel Failed to invoke CommentsReflex#edit! http://localhost:3014/projects/4/issues/6 Devise could not find the `Warden::Proxy` instance on your request environment.
Make sure that your application is loading Devise and Warden as expected and that the `Warden::Manager` middleware is present in your middleware stack.
If you are seeing this on one of your tests, ensure that your tests are either executing the Rails middleware stack or that your tests are using the `Devise::Test::ControllerHelpers` module to inject the `request.env['warden']` object for you. /Users/nathan/.rvm/gems/ruby-2.7.1@geotrax/gems/devise-4.7.2/lib/devise/controllers/helpers.rb:143:in `warden'

I do not completely understand this error, or the difference (a complete page replacement does work).

For completeness, some code:

module ApplicationCable
  class Connection < ActionCable::Connection::Base
    identified_by :current_user

    def connect
      self.current_user = env["warden"].user || reject_unauthorized_connection
    end
  end
end

My reflex is very simple:

class CommentsReflex < ApplicationReflex

  def edit
    Rails.logger.debug("Element dataset = #{element.dataset.inspect}")
    @edit_id = element.dataset[:id].to_i
    @issue = Issue.find(element.dataset[:parent_id].to_i)
    morph "#comments-container", ApplicationController.render(partial: "comments/thread", locals: {parent: @issue})
  end

  def cancel_edit
    @edit_id = nil
  end

end

This probably only makes the current_user available in my reflex and does not include the middleware in the morph.

So my question is

If this is a bug, I am willing to dive into some code to produce a PR (if you can point me into the right correction).

To be honest: I am incredibly impressed with stimulus-reflex and just starting to play around with it, thank you for this great approach and I am really curious how I can improve my apps this way.

Versions

StimulusReflex

External tools

Browser

leastbad commented 4 years ago

Hi Nathan! Thanks for the kind of words. This is definitely a strange one, but I'm confident that we'll get you sorted. FWIW, you should consider jumping on Discord when this sort of thing comes up - there's a solid group of folks able to help pretty much 24/7.

Generally, the fastest way for us to troubleshoot this sort of thing is to ask people to post an MVCE. We even have a project set up that you can fork, clone and branch as a starting point.

I just revised the Devise authentication section of the documentation back to a more expressive version of the connection module, because upon reflection I don't like treating the rejection as a possible return value. Given that full page reflexes are working for you, the connection isn't likely the concern, but it might make it easier to reason about things in the future. YMMV.

Alright, let's get to the meat of this. Whatever is causing you grief is happening when ActionPack is trying to re-render your partial. Unfortunately, you didn't include the contents of your partial in the issue, so I'm going to need you to do that. (This is why we like MVCEs - it's hard to anticipate what we'll need to see.)

The optimistic news for you is that almost all of us regularly render partials using selector morphs in an environment that uses Devise, so it definitely works and I believe we'll see something in the partial itself that is not resolving properly. You could test this by swapping out the ApplicationController.render() with an HTML string.

Looking forward to seeing this partial.

nathanvda commented 4 years ago

Thank you for your feedback!

Yes obviously you were right, when I just morphed using a string it just works. So then I started chipping at the view to see what could cause the error. Still not entirely clear what caused the error, because I then built a second partial from scratch to eliminate the error and I got it to work (wooooot!).

I think it could be caused by:

Thank you for your help/guidance!!

leastbad commented 4 years ago

I was hoping that you'd paste the partial, Nathan. Otherwise, you're the only one that benefits from this resolution.

Calling current_user should be fine. Creating forms should be fine.

Can you confirm that you're delegating current_user to connection in your ApplicationReflex?

Again, this is why we ask for MVCEs we can run locally. You're making me guess.

As far as I'm concerned, this issue shouldn't be closed until we know exactly why you were seeing this error.

nathanvda commented 4 years ago

Ow I thought I was doing you a favour working it out for myself. Also: I am just starting with stimulus and stimulus-reflex and I notice I am also making a lot of "errors" in hooking things up (so it could just be human error?).

But I understand that the error is probably confusing, and you might want to be able to get to the bottom of it.

I will try to pinpoint the explicit cause, and then check if I can create an MVCE.

leastbad commented 4 years ago

I do appreciate you wanting to solve your own issues, for sure! But I'm now asking to see the partial for the third time...

No need to be hard on yourself, we all start at the beginning. :)

leastbad commented 4 years ago

Oh and a late-blooming thought: I saw above that you mentioned converting your form code to work with SR. Don't do this unless you have a good reason to do so! Standard UJS remote forms work great, especially if you use Optimism to handle validation errors.

leastbad commented 4 years ago

I would appreciate you giving this conversation a read-through and tell me what you think. I give some actionable steps that should get current_user working for you: https://discordapp.com/channels/629472241427415060/733725826411135107/754809268917502084

nathanvda commented 4 years ago

Hi @leastbad wow thank you for your work.

So it is actually really easy to reproduce: just reference current_user in the partial and I get the error.

As mentioned before I solved this passing in current_user as a local parameter to my partial (I can access it inside the reflex just not in the partial).

Your code in the discord thread you pointed seems like it would solve that even better. I am going to try that out later (not sure when, since I am going on a small holidayyyyy 👍 🕺

BTW I have a small problem with a controller I added which does not seem to get picked up by stimulus/reflex, where is the best place to ask questions, do I jump in the discord or open a new issue?

leastbad commented 4 years ago

Hi Nathan,

I admit that I ended up pretty confused. In the end, I first figured out how to create an enhanced renderer that contains the env["warden"] and I was excited because it seemed to work. Then I got curious and I pulled it all out, just testing morphing a partial that contained a current_user and it actually worked fine, even without the custom renderer. So that was kind of confusing! Now I am anxiously waiting to hear whether the technique fixes your problem. I hope that you have a great vacation, but I also hope you have a few minutes to test it before you head out.

Controllers are only run when you're doing page morphs, selector morphs don't run controllers.

I would personally hit Discord before filing issues... issues are for bugs. :)