Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed to allow a different user class from User. And not step on existing current_user methods. #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

candland
Copy link
Contributor

Changed current_user to kublog_current_user so that it doesn't step on an existing current_user method. Changed the user association to be polymorphic so that classes other then user can be used.

  • this will be a breaking change in the application controller. Any current_user method will need to be changed or added to kublog_current_user.

    def kublog_current_user
    current_user
    end

…n an existing current_user method. Changed the user association to be polymorphic so that classes other then user can be used.
@adriancuadros
Copy link
Member

Hello Dusty,

Since this is a mountable Engine, I think every method under the kublog
namespace will be safe from clashing with the methods in your general
application controller. Is there a specific case i'm missing, if so I think
it would definitely be a valid change.

On Tue, Apr 17, 2012 at 10:26 PM, Dusty Candland <
reply@reply.github.com

wrote:

Changed current_user to kublog_current_user so that it doesn't step on an
existing current_user method. Changed the user association to be
polymorphic so that classes other then user can be used.

  • this will be a breaking change in the application controller. Any
    current_user method will need to be changed or added to kublog_current_user.

    def kublog_current_user
    current_user
    end

You can merge this Pull Request by running:

git pull https://github.com/candland/kublog alias_current_user

Or you can view, comment on it, or merge it online at:

#5

-- Commit Summary --

  • Changed current_user to kublog_current_user so that it doesn't step on
    an existing current_user method. Changed the user association to be
    polymorphic so that classes other then user can be used.

-- File Changes --

M app/controllers/kublog/application_controller.rb (6)
M app/controllers/kublog/comments_controller.rb (2)
M app/controllers/kublog/posts_controller.rb (3)
M app/models/kublog/comment.rb (2)
M app/models/kublog/post.rb (4)
M app/views/kublog/posts/show.html.erb (4)
M db/migrate/20110816211552_create_kublog_posts.rb (2)
M db/migrate/20110822194341_create_kublog_comments.rb (2)
M lib/kublog/author.rb (4)
M lib/kublog/user_integration/common.rb (12)
M spec/dummy/app/controllers/application_controller.rb (12)
M spec/dummy/app/controllers/sessions_controller.rb (2)
M spec/dummy/app/controllers/users_controller.rb (2)
M spec/dummy/app/views/layouts/_account.html.erb (2)

-- Patch Links --

https://github.com/innku/kublog/pull/5.patch
https://github.com/innku/kublog/pull/5.diff


Reply to this email directly or view it on GitHub:
#5

Adrian Cuadros
Innku
(81) 83870544
Monterrey, Nuevo León

@candland
Copy link
Contributor Author

Hi Adrian,

I think that is generally the case too, except for the current_user method
that devise sets up. In this application, I had a devise User class already
setup but stored in Mongo DB. It's for general uses of the site anyway, so
I created a devise Writer to use. I might not have setup something
correctly, but the engine was calling the devise current_user, not the one
I defined in the Kublog::AppicationController, which in turn called
current_writer. An edge case for sure.

I have another app like this, but since the User is using a relational DB,
I can just add a role for blogging. However, I also have another app that
is exactly the same with a User class in Mongo DB.

I would rather have made the models support a Mongoid, but I'm not sure how
to do that or any sample code to look at.

Thoughts?

Thanks for considering these changes!

Dusty

On Fri, Apr 20, 2012 at 8:57 AM, Adrian Cuadros <
reply@reply.github.com

wrote:

Hello Dusty,

Since this is a mountable Engine, I think every method under the kublog
namespace will be safe from clashing with the methods in your general
application controller. Is there a specific case i'm missing, if so I think
it would definitely be a valid change.

On Tue, Apr 17, 2012 at 10:26 PM, Dusty Candland <
reply@reply.github.com

wrote:

Changed current_user to kublog_current_user so that it doesn't step on an
existing current_user method. Changed the user association to be
polymorphic so that classes other then user can be used.

  • this will be a breaking change in the application controller. Any
    current_user method will need to be changed or added to
    kublog_current_user.

    def kublog_current_user
    current_user
    end

You can merge this Pull Request by running:

git pull https://github.com/candland/kublog alias_current_user

Or you can view, comment on it, or merge it online at:

#5

-- Commit Summary --

  • Changed current_user to kublog_current_user so that it doesn't step on
    an existing current_user method. Changed the user association to be
    polymorphic so that classes other then user can be used.

-- File Changes --

M app/controllers/kublog/application_controller.rb (6)
M app/controllers/kublog/comments_controller.rb (2)
M app/controllers/kublog/posts_controller.rb (3)
M app/models/kublog/comment.rb (2)
M app/models/kublog/post.rb (4)
M app/views/kublog/posts/show.html.erb (4)
M db/migrate/20110816211552_create_kublog_posts.rb (2)
M db/migrate/20110822194341_create_kublog_comments.rb (2)
M lib/kublog/author.rb (4)
M lib/kublog/user_integration/common.rb (12)
M spec/dummy/app/controllers/application_controller.rb (12)
M spec/dummy/app/controllers/sessions_controller.rb (2)
M spec/dummy/app/controllers/users_controller.rb (2)
M spec/dummy/app/views/layouts/_account.html.erb (2)

-- Patch Links --

https://github.com/innku/kublog/pull/5.patch
https://github.com/innku/kublog/pull/5.diff


Reply to this email directly or view it on GitHub:
#5

Adrian Cuadros
Innku
(81) 83870544
Monterrey, Nuevo León


Reply to this email directly or view it on GitHub:
#5 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants