jekuno / milia

Easy multi-tenanting for Rails5 (or Rails4) + Devise
MIT License
341 stars 72 forks source link

Rails 5 support #64

Closed jekuno closed 7 years ago

jekuno commented 8 years ago

Did you test milia with Rails 5 already or do you have plans to do so?

dsaronin commented 8 years ago

I have not tested milia on Rails 5. We should probably try to find interested people to help with milia maintenance, as I am retired now from software development.

On Tue, Mar 8, 2016 at 9:10 AM jekuno notifications@github.com wrote:

Did you test milia with Rails 5 already or do you have plans to do so?

— Reply to this email directly or view it on GitHub https://github.com/dsaronin/milia/issues/64.

jekuno commented 7 years ago

@MatthewMarkgraaff @julienroger @epoplavskis @2point0concepts @dearty @kylegraydev @pineapplethief All of you forked milia and worked towards using it with Rails 5. Does anybody of you already use milia with Rails5? This would be a great help for many people who use milia and want to switch to Rails5.

Please drop a short note in this thread!

pineapplethief commented 7 years ago

@jekuno I hadn't tested it thoroughly, but it seems to work after you bump activerecord and devise versions to be compatible with rails 5

MatthewMarkgraaff commented 7 years ago

@jekuno Hey there. Unfortunetely I didn't get too far with it but if I get some time I'll take a look. Try @pineapplethief suggestions and let us know how it goes.

dsaronin commented 7 years ago

I'm retired now and looking for someone to take over the Milia project... Any volunteers?

On Wed, Jan 18, 2017, 02:59 Matthew Markgraaff notifications@github.com wrote:

@jekuno https://github.com/jekuno Hey there. Unfortunetely I didn't get too far with it but if I get some time I'll take a look. Try @pineapplethief https://github.com/pineapplethief suggestions and let us know how it goes.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dsaronin/milia/issues/64#issuecomment-273446224, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZDDLS1Ewobu74XO8nSs-54bEY3-6KFks5rTfCdgaJpZM4Hr8fy .

jekuno commented 7 years ago

I like the project and it looks as if the only other popular row based multi tenanting gemacts_as_tenant has been put on ice as well (https://github.com/ErwinM/acts_as_tenant/issues/152#issuecomment-269588618).

So even as I'm quite busy right now I might jump into keeping milia alive if we find more developers willing to participate.

dsaronin commented 7 years ago

That would be awesome. My design philosophy has always been simplicity and combinatorial basic features like Lego blocks; not trying to meet every use case, only the most common 80%. You'll find some users trying to pull the project in all kinds of directions.

On Wed, Jan 18, 2017, 08:31 jekuno notifications@github.com wrote:

I like the project and it looks as if the only other popular row based multi tenanting gemacts_as_tenant has been put on ice as well (ErwinM/acts_as_tenant#152 (comment) https://github.com/ErwinM/acts_as_tenant/issues/152#issuecomment-269588618 ).

So even as I'm quite busy right now I might jump into keeping milia alive if we find more developers willing to participate.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dsaronin/milia/issues/64#issuecomment-273525822, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZDDFK6JiyNCtwqBvL4nhLCrDTdTWWyks5rTj54gaJpZM4Hr8fy .

jekuno commented 7 years ago

Sounds good - simplicity is important to keep milia maintainable in long term. My first goal would be to ensure Rails 5 compatibility. Can I reach you via your majozi email address so we can talk about some details?

dsaronin commented 7 years ago

use dsaronin@gmail.com please.

On Wed, Jan 18, 2017 at 9:18 AM jekuno notifications@github.com wrote:

Sounds good - simplicity is important to keep milia maintainable in long term. My first goal would be to ensure Rails 5 compatibility. Can I reach you via your majozi email address so we can talk about some details?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dsaronin/milia/issues/64#issuecomment-273539557, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZDDMETuOmsCMCYTIQKwI_-8s2A6f3Rks5rTklXgaJpZM4Hr8fy .

epoplavskis commented 7 years ago

hey @jekuno, we at @2point0concepts got milia working with rails 5 just by updating the gemspec file, and have been using it for a few months now in one of our major projects without an issue. https://github.com/2point0concepts/milia

jekuno commented 7 years ago

@pineapplethief @MatthewMarkgraaff @epoplavskis Thanks for your responses. I also tested milia with Rails 5 during the past two days and could turn all of my tests green with just a few adaptions. You can see the changes in https://github.com/jekuno/milia/tree/rails5-support. As soon as I finished the work the changes will most probably find their way back to the milia gem.

mattverlaque commented 7 years ago

Thanks for the work you guys are doing. Since you've already walked the walk, I figured I would ask: after upgrading, I've noticed that when I add a staff member, it sends out the "password changed" email instead of the "confirmation" email. The new user is still getting added to the DB properly and no errors are getting thrown - it just seems to be sending the wrong email. Additionally, the flash message about adding a new member can't seem to find the new user's email address from here:

flash[:notice] = "New Staff Member Invited (#{@user.email})!"

Anything obvious I might be overlooking? To be fair, I'm relatively new in the Rails community and this is my first upgrade - could have been something I did incorrectly during that process.

dsaronin commented 7 years ago

Kuno, my first guess would be that Devise, again, has changed its API slightly, probably the callback after a new member is created.

On Fri, Jan 20, 2017, 20:17 runyourgym notifications@github.com wrote:

Thanks for the work you guys are doing. Since you've already walked the walk, I figured I would ask: after upgrading, I've noticed that when I add a staff member, it sends out the "password changed" email instead of the "confirmation" email. The new user is still getting added to the DB properly and no errors are getting thrown - it just seems to be sending the wrong email. Additionally, the flash message about adding a new member can't seem to find the new user's email address from here:

flash[:notice] = "New Staff Member Invited (#{@user.email})!"

Anything obvious I might be overlooking?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dsaronin/milia/issues/64#issuecomment-274233663, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZDDO_AX-a7x8Sv8btc-GoIckQuFxsiks5rUYa9gaJpZM4Hr8fy .

jekuno commented 7 years ago

@runyourgym Did you upgrade using the branch https://github.com/jekuno/milia/tree/rails5-support? Can you confirm that the flash message contained the email address before updating? Which devise version do you use now and which before?

mattverlaque commented 7 years ago

Thanks for the replies @dsaronin and @jekuno. I'm doing the upgrade on a separate branch, my master branch (identical except for the Rails 5 upgrade) still works perfectly. That was my first thought too (that it got messed up previously), but it doesn't seem to be the case.

I was on Devise 3.5.10 prior to the upgrade, and am now on 4.2.0.

@jekuno - I am specifying the Milia gem from your repo and branch, as follows:

gem 'milia', :git => 'git://github.com/jekuno/milia.git', :branch => 'rails5-support'

Thanks so much for any assistance.

Matt

jekuno commented 7 years ago

@runyourgym Hunting on that issue right now. Can you provide any additional info already?

@dsaronin When inviting a new member save_and_invite_member from milia gets called which basically does a user.save. Am I right that you expect this to trigger a callback which sends an invitation? It seems as if this doesn't happen. I had a look at devise_invitable 1.7. It contains aDevise::InvitationsController which has a create action that calls invite_resource (which sends an invitation mail). Alternatively devise_invitable also allows for calling user.invite! directly which sends an invitation. What would be the most elegant way to reenable sending invitations?

mattverlaque commented 7 years ago

@jekuno No additional information at this time, I reconfirmed that the Rails version was indeed what initiated the bug but other than that I'm not sure what's causing it to fail. Were you able to reproduce it?

Based on the logs it looks as if it's creating the record in the database and then immediately updating the same record, except on the second run it's putting the email into the unconfirmed_email field. The re-commit on the password is what's triggering the password change, and the confirmation email is not going out.

screen shot 2017-01-27 at 1 17 22 am

dsaronin commented 7 years ago

Kuno, well this is the problem I've always had with devise: the API is never stable. I don't recognize the names of the devise methods which you mentioned, so I'm assuming there were changes. I'll have to look into the existing milia code to let you know the expectation.

On Thu, Jan 26, 2017, 22:12 runyourgym notifications@github.com wrote:

@jekuno https://github.com/jekuno No additional information at this time, I reconfirmed that the Rails version was indeed what initiated the bug but other than that I'm not sure what's causing it to fail.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jekuno/milia/issues/64#issuecomment-275596371, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZDDLDBlk-_e4pdTePYNLnhJVjeJsCuks5rWYrKgaJpZM4Hr8fy .

dsaronin commented 7 years ago

Kuno, I've looked at the milia-specific code (gem itself). The only places that reference Devise callbacks appear to be in app/controllers/registrations_controller.rb, lines 122 to 138. But they merely continuing the (then at least) standard way to override devise methods.

But I suspect the issue with the either wrong or no email being generated, is really from milia usage & implementation instructions, notably in the sample app which I reference in the milia documentation and which the generator generates. This sample was provided as a way to show how to use milia and to help people see the correct way to implement milia. I just tried to transfer ownership of the github for the sample-milia-app to you, but you already have repository so named. I've also transferred the web-app-theme depository which is the template for the sample-milia-app.

Right now I don't see anywhere specific within sample-milia-app for designating mailer views, so it probably relies totally on the default view templates of devise, and it relies on Devise's default email callbacks. Older versions of milia did have to override Devise callbacks, but with the Rails 4 upgrade, that was no longer necessary.

You will have to dig within Devise itself to find what Devise is now expecting for a successful mail callback. What I always did to do that, is to go into where the Devise gem is installed (I always use Ruby Version Manager, rvm, so it's easy to do that), then I use ack-grep to globally search for terms related to what I'm trying to find.

Good luck.

dsaronin commented 7 years ago

PS: you may want to first look at any changes Devise has made to the expected parameter config file in config/initializers/devise.rb. I suspect that newer versions of Devise may have made changed to expectations in this file.

jekuno commented 7 years ago

@runyourgym The master branch of milia now is dedicated to Rails 5. Could you please confirm the bug with the new Rails 5 sample app: https://github.com/jekuno/milia#sample-app

Regarding the flash[:notice]: You might have to do a @user.reload so that it knows the email before you display the flash message.

jekuno commented 7 years ago

@runyourgym I opened #68 regarding the issue with Rails5 + new devise version and already provided a fix for it in #69. Can you please confirm whether this fixes the issue for you as well?

jekuno commented 7 years ago

You can test this fix by adding the following line to your Gemfile: gem 'milia', github: 'jekuno/milia', branch: 'issue#68'

And then run bundle install.

jekuno commented 7 years ago

Released https://github.com/jekuno/milia/releases/tag/v1.3.1 Have fun!

ghost commented 7 years ago

@jekuno and @epoplavskis I am learning Ruby On Rails now and I am having problems with rails 5. I tried to enter rails g milia:install --org_email='do-not-reply@example.com' to get milia started after putting gem 'milia' on my Gemfile but I got this in response

/usr/local/rvm/gems/ruby-2.3.3/gems/activesupport-5.0.1/lib/active_support/callbacks.rb:641:in block (2 levels) in skip_callback': Before process_action callback :authenticate_tenant! has not been defined (ArgumentError) from /usr/local/rvm/gems/ruby-2.3.3/gems/activesupport-5.0.1/lib/active_support/callbacks.rb:637:ineach' from /usr/local/rvm/gems/ruby-2.3.3/gems/activesupport-5.0.1/lib/active_support/callbacks.rb:637:in block in skip_callback' from /usr/local/rvm/gems/ruby-2.3.3/gems/activesupport-5.0.1/lib/active_support/callbacks.rb:568:inblock in update_callbacks' from /usr/local/rvm/gems/ruby-2.3.3/gems/activesupport-5.0.1/lib/active_support/callbacks.rb:566:in reverse_each' from /usr/local/rvm/gems/ruby-2.3.3/gems/activesupport-5.0.1/lib/active_support/callbacks.rb:566:inupdate_callbacks' from /usr/local/rvm/gems/ruby-2.3.3/gems/activesupport-5.0.1/lib/active_support/callbacks.rb:636:in skip_callback' from /usr/local/rvm/gems/ruby-2.3.3/gems/actionpack-5.0.1/lib/abstract_controller/callbacks.rb:212:inblock (3 levels) in '

and the list goes on for a while but I didn't want to fill this whole page up with what didn't load right. I was wondering how did you get milia to work on rails 5? What is gemspec? Thanks everyone! Ben

jekuno commented 7 years ago

Do you have before_action :authenticate_tenant! in your application_controller.rb as described in https://github.com/jekuno/milia#application-controller?

ghost commented 7 years ago

No, I have class ApplicationController < ActionController::Base protect_from_forgery with: :exception

end But I'll give that a try, thanks! I just try to follow steps on "The Complete Ruby on Rails Developer By Mashrur Hossain"

marlungu commented 6 years ago

@benvarien I had the same issue and resolved it by adding before_action: authenticate_tenant! in application_controller.rb. Then I changed the method from before_filter to before_action in /config/initializers/devise_permitted_parameters.rb and it worked just fine!

PLSV commented 6 years ago

@marlungu : Hey, Your solution worked!! Thanks a lot