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

Fix - fail to honor non-unique property constraint #389

Closed
wants to merge 1 commit into from

Conversation

ewrayjohnson
Copy link

@ewrayjohnson ewrayjohnson commented Aug 11, 2019

Connector does not honor non-unique index definition for a property declaration in model json file on automigrate/autoupdate. Error is generated on autoupdate when adding this non-unique data already exists. Error is generated when instances are inserted that contain a value for the property that already exists in the database.

Example model json file snipet:

"properties": [ 
  "email": {
    "type": "String",
    "index": {
      "unique": false
    }
  }
]

error generated: "could not create unique index "[modelname] _email_idx"

Description

Related issues

#336

  • connect to <link_to_referenced_issue>

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

Connector does not honor non-unique constraint definition for a property declaration in model json file on automigrate.  
Example model json file snipet:
"properties": [ 
  "email": {
    "type": "String",
    "index": {
      "unique": false
    }
  }
]

error generated: "could not create unique index "[modelname] _email_idx"
@slnode
Copy link

slnode commented Aug 11, 2019

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@dhmlau
Copy link
Member

dhmlau commented Aug 12, 2019

@ewrayjohnson, thanks for your PR. Could you please fix the commit linter error?

**************************************************
**
**  Linting commit logs
**
**  1 problems found:
**    8b0cafc - Fix - fail to honor non-unique property constraint: line 3 longer than 72 characters.
**
**************************************************

For details, please refer to our commit message guideline: https://loopback.io/doc/en/contrib/git-commit-messages.html. Thanks.

@dhmlau
Copy link
Member

dhmlau commented Aug 12, 2019

Also, could you please add some test cases regarding the changes?

@dhmlau dhmlau added the community-contribution Patches contributed by community label Aug 12, 2019
@stale
Copy link

stale bot commented Oct 11, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 11, 2019
@ewrayjohnson
Copy link
Author

I will fix the commit linter error and add a test case.

@stale stale bot removed the stale label Oct 11, 2019
@bajtos
Copy link
Member

bajtos commented Oct 15, 2019

FYI, in loopbackio/loopback-next#2712, you can find a description of different ways how a unique index can be defined and also what how we want to unify these different options in the future. See Proposal and Detailed description of the current syntax.

I am not sure how much is it affecting your pull request. At minimum, I think you can use the detailed description to help you write test cases.

To make it clear, I am not asking you to fix all problems related to migration of UNIQUE indices.

I'd just like to better understand which parts will be fixed by your pull request and which parts will stay broken.

@bajtos
Copy link
Member

bajtos commented Oct 15, 2019

@ewrayjohnson Most importantly, thank you for taking time to fix the issue and sending the pull request! ❤️

@stale
Copy link

stale bot commented Dec 14, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 14, 2019
@stale
Copy link

stale bot commented Dec 28, 2019

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

@stale stale bot closed this Dec 28, 2019
@ewrayjohnson
Copy link
Author

I am working on creating tests to show the bug and confirm the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution Patches contributed by community stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants