MongoDB Support

[mathieu-rasse-dubbing]

Here a support for MongoDB user's and some bug correction.

Thanks a lot for this amazing plugin !

[beeman]

@mathieu-rasse-dubbing thanks for testing my plugin and committing your MongoDB support! I will have to check the implementation before I merge this pull request.

Some comments:

• I think that this change should use CakePHP's translation system.
• I see you are using $notification['Sender']['User']['email'] instead of $notification['Sender']['email'], is this something really MongoDB specific, or is it possible to update your model to reflect this change? In this case the sender might be a user, but you can imagine other parts of the application being the sender as well...
• Another thing is linking the UsersController directly, it might be better practice to make this controller variable (e.g., check the Auth-component to see what controller to use).

Overall I'm looking to have an implementation that is as flexible and clean as possible, with all the options being configurable.

I'll install a MongoDB instance later on and check how this works. Thanks a lot for your commit!!

[mathieu-rasse-dubbing]

Hi Bram,

Thanks for your answer. It’s my first time I request a pull and it’s nice that I’m talking with someone interested.

You’re right concerning the modifications. I’ve made this modifications to be compliant with my project and I pushed it to help you with your development, but I can be more involved in your project if you’re interested.

Best regards,

Mathieu.

De : Bram Borggreve [mailto:notifications@github.com]
Envoyé : mercredi 19 juin 2013 16:17
À : beeman/cakephp-bootstrap-notifications
Cc : Mathieu Rasse
Objet : Re: [cakephp-bootstrap-notifications] MongoDB Support (#2)

@mathieu-rasse-dubbinghttps://github.com/mathieu-rasse-dubbing thanks for testing my plugin and committing your MongoDB support! I will have to check the implementation before I merge this pull request.

Some comments:

• I think that this changehttps://github.com/mathieu-rasse-dubbing/cakephp-bootstrap-notifications/commit/c6b1a88237d1d5008cfa72e692e028ccdc9224f8#L2L9 should use CakePHP's translation system.
• I see you are using $notification['Sender']['User']['email'] instead of $notification['Sender']['email'], is this something really MongoDB specific, or is it possible to update your model to reflect this change? In this case the sender might be a user, but you can imagine other parts of the application being the sender as well...
• Another thing is linking the UsersController directly, it might be better practice to make this controller variable (e.g., check the Auth-component to see what controller to use).

Overall I'm looking to have an implementation that is as flexible and clean as possible, with all the options being configurable.

I'll install a MongoDB instance later on and check how this works. Thanks a lot for your commit!!

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2#issuecomment-19686701.