#125 Objects SQL definition #15
Conversation
| const [createScript] = await dbConn.getTableCreateScript('users'); | ||
|
|
||
| if (dbClient === 'mysql') { | ||
| expect(createScript).to.contain('CREATE TABLE `users` (\n' + |
There was a problem hiding this comment.
Since we are using ES6. Couldn't you use template string instead of these concatenations? Template string keeps the code more readable.
There was a problem hiding this comment.
I can, but than the code won't be indented, since it will count tab spaces as a part of the template string, so the test will fail. I find that way less readable, since it's moved to the left, but if others think differently I'll change it, no problem 👍
There was a problem hiding this comment.
You are right. If it depends on the output format then template string is a bad choice. Is gonna look to weird with all content moved to left.
|
I rather let all the database logic inside sqlectron-core. If all the clients really have the same script for those listed in the screenshot I guess you could put the code directly inside the client.js. |
|
Thanks on the review! Well CREATE script is bit different for each DB, but that's already implemented now with getTableCreateScript method. I'll put other scripts that have the same SQL for each client (ex. SELECT script) directly inside the client.js than :) |
|
@maxcnunes I added generating SELECT/INSERT/UPDATE/DELETE scripts directly in client.js, as they are SQL standard and work the same for all three DB engines we support (I tested manually just to be sure). BTW seems there's problem with |
|
Sometimes the build on appveyor breaks for no reason. Running the build again usually solves the problem. 😕 |
|
@maxcnunes Created account on AppVeyor, it's linked to GitHub acc, so you can add me. Well if the build passes I guess that is pretty much it for this PR :) |
|
I tried fix the build on appveyor. But couldn't get that working. That is very weird because we have not changed anything in the appveyor configuration and that has just stopped working at all. |
|
Hey @maxcnunes , it seems that guys at AppVeyor messed it up while installing MySQL 5.7 haha. Check this out AppVeyor MySQL 5.7 issue. I think problem should be solved with single commit 741b868 you made. |
|
Everything runs smoothly now! 😄 I'll leave it up to you @maxcnunes to check it all once again and merge it into master. |
|
Glad you figured out how to solve the problem on Appveyor 👏 👏 👏 👏 |
|
Published as version 3.5.0. Thanks @BornaP !!! |
PR based on this issue.
This is still a working version. PR is opened for further discussion. Here's what's implemented:
getTableCreateScriptmethod. While it was trivial to implement this for MySQL, I reused (and slightly redefined) SQL scripts found on stackoverflow (links PostgreSQL, SQL Server).Now, similar can be done for Views (in fact it's the same function for MySQL). But what I had also in mind is having context menu (on right clicking a table) with SELECT/INSERT/UPDATE/DELETE scripts, similar to pgAdmin:
At first I thought about including this to back-end, but on the second thought, these are entirely the same for each DB server since that's a SQL standard. So maybe generating scripts logic could be included directly in sqlectron-gui or in one method that returns object containing this script's for specified table. If we will choose second option I'll implement it in this same PR.