Codeimprovement/migrate to sequelize #132
Conversation
There was a problem hiding this comment.
Looks good to me, haven't tested it locally yet.
What happens with net installations? Should they use the reset.js script? What happens with the old migrations from knex. Maybe some installations aren't yet up to date with changes that have been made in those migrations.
| const { Umzug, SequelizeStorage } = require('umzug'); | ||
|
|
||
| const umzug = new Umzug({ | ||
| migrations: { glob: './migrations/*.js' }, |
There was a problem hiding this comment.
What happens with the old migrations that got removed? Shouldn't they be migrated to use umzug instead of knex?
There was a problem hiding this comment.
I think that's overkill: probably you're running an up to date database, or you're creating a new one.
My guess is that there should not be issues with current users of OpenStad. If there are it's probably less work to fix those then to rewrite these migrations.
Only if there are recent migrations those should be rewritten.
There was a problem hiding this comment.
Thanks for this PR! Looks to be quite a lot of work, and imo it's a lot better to have all services use Sequelize. 👍
I fixed the default JSON values in a commit (3bece85) as discussed. I did not test it thoroughly, but I did test the uniqueCode authType, and I was able to log in locally. Just a few small additions, and I have the same question as @Badmuts about the migrations and how this will work for existing environments (do we use reset.js, or..?)
memoryStorage/accesstokens.js
Outdated
There was a problem hiding this comment.
Why did you move this to memoryStorage? The implementation seems to be database-based instead of in-memory.
Co-authored-by: Daan Rosbergen <Badmuts@users.noreply.github.com>
Co-authored-by: Rudi van Hierden <rudi@draad.nl>
Co-authored-by: Rudi van Hierden <rudi@draad.nl>
Description
The oauth-server uses Bookshelf as ORM. Bookshelf is no longer maintained and should be removed.
Issue reference
Fixes #110
Type of change
This is a code improvement
Documentation
The README will be updated before merge. docs.openstad.org will be updated after merge