CHE-4372; add unique constraing for factory name + user #4521
Conversation
| @@ -133,8 +141,12 @@ public Factory updateFactory(Factory update, Set<FactoryImage> images) throws Co | |||
| ServerException { | |||
| requireNonNull(update); | |||
| final AuthorImpl creator = factoryDao.getById(update.getId()).getCreator(); | |||
| FactoryImpl updateImpl = new FactoryImpl(update, images); | |||
| if (isNullOrEmpty(updateImpl.getName())) { | |||
There was a problem hiding this comment.
It would be better not to generate new name when update doesn't contain it. Let's set old name when it is null or empty in update.
There was a problem hiding this comment.
In this case you cant re-assign the name to another factory. I don't know if it's good.
There was a problem hiding this comment.
Why you can't re-assign the name? You can send name in update then name will be updated. Or you can miss it, then old name will be used (but not generated new)
There was a problem hiding this comment.
For me it looks stupid, if you deleting the factory name on Dashb, and it returns back after update :). But seems others are with you, so ok.
There was a problem hiding this comment.
So seems it must not be generated on update at all.
| -- from Codenvy S.A.. | ||
| -- | ||
|
|
||
| CREATE UNIQUE INDEX index_name_plus_userid ON che_factory (user_id, name); |
There was a problem hiding this comment.
index_name_plus_userid -> index_che_factory_name_user_id
| -- | ||
|
|
||
| UPDATE che_factory | ||
| SET name = concat('f', right(id, 9)) WHERE name IS NULL OR name = ''; |
There was a problem hiding this comment.
Script name does not match the content
|
Build # 2256 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2256/ to view the results. |
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2260/ |
|
|
||
| -- Make names unique for the same user, e.g if there is more than one factory with same name and user, | ||
| -- leave first one and rename others. | ||
| WITH dupes AS |
There was a problem hiding this comment.
Can we do this migration on codenvy tables before we move factory to che?
Because it's weird to see such a postgresql specific migration in che, while che doesn't have factories yet.
Moreover it allows to define unique constraint next to che_factory declaration.
There was a problem hiding this comment.
Yes I can move naming uniques into 1.1 in codenvy, then 1.2 will do the migration, and costraint will be just in che_factory table definition.
|
Build # 2267 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2267/ to view the results. |
What does this PR do?
Makes factory name mandatory on database level (auto generated if not set by user)
What issues does this PR fix or reference?
#4372
Changelog
Factory name made mandatory on database level
Release Notes
n/a
Docs PR
N/A