Skip to content

Add missing migration for permissions#757

Merged
SISheogorath merged 1 commit intohackmdio:masterfrom
SISheogorath:fix/migration
Mar 17, 2018
Merged

Add missing migration for permissions#757
SISheogorath merged 1 commit intohackmdio:masterfrom
SISheogorath:fix/migration

Conversation

@SISheogorath
Copy link
Copy Markdown
Contributor

Potential fix for #752

Needs some testing on Postgres as well as MySQL.

@SISheogorath SISheogorath added bug Something isn't working WIP Do not merge untested This has not be verified to be working database/sequelize Somehow this is related to the database or ORM labels Mar 6, 2018
@SISheogorath SISheogorath requested a review from ccoenen March 6, 2018 15:25
@ccoenen
Copy link
Copy Markdown
Contributor

ccoenen commented Mar 6, 2018

/var/www/hackmd$ NODE_ENV=production node_modules/.bin/sequelize db:migrate

Sequelize [Node: 9.7.1, CLI: 2.8.0, ORM: 3.30.4]

Parsed url mysql://hackmd:*****@localhost:3306/hackmd
== 20180306150303-fix-enum: migrating =======
ReferenceError: DataTypes is not defined
    at Object.up (/var/www/hackmd/lib/migrations/20180306150303-fix-enum.js:5:63)
    at constructor._exec (/var/www/hackmd/node_modules/umzug/lib/migration.js:104:23)
    at constructor.up (/var/www/hackmd/node_modules/umzug/lib/migration.js:69:17)
    at constructor.<anonymous> (/var/www/hackmd/node_modules/umzug/index.js:124:28)
    at PassThroughHandlerContext.finallyHandler (/var/www/hackmd/node_modules/bluebird/js/release/finally.js:57:23)
    at PassThroughHandlerContext.tryCatcher (/var/www/hackmd/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:693:18)
    at Promise._fulfill (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:638:18)
    at Promise._resolveCallback (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:432:57)
    at Promise._settlePromiseFromHandler (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:524:17)
    at Promise._settlePromise (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/var/www/hackmd/node_modules/bluebird/js/release/promise.js:693:18)

Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
@ccoenen
Copy link
Copy Markdown
Contributor

ccoenen commented Mar 6, 2018

this worked for me on 1.0.1-ce

MariaDB [(none)]> show fields from hackmd.Notes;
+------------------+--------------------------------------------------------------------+------+-----+---------+-------+
| Field            | Type                                                               | Null | Key | Default | Extra |
+------------------+--------------------------------------------------------------------+------+-----+---------+-------+
| id               | char(36)                                                           | NO   | PRI |         |       |
| shortid          | varchar(255)                                                       | NO   | UNI | NULL    |       |
| alias            | varchar(255)                                                       | YES  | UNI | NULL    |       |
| permission       | enum('freely','editable','limited','locked','protected','private') | YES  |     | NULL    |       |
| viewcount        | int(11)                                                            | NO   |     | 0       |       |
| title            | text                                                               | YES  |     | NULL    |       |
| content          | longtext                                                           | YES  |     | NULL    |       |
| authorship       | text                                                               | YES  |     | NULL    |       |
| lastchangeAt     | datetime                                                           | YES  |     | NULL    |       |
| savedAt          | datetime                                                           | YES  |     | NULL    |       |
| createdAt        | datetime                                                           | NO   |     | NULL    |       |
| updatedAt        | datetime                                                           | NO   |     | NULL    |       |
| deletedAt        | datetime                                                           | YES  |     | NULL    |       |
| ownerId          | char(36)                                                           | YES  |     | NULL    |       |
| lastchangeuserId | char(36)                                                           | YES  |     | NULL    |       |
+------------------+--------------------------------------------------------------------+------+-----+---------+-------+

@ccoenen
Copy link
Copy Markdown
Contributor

ccoenen commented Mar 6, 2018

value is now stored just fine in the table. Thanks!

@SISheogorath
Copy link
Copy Markdown
Contributor Author

Nice! That looks good. We should probably check the entire database history for more missing migrations.

@SISheogorath SISheogorath removed the WIP Do not merge label Mar 6, 2018
@ccoenen ccoenen removed the untested This has not be verified to be working label Mar 6, 2018
@SISheogorath SISheogorath added this to the 1.1.0-CE Release milestone Mar 7, 2018
@ccoenen
Copy link
Copy Markdown
Contributor

ccoenen commented Mar 11, 2018

We should probably check the entire database history for more missing migrations.

Probably, but we could also just deal with the things that turn up one by one. I believe the enum problem stems from "less than optimal" support in sequelize. And note permissions are the only place we're using them. I'd guess that sequelize.sync probably works for most other cases just fine without migration.

@SISheogorath SISheogorath merged commit 6b30f66 into hackmdio:master Mar 17, 2018
@SISheogorath SISheogorath deleted the fix/migration branch March 17, 2018 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working database/sequelize Somehow this is related to the database or ORM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants