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

Do not rely on the user module in migrations #6

Closed
schmunk42 opened this issue Apr 2, 2014 · 16 comments
Closed

Do not rely on the user module in migrations #6

schmunk42 opened this issue Apr 2, 2014 · 16 comments
Assignees

Comments

@schmunk42
Copy link
Contributor

If we decide to change the table names later the migrations will break.
It is also nicer not to use the user module in console config.

@robregonm
Copy link
Collaborator

We can use table prefixes AND custom names (with default names), like I suggested in #4.
So, the developer can choose another name than default in different scenarios. In my personal case, two scenarios:

  1. One existing app which I want migrate to use this extension. If the table name is changed, that will break some code and queries.
  2. Some DBs needs to coexist in the same DB (and sometimes to use each other), so, (in my case) I prefer to use a spanish name (or some custom name) for this particular case in order to have different user tables for different applications.

In the case of the old yii-user extension, I stopped to use it when it started to experience those odd bugs. Another guy and me developed another user extension (which also uses custom table names) and worked perfectly because the user can decide whether to use custom names.

@schmunk42
Copy link
Contributor Author

What would we do, if we decide to rename a table?
Option A: Update the constant, which will make migrations fail on several update scenarios.
Option B: Introduce a new constant and update all the extension code and other parts of the app, which may use it.

Btw: If we introduce constants for tables, I'd name them more verbose, nobody knows what T4 or T3 actually is. Which would lead to USER2 or something similar for Option B.

Totally unviable solutions IMHO. Maybe it's about personal coding style, but I think you can't translate or make changes on that level, unless you're willing to do a lot of work. You also don't translate the PHP Classes and functions, do you?

But let me also propose alternatives:

  1. One existing app which I want migrate to use this extension. If the table name is changed, that will break some code and queries.

Use a database-view.

  1. Some DBs needs to coexist in the same DB (and sometimes to use each other), so, (in my case) I prefer to use a spanish name (or some custom name) for this particular case in order to have different user tables for different applications.

Use a different prefix (db connection) or also a view.

@kartik-v
Copy link
Member

kartik-v commented Apr 3, 2014

I am ok with any approach on this. I had set this to use constant table-names earlier - and then used @robregonm suggestion to parametrize table names in the Module. I am also ok with this as it allows developer to set the names. The only thing in the second case, is module needs to be setup in yii config, before migration is run.

Personally speaking (for self) I would very very rarely change DATABASE table names for a production ready module extension - once I have set it up initially and configured as per my design. So any of the above approach personally would not have a problem for someone like me, once I have set it up initially. But its upto developers. IMO, if one is changing table names quite often - he/she would end up re-creating his/her own module - rather than actually extending from what is available.

I can understand columns/indices getting added or dropped or changed - but changing table names during course of development for me is very rare.

Let's agree on one approach (I am ok for any) and its more from your experience on what you feel is needed by most developers.

@kartik-v
Copy link
Member

kartik-v commented Apr 3, 2014

@schmunk42: Btw: If we introduce constants for tables, I'd name them more verbose, nobody knows what T4 or T3 actually is. Which would lead to USER2 or something similar for Option B.

I had setup this for table constants. Do we still need this to be different?

@nineinchnick
Copy link
Collaborator

I keep it simple. I just advice users to copy the migrations to their project's migration dir and adjust timestamps in names to match their timeline.

Actually, I will forever advocate not forcing users to specific structures, just providing full ready to use examples that can and should be adjusted.

@schmunk42
Copy link
Contributor Author

I am sorry to say that, but parametrizing table names like this is broken by design. I also don't want to change table names, but that's what migrations are also for - and it happens.

It's really complicated to actually find the table name for a model, because you first have to look up tableName() in the model which is stored in a constant in the module.

Let me give you an example:

  • Release 0.1: TBL_USER = "user"
  • Release 0.2: TBL_USER = "user"
  • Release 0.3: TBL_USER = "people"

Let's assume all three releases have migrations.
If you're on 0.1 and did NOT update to 0.2, but now running an update to 0.3 the migration from 0.2 will fail because it will use the table name you have defined in your source code in version 0.3, which doesn't exist in 0.2.

If we'd used static prefixes in the first place, we'd have exactly this problem now.

See also https://github.com/yiisoft/yii2/blob/master/docs/guide/extensions.md#working-with-database - it's essentially the same as "Do not use Active Record models in your migrations.".

Let's keep it simple.

@kartik-v
Copy link
Member

kartik-v commented Apr 3, 2014

@schmunk42 I agree with all the points from all of you which are valid (only that I have rarely experienced changing of table names during course of design) .

So to summarize how should we simplify it ... i..e what is the code to be included in the migrations

  • do we need to name the table as '{{%user}}' or
  • do we refer to a constant Module::TBL_USER which is set to '{{%user}}'

Based on this, would make the changes.

@nineinchnick
Copy link
Collaborator

Why would you need a constant with the table name? Can't this be read from the User model?

@kartik-v
Copy link
Member

kartik-v commented Apr 3, 2014

Ok to ask again.. this kind of violates point # 3 of using ActiveRecord models mentioned before.

But even if we use User::tableName() ... what should this function's return value be? Do we set it to '{{%user}}' or Module::TBL_USER.

I am asking this question again only because the topic is around changing of table names during the development process. Should developers change table names across all models or extend the module and change the constants.

PS: If you ask me... I am ok with any option - because I personally may not be needing to change table names - once its set initially.

Some great inputs here. This can be closed based on your experiences guys.

@schmunk42
Copy link
Contributor Author

We set is as {{%user}} so you can still choose a prefix if you've conflicts with existing tables. But we have no variables or constants involved in our migrations.

@nineinchnick No, it can't. See the link to the Yii 2 extension guidelines.

@nineinchnick
Copy link
Collaborator

@schmunk42 I agree, but I'd only use the constant in the migrations and User::tableName(). In every other place the table name should be read from the model.

@kartik-v
Copy link
Member

kartik-v commented Apr 3, 2014

Ok if I understand from you guys... here is the summary then. For a table named '{{%user}}' - these are the places where this will be hard-coded... so any change for every table name needs to be done in the first 2 places below:

  1. In migration script - the table will be named '{{%user}}'
  2. In User model - the tableName() function will return '{{%user}}'
  3. Any other place - we get the tableName as User::tableName()

Are we ok with this?

So for folks who will change table names frequently during development... as an example... if there are 10 tables ... then developers need to change the name in at least 20 places (migration and models). This is for every instance they need to change the table names.

@schmunk42
Copy link
Contributor Author

@kartik-v Yes, I am OK with 1., 2. and 3.

@nineinchnick Do not use a constant in the migrations, it just makes no sense, complicates things and has more disadvantages. As mentioned beforehand, there are some things you can not change or define dynamically, such as ClassNames or functions().

@kartik-v
Copy link
Member

kartik-v commented Apr 3, 2014

Closed via commit 70df716.

@kartik-v kartik-v closed this as completed Apr 3, 2014
@schmunk42
Copy link
Contributor Author

Nice!

@robregonm
Copy link
Collaborator

Looks great!

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

No branches or pull requests

4 participants