-
Notifications
You must be signed in to change notification settings - Fork 633
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
SQL Server migrations fixes and improvements #843
base: master
Are you sure you want to change the base?
Conversation
tsdogs
commented
Jan 18, 2017
Q | A |
---|---|
Is bugfix? | yes |
New feature? | yes |
Breaks BC? | don't think so |
Fixed issues | When doing migration up and down SQL server constraints tend to block operations on tables and columns. This should fix all the issues. |
@@ -36,7 +36,7 @@ public function down() | |||
if ($this->dbType == 'sqlsrv') { | |||
$this->dropIndex('{{%user_unique_username}}', '{{%user}}'); | |||
} | |||
$this->alterColumn('{{%user}}', 'username', $this->string(255)->null()); | |||
$this->alterColumn('{{%user}}', 'username', $this->string(255), ' NULL'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this line is changed, and shouldn't it be .
instead of ,
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some strange reasons SQL server does not accept the ->null(), it creates a strange SQL like
DEFAULT NULL NULL string which is not correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any news on this one? Should I change something for you to include?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tsdogs you should change ,
to .
in the line I was talking about, because tests won't pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmeroff ok should be fixed now. I tested all up and down on SQL Server 2012 and it works correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dunno why but tests still fail? though it's complaining about token table and last_login fields!?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, there was a very ugly bug in the check of the db type. (though for some strange reason in my test environment migration with mariadb worked without errors).
Have you had time to check this? |
ImpersonateUser fix
Updated italian translation
Not all timezones have hour offset, so we can not just divide offset
Include username & url in welcome email
Some more italian translations
Fix for accented letters in italian translation
Allow configuration of different database connection for all models.
* Tests no longer use deprecated package. Updated shields in README. * Fixed typos * Dont install codeception globally in travis. * Use /tmp for assets
* add ability to filter by permission in case yii2-rbac module is used * return empty array when there is no authManager * allow to use anything that implements ManagerInterface; show filter in grid when it is DbManager * code style
…me() gives an error on PHP 5.6). Fix for bug where only admin users can be impersonated (while it should be that only NON-admin users can be impersonated).