Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

## UNRELEASED

### Add

* Add `@apostrophecms/migration:add-missing-schema-fields` task. This task does not run database migrations.

### Changes

### Fixes

## UNRELEASED
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Recreating the section because this is not part of today's release, easier for me to handle future conflict.


### Adds

* Add locale picker in the page and piece manager modals.
Expand Down
32 changes: 31 additions & 1 deletion modules/@apostrophecms/migration/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,28 @@ const addMissingSchemaFields = require('./lib/addMissingSchemaFields.js');
// is difficult to guarantee, you may wish to write a task instead.

module.exports = {
options: { alias: 'migration' },
options: {
alias: 'migration',
skipMigrationTasks: [
'@apostrophecms/migration:add-missing-schema-fields'
]
},
async init(self) {
self.migrations = [];
await self.enableCollection();
},
handlers(self) {
return {
[`${self.__meta.name}:before`]: {
setSkipMigration() {
if (
self.apos.isTask() &&
self.options.skipMigrationTasks.includes(self.apos.argv._.at(0))
) {
self.apos.skipMigration = true;
}
}
},
'apostrophe:ready': {
addSortifyMigrations() {
const managers = self.apos.doc.managers;
Expand Down Expand Up @@ -237,6 +252,10 @@ module.exports = {
// the @apostrophecms/migration:migrate task
async migrate(options) {
await self.emit('before');
Copy link
Member

Choose a reason for hiding this comment

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

You should skip this line too, even though it means your task will have to explicitly call addMissingSchemaFields.

This is appropriate because:

  • We used the before hook to implement addMissingSchemaFields, which is a special kind of migration
  • Other people and other modules will likely do the same thing
  • Michelin doesn't want anything else that smells like a migration at all to happen in this task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding the following code

    await self.apos.lock.withLock('@apostrophecms/migration:migrate', async () => {
      await self.apos.migration.migrate(self.argv);
      // Inserts the global doc in the default locale if it does not exist;
      // same for other singleton piece types registered by other modules
      for (const apostropheModule of Object.values(self.modules)) {
        if (self.instanceOf(apostropheModule, '@apostrophecms/piece-type') && apostropheModule.options.singletonAuto) {
          await apostropheModule.insertIfMissing();
        }
      }
      await self.apos.page.implementParkAllInDefaultLocale();
      await self.apos.doc.replicate(); // emits beforeReplicate and afterReplicate events
      // Replicate will have created the parked pages across locales if needed,
      // but we may still need to reset parked properties
      await self.apos.page.implementParkAllInOtherLocales();
    });

Should I skip all apostropheModule.insertIfMissing + page.implementParkAllInDefaultLocale, doc.replicate & page.implementParkAllInOtherLocales too?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that makes sense.

if (self.apos.skipMigration === true) {
return;
}

if (self.apos.isNew) {
// Since the site is brand new (zero documents), we may assume
// it requires no migrations. Mark them all as "done" but note
Expand Down Expand Up @@ -305,6 +324,17 @@ module.exports = {
// and automatically detect whether any work
// must be done
task: () => {}
},
'add-missing-schema-fields': {
usage: 'Add missing schema fields to existing database documents',
task: async () => {
const result = await self.addMissingSchemaFields();
const migrations = await self.db.find().toArray();
console.log({
result,
migrations
});
}
}
};
}
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "index.js",
"scripts": {
"pretest": "npm run lint",
"test": "nyc mocha -t 10000 --ignore=test/assets.js && nyc mocha -t 10000 test/assets.js && mocha -t 1000 test/esm-project/esm.js",
"test": "nyc mocha -t 10000 --ignore=test/assets.js && nyc mocha -t 10000 test/add-missing-schema-fields/test.js && nyc mocha -t 10000 test/assets.js && mocha -t 1000 test/esm-project/esm.js",
"eslint": "eslint .",
"eslint-fix": "npm run eslint -- --fix",
"i18n": "node scripts/lint-i18n",
Expand Down Expand Up @@ -144,4 +144,4 @@
"browserslist": [
"ie >= 10"
]
}
}
4 changes: 4 additions & 0 deletions test/add-missing-schema-fields-project/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import config from './config.js';
import apostrophe from '../../index.js';

export default apostrophe(config);
13 changes: 13 additions & 0 deletions test/add-missing-schema-fields-project/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export default {
root: import.meta,
shortName: 'add-missing-schema-fields-project',
baseUrl: 'http://localhost:3000',
modules: {
'@apostrophecms/express': {
options: {
address: '127.0.0.1'
}
},
product: {}
}
};
77 changes: 77 additions & 0 deletions test/add-missing-schema-fields-project/modules/product/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
export default {
extend: '@apostrophecms/piece-type',
options: {
label: 'Product',
pluralLabel: 'Products'
},
fields: {
add: {
price: {
type: 'float',
label: 'Price',
required: true
},
description: {
type: 'string',
label: 'Description',
textarea: true,
required: true
},
image: {
label: 'Product photo',
type: 'area',
options: {
max: 1,
widgets: {
'@apostrophecms/image': {}
}
}
},
copyright: {
type: 'string',
label: 'Copyright'
}
},
group: {
basics: {
label: 'Basics',
fields: [ 'title', 'price', 'description', 'image', 'copyright' ]
// 👆 'title' is included here because it is in the default `basics`
// group for all piece types. Since we are replacing that group, we
// include it ourselves.
}
}
},
init(self) {
self.apos.migration.add('add-first-product', self.addFirstProduct);
self.apos.migration.add('add-copyright-notice', self.addCopyrightNotice);
},
methods(self) {
return {
async addFirstProduct() {
await self.apos.modules.product.insert(
self.apos.task.getReq(),
{
...self.apos.modules.product.newInstance(),
title: 'My first product',
price: 10.00,
description: 'Product description'
}
);
},
async addCopyrightNotice() {
await self.apos.migration.eachDoc({
type: 'product'
}, async (doc) => {
if (doc.copyright === undefined) {
await self.apos.doc.db.updateOne({
_id: doc._id
}, {
$set: { copyright: '©2024 ApostropheCMS. All rights reserved.' }
});
}
});
}
};
}
};
16 changes: 16 additions & 0 deletions test/add-missing-schema-fields-project/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"name": "add-missing-schema-fields-project",
"version": "1.0.0",
"description": "",
"main": "app.js",
"type": "module",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
"license": "ISC",
"dependencies": {
"apostrophe": "file:../../."
}
}
120 changes: 120 additions & 0 deletions test/add-missing-schema-fields-project/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { strict as assert } from 'node:assert';
import process from 'node:process';
import path from 'node:path';
import util from 'node:util';
import { exec } from 'node:child_process';
import t from '../../test-lib/test.js';
import config from './config.js';

describe('Apostrophe - add-missing-schema-fields task', function() {
this.timeout(t.timeout);

let apos;

before(async function() {
await util.promisify(exec)(
'npm install',
{ cwd: path.resolve(process.cwd(), 'test/add-missing-schema-fields-project/') }
);
});

afterEach(function() {
return t.destroy(apos);
});

it('should not run migrations when running the task', async function() {
await util.promisify(exec)(
'node app.js @apostrophecms/migration:add-missing-schema-fields',
{ cwd: path.resolve(process.cwd(), 'test/add-missing-schema-fields-project/') }
);

apos = await t.create({
...config,
root: import.meta,
modules: {
...config.modules,
debug: {
handlers(self) {
return {
'@apostrophecms/migration:before': {
forceSkipMigration() {
self.apos.skipMigration = true;
}
}
};
}
}
}
});

const migrations = await apos.migration.db.find().toArray();

const actual = {
migrations: migrations.map(migration => migration._id)
};
const expected = {
migrations: [ '*lastPropLists' ]
};

assert.deepEqual(actual, expected);
});

it('should run migrations when running @apostrophecms/migration:migrate task', async function() {
await util.promisify(exec)(
'node app.js @apostrophecms/migration:migrate',
{ cwd: path.resolve(process.cwd(), 'test/add-missing-schema-fields-project/') }
);

apos = await t.create({
...config,
root: import.meta,
modules: {
...config.modules,
debug: {
handlers(self) {
return {
'@apostrophecms/migration:before': {
forceSkipMigration() {
self.apos.skipMigration = true;
}
}
};
}
}
}
});

const migrations = await apos.migration.db.find().toArray();

const actual = {
migrations: migrations.map(migration => migration._id)
};
const expected = {
migrations: [ '*lastPropLists' ].concat(
apos.migration.migrations.map(migration => migration.name)
)
};

assert.deepEqual(actual, expected);
});

it('should run migrations when starting apostrophe', async function() {
apos = await t.create({
...config,
root: import.meta
});

const migrations = await apos.migration.db.find().toArray();

const actual = {
migrations: migrations.map(migration => migration._id)
};
const expected = {
migrations: [ '*lastPropLists' ].concat(
apos.migration.migrations.map(migration => migration.name)
)
};

assert.deepEqual(actual, expected);
});
});
Loading