janko / rodauth-rails

Rails integration for Rodauth authentication framework
https://github.com/jeremyevans/rodauth
MIT License
571 stars 40 forks source link

rodauth-rails omits setting title when calling the view #112

Closed HoneyryderChuck closed 2 years ago

HoneyryderChuck commented 2 years ago

rodauth-rails, when overriding view, skips the set_title logic, which means that @page_title won't be set. I believe this is a bug, at least considering someone using the title_instance_variable option.

janko commented 2 years ago

Thanks for raising this point, honestly, I didn't know how I should handle this. By default, the page title instance variable will be set on the Roda instance, is this useful in context of Rails? Maybe it would help if we set it on the controller instance instead?

HoneyryderChuck commented 2 years ago

I think that, if you're using the title_instance_variable, you expect it to be set when the layout/page renders. That being said, the way it is done right now, I'd have to do rodauth.scope.instance_variable_get(:@page_title) to access it from within a rails view.

I think this can be fixed in two ways: either rodauth gets a page_title method one can use (and then it's just rodauth.page_title), or rodauth-rails finds a way to make all instance variables defined in the routes available in the scope of view rendering, so that instead of rodauth.scope.instance_variable_get(:@page_title), one just does @page_title.

The second is probably the most encompassing one, and it'd solve an issue I have in rodauth-oauth rails templates, but may be harder to get right. What are your thoughts?

janko commented 2 years ago

I was thinking that rodauth-rails could override set_title to set the instance variable on the controller instance instead of the Roda instance (since Roda instance isn't doing layout rendering with rodauth-rails). The controller instance is cached during a Rodauth request, so I'm hoping that would work. What do you think?

HoneyryderChuck commented 2 years ago

That sounds like a decent workaround. Is this the only instance variable to cater to?

janko commented 2 years ago

I believe so, AFAIK it's the only variable set on the Roda instance by Rodauth.

HoneyryderChuck commented 2 years ago

👍 would it be as simple as do controller_instance.instance_variable_set(... in my MR? Or smth more involved?

janko commented 2 years ago

Yes, I believe that's all it would take 👍🏻

HoneyryderChuck commented 2 years ago

just pushed a patch.