Adds index on _Role name property #3586
Conversation
natanrolnik
requested review from
TylerBrock,
flovilmart,
acinader and
cherukumilli
| .then(() => this.adapter.ensureUniqueness('_Role', requiredRoleFields, ['name'])) | ||
| .catch(error => { | ||
| logger.warn('Unable to ensure uniqueness for role name: ', error); | ||
| return Promise.reject(error); |
There was a problem hiding this comment.
i think you can safely just return the error here instead of returning a rejected error, but don't take my word for it.....
There was a problem hiding this comment.
for continuations of errors, the pattern should be throw error
|
@natanrolnik do the unit tests pass locally? when i run locally, the same tests fail as on travis and they are related to roles and schemas....suspicious.... ;)
|
natanrolnik
force-pushed
the
dont-allow-duplicate-roles
branch
from
5b5bfff to
e755d34
Compare
|
@natanrolnik updated the pull request - view changes |
In order to avoid having different _Role objects with the same name, adding an index on the name property of _Role is necessary. Fixes parse-community#3579
natanrolnik
force-pushed
the
dont-allow-duplicate-roles
branch
from
e755d34 to
c3e3584
Compare
|
@natanrolnik updated the pull request - view changes |
| .then(() => this.adapter.ensureUniqueness('_Role', requiredRoleFields, ['name'])) | ||
| .catch(error => { | ||
| logger.warn('Unable to ensure uniqueness for role name: ', error); | ||
| return Promise.reject(error); |
There was a problem hiding this comment.
for continuations of errors, the pattern should be throw error
|
@natanrolnik updated the pull request - view changes |
| @@ -494,7 +494,8 @@ describe('SchemaController', () => { | |||
|
|
|||
| it('creates non-custom classes which include relation field', done => { | |||
| config.database.loadSchema() | |||
| .then(schema => schema.addClassIfNotExists('_Role', {})) | |||
There was a problem hiding this comment.
this should still be OK no? or does the addClassIfNoExists fail if the class exists?
There was a problem hiding this comment.
That's what I was expecting as well, but I was getting Class _Role already exists.
There was a problem hiding this comment.
We could add a test, though, that makes sure addClassIfNotExists returns the existing schema, instead of an error, if the class already exists.
|
@natanrolnik updated the pull request - view changes |
|
@acinader, it's looking good for me. Anything on your side? |
|
👍 lgtm! |
In order to avoid having different
_Roleobjects with the same name, adding an index on the name property of_Roleis necessary.In apps migrated from Parse.com, the index ensuring uniqueness on
_Rolenames already existed. This issue only happens on new Parse Server apps.Fixes #3579