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

#362 fix bug Foreign key constraint is incorrectly formed #363

Conversation

myvo
Copy link

@myvo myvo commented Sep 17, 2018

The issue #362 is fixed.
After checking in MySQL by command SHOW ENGINE INNODB STATUS;, I got the reason.

LATEST FOREIGN KEY ERROR
------------------------
2018-09-17 10:40:11 0x7f2eb41b9700 Error in foreign key constraint of table `dbname`.`Book`:
Alter  table `lbmysql`.`Book` with foreign key constraint failed. Parse error in ' FOREIGN KEY (`author_id`) REFERENCES `Author`(`aId`)' near '`aId`)'.

We forgot get the MySQL column name before building the Foreign key.

@slnode
Copy link

slnode commented Sep 17, 2018

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

@myvo myvo force-pushed the bug-362-foreign-key-constraint-incorrectly-formed branch from 625079b to c0903af Compare September 17, 2018 08:47
@myvo myvo force-pushed the bug-362-foreign-key-constraint-incorrectly-formed branch from c0903af to d2870ec Compare September 17, 2018 08:56
@dhmlau
Copy link
Member

dhmlau commented Sep 17, 2018

@slnode test please

Copy link
Contributor

@b-admike b-admike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@myvo Thank you for your contribution! Can you please add some test(s) to verify your patch and prevent regressions in the future?

Copy link
Contributor

@jannyHou jannyHou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@myvo Thank you for fixing the issue! Your code LGTM, can you also add a test case for it?

@myvo
Copy link
Author

myvo commented Sep 30, 2018

I've just added new test case. Please help me review it again

@dhmlau
Copy link
Member

dhmlau commented Nov 12, 2018

@slnode test please

@dhmlau
Copy link
Member

dhmlau commented Nov 13, 2018

@myvo , sorry about the late response. Were you able to get the local tests all passed? Jenkins has one test case failing. Could you please take a look if it's caused by your PR? Thanks.

 1) mysql imported features include should report errors if the PK is excluded:
14:25:19      Uncaught AssertionError: expected null to exist
14:25:19       at node_modules/loopback-datasource-juggler/test/include.test.js:81:14
14:25:19       at node_modules/loopback-datasource-juggler/lib/dao.js:2202:9
14:25:19       at node_modules/async/dist/async.js:1140:9
14:25:19       at node_modules/async/dist/async.js:473:16
14:25:19       at iteratorCallback (node_modules/async/dist/async.js:1064:13)
14:25:19       at node_modules/async/dist/async.js:969:16
14:25:19       at node_modules/async/dist/async.js:1137:13
14:25:19       at buildResult (node_modules/loopback-datasource-juggler/lib/dao.js:2168:11)
14:25:19       at node_modules/loopback-datasource-juggler/lib/dao.js:2182:13
14:25:19       at doNotify (node_modules/loopback-datasource-juggler/lib/observer.js:155:49)
14:25:19       at doNotify (node_modules/loopback-datasource-juggler/lib/observer.js:155:49)

@dhmlau
Copy link
Member

dhmlau commented Dec 18, 2018

@slnode test please

2 similar comments
@b-admike
Copy link
Contributor

@slnode test please

@b-admike
Copy link
Contributor

@slnode test please

@dhmlau
Copy link
Member

dhmlau commented Jan 28, 2019

@slnode test please
seems like the juggler issue has been fixed by @b-admike. Kicking off the tests again.

@myvo
Copy link
Author

myvo commented Feb 11, 2019

Hi @dhmlau
Please only test file test/mysql.autoupdate.test.js. I've just added the testing code into this file.

@stale
Copy link

stale bot commented Jun 18, 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 Jun 18, 2019
@stale
Copy link

stale bot commented Jul 2, 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 Jul 2, 2019
@agnes512 agnes512 reopened this Aug 18, 2020
@stale stale bot removed the stale label Aug 18, 2020
@agnes512 agnes512 requested review from agnes512 and removed request for virkt25 August 18, 2020 17:49
@dhmlau
Copy link
Member

dhmlau commented Aug 21, 2020

We just switch the contribution method from CLA to DCO, making your contribution easier in the future. Please sign the commits with DCO by amending your commit messages with -s flag and push the changes again. If you're using the command line, you can:

git commit --amend -s
git push --force-with-lease

Please refer to this docs page for details. Thanks!

For more questions, please join our Slack workspace - #loopback-contributors.

@dhmlau dhmlau added the community-contribution Patches contributed by community label Aug 21, 2020
@agnes512 agnes512 closed this Aug 21, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants