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

Feature add foreign key on update on delete trigger config #370

Merged
merged 3 commits into from Apr 26, 2019

Conversation

Projects
None yet
5 participants
@HugoPoi
Copy link
Contributor

commented Dec 4, 2018

Description

Add possibility to configure the foreign key constraint with 2 optional triggers.

  • add onUpdate foreign key settings
  • add onDelete foreign key settings
  • fix a issue, foreign key index try to be deleted on autoupdate
  • fix a issue, index being recreate each migration when declared directly on property

Related issues

  • #366
  • I touch code in the same section than the PR #363

Checklist

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

@HugoPoi HugoPoi requested review from b-admike, dhmlau, jannyHou and virkt25 as code owners Dec 4, 2018

@slnode

This comment has been minimized.

Copy link

commented Dec 4, 2018

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

@HugoPoi HugoPoi force-pushed the HugoPoi:feature/foreign-key-on-config branch from 08f6788 to 67345bd Dec 4, 2018

@jannyHou

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2018

@slnode test please

@HugoPoi

This comment has been minimized.

Copy link
Contributor Author

commented Dec 7, 2018

I'm not sure but some test in datasource juggler in windows node 6.x failed, is it related with my changes ?

@derdeka

This comment has been minimized.

Copy link

commented Mar 5, 2019

@dhmlau @virkt25 @jannyHou @b-admike
Is this project dead / end of live as no one is reviewing the PR?
This PR solves several bugs in loopback-connector-mysql which prevent me to use loopback at all.

@jannyHou
Copy link
Contributor

left a comment

@HugoPoi Sorry for the late reply. I left a comment and triggered a new build for CI.

});
rawConstraints.push(match);
}
} while (match != null);

This comment has been minimized.

Copy link
@jannyHou

jannyHou Mar 28, 2019

Contributor

the while loop is a little bit risky to me, what if the statement doesn't contain the constraint? Maybe change the logic to stop when reaches the last line of the statement.

This comment has been minimized.

Copy link
@HugoPoi

HugoPoi Mar 28, 2019

Author Contributor

the if statement line 54 in the while protect for no match in the CREATE TABLE, I think the tricky part is when someone rework my code later, because the iteration over the Regex is not obvious. Maybe add some comment before the line 52 to explain the iteration on the multiple match with RegExp object internal pointer.
Or is there a other way to iterate with complex RegExp ?

@jannyHou

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

@slnode test please

@dhmlau

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2019

@jannyHou, the CI is all green. Do you think this PR is good to merge? Thanks.

@dhmlau dhmlau referenced this pull request Apr 1, 2019

Closed

Monthly Milestone - April 2019 🌿🐰☔️ #2669

20 of 46 tasks complete

@jannyHou jannyHou force-pushed the HugoPoi:feature/foreign-key-on-config branch from 67345bd to 24fe635 Apr 26, 2019

@HugoPoi HugoPoi requested a review from emonddr as a code owner Apr 26, 2019

@jannyHou

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2019

@slnode test please

@jannyHou
Copy link
Contributor

left a comment

@HugoPoi Thank you for the contribution 🎉 !!
LGTM. The travis failure is not related to your PR. I will land it when the jenkins tests pass.

@jannyHou jannyHou merged commit 6218665 into strongloop:master Apr 26, 2019

8 of 9 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Commit Linter commits are all properly formatted
Details
PR Linter PR is up to date
Details
clahub All contributors have signed the Contributor License Agreement.
Details
loopback-connector-mysql Success! (24fe635)
Details
loopback-connector-mysql/node=4.x,os=windows Success! (24fe635)
Details
loopback-connector-mysql/node=6.x,os=windows Success! (24fe635)
Details
pr-builder
Details
security/snyk - package.json (StrongLoop) No manifest changes detected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.