Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,21 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [20, 22]
node-version: [20, 22, 24]
mongodb-version: [6.0, 7.0, 8.0]

# Steps represent a sequence of tasks that will be executed as part of the job
steps:
- name: Git checkout
uses: actions/checkout@v4
uses: actions/checkout@v5

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
uses: actions/setup-node@v6
with:
node-version: ${{ matrix.node-version }}

- name: Start MongoDB
uses: supercharge/mongodb-github-action@1.11.0
uses: supercharge/mongodb-github-action@1.12.0
with:
mongodb-version: ${{ matrix.mongodb-version }}

Expand Down
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

## UNRELEASED

### Add

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

### Changes

* `@apostrophecms/migration:after` handler now runs the migration requirements like `insertIfMissing`, `implementParkAllInDefaultLocale`, `replicate` and `implementParkAllInOtherLocales`.

### 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
16 changes: 1 addition & 15 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,21 +323,7 @@ async function apostrophe(options, telemetry, rootSpan) {
await self.emit('modulesRegistered'); // formerly modulesReady
self.apos.schema.validateAllSchemas();
self.apos.schema.registerAllSchemas();
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();
});
await self.apos.migration.migrate(self.argv);
await self.emit('ready'); // formerly afterInit

if (self.taskRan) {
Expand Down
124 changes: 86 additions & 38 deletions modules/@apostrophecms/migration/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,52 @@
// 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 {
'apostrophe:modulesRegistered': {
setSkipMigration() {
if (
self.apos.isTask() &&
self.options.skipMigrationTasks.includes(self.apos.argv._.at(0))
) {
self.apos.skipMigration = true;
}
}
},
before: {
async addMissingSchemaFields() {
await self.addMissingSchemaFields();
}
},
after: {
Copy link
Member

Choose a reason for hiding this comment

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

I see why you did this, but previously, these "requirements" were guaranteed to come last, even after the after event. And changing that is an unnecessary risk.

So, please make them individual handlers on a new requirements event which is fired after the after event has been safely awaited.

async requirements() {
// 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.apos.modules)) {
if (
self.apos.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

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (22, 6)

This line has a length of 93. Maximum allowed is 90

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (20, 7)

This line has a length of 93. Maximum allowed is 90

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (20, 6)

This line has a length of 93. Maximum allowed is 90

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (24, 6)

This line has a length of 93. Maximum allowed is 90

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (24, 7)

This line has a length of 93. Maximum allowed is 90

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (22, 8)

This line has a length of 93. Maximum allowed is 90

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (20, 8)

This line has a length of 93. Maximum allowed is 90

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (24, 8)

This line has a length of 93. Maximum allowed is 90

Check warning on line 57 in modules/@apostrophecms/migration/index.js

View workflow job for this annotation

GitHub Actions / build (22, 7)

This line has a length of 93. Maximum allowed is 90
// 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();
}
},
'apostrophe:ready': {
addSortifyMigrations() {
const managers = self.apos.doc.managers;
Expand All @@ -41,11 +80,6 @@
});
});
}
},
before: {
async addMissingSchemaFields() {
await self.addMissingSchemaFields();
}
}
};
},
Expand Down Expand Up @@ -236,43 +270,51 @@
// Perform the actual migrations. Implementation of
// the @apostrophecms/migration:migrate task
async migrate(options) {
await self.emit('before');
if (self.apos.isNew) {
await self.apos.lock.withLock('@apostrophecms/migration:migrate', async () => {
if (self.apos.skipMigration === true) {
return;
}

await self.emit('before');

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
// that they were skipped, just in case we decide that's an issue
// later
const at = new Date();
// Just in case the db has no documents but did
// start to run migrations on a previous attempt,
// which causes an occasional unique key error if not
// corrected for here.
//
// Other migration-related facts that are not migration
// names are stored with a leading *, leave them alone
await self.db.removeMany({
_id: /^[^*]/
});
await self.db.insertMany(self.migrations.map(migration => ({
_id: migration.name,
at,
skipped: true
})));
} else {
for (const migration of self.migrations) {
await self.runOne(migration);
const at = new Date();
// Just in case the db has no documents but did
// start to run migrations on a previous attempt,
// which causes an occasional unique key error if not
// corrected for here.
//
// Other migration-related facts that are not migration
// names are stored with a leading *, leave them alone
await self.db.removeMany({
_id: /^[^*]/
});
await self.db.insertMany(self.migrations.map(migration => ({
_id: migration.name,
at,
skipped: true
})));
} else {
for (const migration of self.migrations) {
await self.runOne(migration);
}
}
}
// In production, this event is emitted only at the end of the migrate
// command line task. In dev it is emitted at every startup after the
// automatic migration.
//
// Intentionally emitted regardless of whether the site is new or not.
//
// This is the right time to park pages, for instance, because the
// database is guaranteed to be in a stable state, whether because the
// site is new or because migrations ran successfully.
await self.emit('after');

// In production, this event is emitted only at the end of the migrate
// command line task. In dev it is emitted at every startup after the
// automatic migration.
//
// Intentionally emitted regardless of whether the site is new or not.
//
// This is the right time to park pages, for instance, because the
// database is guaranteed to be in a stable state, whether because the
// site is new or because migrations ran successfully.
await self.emit('after');
Copy link
Member

Choose a reason for hiding this comment

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

Emit a new requirements event "after after" 😄

});
},
async runOne(migration) {
const info = await self.db.findOne({ _id: migration.name });
Expand Down Expand Up @@ -305,6 +347,12 @@
// 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 () => {
await self.addMissingSchemaFields();
}
}
};
}
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-project/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:../../."
}
}
Loading