Refactor admin as a component (part of Issue 2490) #2492
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change app.js to seed database in promise chain before startServer, move seeding conditional to seed.js and wrap in function.
Previously, .sync() was being called once in server/config/seed.js and once during sqldb.sequelize.sync() in server/app.js. As a result, duplicate queries were being sent to the database if the tables needed to be defined causing an error when it tries to define constraints a second time (like for an identity column). Seed.js is wrapped in a function and contains the conditional to determine whether to seed the database. This makes it easy to include in the promise chain under sqldb.sequelize.sync().
Previously, .sync() was being called once in server/config/seed.js and once during sqldb.sequelize.sync() in server/app.js. As a result, duplicate queries were being sent to the database if the tables needed to be defined causing an error when it tries to define constraints a second time (like for an identity column). Seed.js is wrapped in a function and contains the conditional to determine whether to seed the database. This makes it easy to include in the promise chain under sqldb.sequelize.sync().
Previously, admin was implemented as a loosely coupled controller. Converting it to a full component makes it consistent with main and generated components.
For projects with auth=Y, users were not being populated for those using Sequelize because of the return on Think.bulkCreate. The return was changed to a Sequelize.Promise.all so that all the promises are properly managed.
|
I think you should use the same model as the main component, and have the admin controller class in the same |
|
Also, you'll need to pull your latest commit for |
|
I've added you as a collaborator, so you can make a branch directly on this repository if you'd like. |
Previously, admin was implemented as a loosely coupled controller. Converting it to a full component makes it consistent with main and generated components.
|
Thanks for adding me as a contributor. I pushed an additional commit that put the controller code back into admin.module.js. Also added the corrected seed(model).js. |
|
Did a git revert f6da78f to undo the changes to templates/app/server/config/seed(models).js. Please let me know if that was what you were looking for. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
generator-angular-fullstack$ npm test)First modification towards 2490.. Wasn't sure whether this should go in canary or master. BTW, I found a problem in seed.js from my previous PR and will submit another to correct it.