Closed schmunk42 closed 9 years ago
Use migrations instead of an SQL dump.
Yes... we should. But for that we need to confirm the db structure. Are we ok on the db design?
Possible DB Design Areas to Close:
If you note, I had done some slight enhancements to the db design:
last_login_ip
in addition to last_login_on
auth_key
: for rememberMe cookieactivation_key
: for activating user after registrationreset_key
: for resetting the user passwordban_log
(a text field) added to adm_user_profile
. This will be used to store the ban_history for every instance an user was banned (will concatenate and maintain the ban_reason, login_ip, and time). Ideally there should be a ban_log detail table for this... so let know.Latest changes looks great.
Regarding the image upload integration. It may not impact, since we can use is_readable
function, which executes fast, for that purpose.
Btw, in the migrations file we must include the indices code, since there are some DBMS that don't create indices for foreign keys automatically (e.g. PostgreSQL).
@robregonm can you take a stab at doing the IdentityInterface
or you need some help?
I have incorporated quite a few additions.
Have updated the module, models, and added controllers. Check the latest committed code stack.
setConfig
method will generate the default configuration.User
and LoginForm
updated with quite a few methods/functions and all scenarios.BaseController
. Thought is to have these controllers
AccountController
: Will control all user account actions. Currently login
, logout
, and registration
actions have been updated. But it will include few more actions like recovery
, reset
.SocialController
: Social authentication actions. Probably can be merged with the AccountController
in case there are not too many actions in here.ProfileController
: Will help user to manage user profile.AdminController
: Will be used to manage all admin actions.The names above will also help generate friendly url's.
We must soon have the db migration finalized.
nice addition to data structure: User table has a last_login_ip in addition to last_login_on. Shaping up to be a very robust data structure for user.
I noticed the timestamp data type for created_on and updated_on fields. In order for me to get that working properly in my Yii2 project, I had to specify the data type in behaviors by adding:
'value' => new Expression('NOW()'),
to behaviors method and use yii\db\Expression; So you might want to add that to behaviors in the BaseModel. Otherwise it might record the wrong data type or fail entirely.
Working on IdentityInterface
, hopefully I will push tomorrow.
@evercode1 for handling timestamps it depends on the DB type (mysql, sqllite, postgres, nosql etc.) which developers will be using and the Expression may not work for some DB types. Hence, I have now included a module level setting now
with the latest commit. All timestamps will use this setting to generate current time. This is a Closure anonymous function and for this module it defaults to:
'now' => function() {
return date('Y-m-d H:i:s');
}
The above function will return value similar to Expression('NOW()')
and this can be changed by the developer based on his/her need.
Working on
IdentityInterface
, hopefully I will push tomorrow.
@robregonm - great... also check if we need to have a modified User
component based on your experience.
@robregonm please pull the latest code first ... before pushing (as there are updates across)... or can submit a PR.
BTW Migrations are really cool and can u pls make them "ongoing" and not changing the "first" one only?
RBAC/Roles related: We need some idea for the role part for Simple RBAC. @schmunk42 and others where should we do this (in user table or a separate db table or as PHP config)? Also IMO, we may need to keep the module open for any other rbac module to plugin easily.
Sure, atm I'd we just use the RBAC API Yii provides, so it doesn't matter which RBAC module you've plugged into your app. There's already a PHP and DB version available.
Image upload integration: Opening it for third party image upload plugins (not sure there is any impact on db)... but let know
That would be really nice, see also https://github.com/phundament/yii2-extension-requests/issues/9 I used this approach in Yii 1 several times, the imagemanger should provide an input widget with select and upload functionality. IMHO we should do this in a separate extension.
Migrations added and models updated with commit 06f9a68.
Couple of tables and models added:
UserBanLog
MailQueue
Cool, looks nice so far, however, IMHO the table names should be customizable, so, they shouldn't be constants. Take a look at this and this That way, the user will not be forced to use a certain table name, or (in my case, which I want to use this extension within a current ongoing development, that initially developed using yii2-auth) if you have already some table and you just want to migrate to this extension, then the dev only needs to change some field names.
Added tableSettings
to Module and table names are now configurable. Updated migration to use the config. Ref commit 66b744f.
@robregonm, @kartik-v I created an issue for this ... https://github.com/communityii/yii2-user/issues/6 Sorry, I am working through my e-mail from youngest to oldest.
Why should we make the table names customizable, where's the benefit? You can choose a prefix in your db config.
As mentioned in the other issue, this will break if we change the table names later and adds (IMHO unneeded) complexity and a dependency to the migration.
I.e., mishamx/yii-user
also used parts from its module in the migration, which raised a number of problems. Would be cool to keep it simple.
@kartik-v Thank you for the explanation, it's a well-thought out solution.
I think the ability to customize table names is cool, but it will also make support more difficult when developers ask for help tracking down a problem and their tables names are all different.
See #21.
Use migrations instead of an SQL dump.
We can use the configured table prefix, like the advanced app now does: https://github.com/yiisoft/yii2/blob/master/apps/advanced/console/migrations/m130524_201442_init.php#L14
Just as a note: Usually migrations must not be changed after they've been pushed to the repo. There may be exceptions to this rule for initial development, but when we've released a first version, this is a very important convention.