Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pro 6471 migrations #401

Merged
merged 5 commits into from
Oct 2, 2024
Merged

Pro 6471 migrations #401

merged 5 commits into from
Oct 2, 2024

Conversation

BoDonkey
Copy link
Contributor

Summary

Summarize the changes briefly, including which issue/ticket this resolves. If it closes an existing Github issue, include "Closes #[issue number]"
The PR adds a section on writing migrations to the docs. Closes PRO-6471.

What are the specific steps to test this change?

For example:

  1. Run the website and log in as an admin
  2. Open a piece manager modal and select several pieces
  3. Click the "Archive" button on the top left of the manager and confirm that it should proceed
  4. Check that all pieces have been archived properly

What kind of change does this PR introduce?

(Check at least one)

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Build-related changes
  • Other

Make sure the PR fulfills these requirements:

  • It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
  • The changelog is updated
  • Related documentation has been updated
  • Related tests have been updated

If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

Copy link

linear bot commented Sep 18, 2024


While the migration function can be added as an anonymous function as the second argument to the `add()`method, they can also be defined in the `methods(self)` customization function of the module. This can provide for a cleaner `init(self)` function, but is a matter of preference.

Example adding the migration to `init(self)`
Copy link
Member

Choose a reason for hiding this comment

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

How about a trailing colon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overlooked and added!

return {
async alignImages(self) {
return self.apos.migration.eachWidget({
type: 'image'
Copy link
Member

Choose a reason for hiding this comment

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

This is the criteria object used to find the documents, it has nothing to do with the widget type. If you want to find this widget no matter where it lies across all document types, use an empty criteria object, which is the norm. If this widget only ever occurs in one document type you can speed things up a bit by leveraging that here in the criteria object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. I thought I saw an example where this seemed to be the widget name, but now I can't find it. Makes more sense the way you explain it. Changed.

docs/guide/writing-migrations.md Show resolved Hide resolved

## Adding a Missing Property to Existing Widgets

Similar to updating pieces and pages, you can use the `eachWidget` helper to add or remove properties from widgets of a specific type. This is useful when updating the schema of a widget across all pages or pieces.
Copy link
Member

Choose a reason for hiding this comment

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

Worth noting that eachWidget will work no matter how the widget has been nested, e.g. in object fields, array fields and areas within other widgets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

};
```

- The `eachWidget` method iterates over **every** widget in **every** area in **every** document. For this reason, you should carefully ensure that the criteria for passing the widget to the iterator is specific to only the widgets that need to be altered.
Copy link
Member

Choose a reason for hiding this comment

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

We should water this last sentence way down because you usually can't and have to accept that a widget migration takes some time to scan all documents. You can speed it up only if you know only certain page or doc types ever contain that widget in their schema, and in that case you can do a query on type, but you must stress that it is the document type and not the widget type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

- In our `criteria` argument we pass a type of `image` to return all instances of `image` widgets that are found.
- In the iterator we check if the `alignment` property exists, and if not, we use `$set` to add it.

The `iterator` in an `eachWidget` method gets three arguments. In addition to the document, `doc`, where the widget is found, it also receives the `widget` object that will be modified and the `dotPath`. The `dotPath` argument represents the location of the current widget within the document's structure, using a "dot notation" format. It allows you to trace exactly where the widget is nested within its parent area, such as `main.content[0]`, where `main` is the area, `content` is the widget array, and `[0]` is the first widget in that array. This simplifies the process of pointing the MongoDB operation at the correct widget within a document.
Copy link
Member

Choose a reason for hiding this comment

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

It'll look like main.content.0 actually, but as long as you honor it and use it when calling $set it doesn't matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

return {
async migrate(self) {
return self.apos.migration.eachWidget({
type: 'video'
Copy link
Member

Choose a reason for hiding this comment

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

See above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

- We use `$unset` to remove the `border` property from the widget.

## Additional Migrations
While the examples above use `eachDoc` and `eachWidget` to iterate over and modify documents, you're welcome to use any MongoDB APIs you're familiar with to perform migrations. For instance, if your migration needs are simple and easily expressed through MongoDB's query capabilities, methods like `updateMany` can be more efficient than iterating over every document individually.
Copy link
Member

Choose a reason for hiding this comment

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

It is worth mentioning that the first two examples above (the eachDoc ones) can also be written with updateMany, but that this is not always practical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@@ -0,0 +1,262 @@
# Writing Migrations

Migrations in ApostropheCMS allow you to make targeted changes to your database, ensuring that your data stays in sync with the evolving structure of your code. They are particularly useful when new code deployments introduce changes to the underlying data model or schema, such as adding new fields, removing deprecated properties, or correcting inconsistencies in existing content. In this guide, we will walk through how to write migrations to add or remove properties from existing pieces and widgets.
Copy link
Member

Choose a reason for hiding this comment

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

In this coming release we are shipping logic to automatically add a default value for any new schema field that has a def property or is of a type that has its own def, which covers most types (for instance, for string the fallback def is ''). So the need to write migrations just for a brand new field alone is about to drop off considerably.

Copy link
Member

Choose a reason for hiding this comment

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

Doubling down on this because my work to automatically add default values has merged to main and release day is Wednesday, so it makes sense to address it here:

If your goal is to ensure a newly-added field in the schema will be present in the database for existing documents with its default value (as specified by def, or the fallback def of the field type such as the empty string for string fields), then you do not need to write a migration. As of version 4.x, this is automatic. However if you need to transform the existing content of the database in another way, such as renaming or removing a property or transforming a number to a string, et cetera, then migrations are the right tool for you.


## Removing a Property from Existing Documents

If you need to remove a property, you can use `$unset`. Here’s an example that removes a `temporaryNote` property from all `default` page types:
Copy link
Member

Choose a reason for hiding this comment

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

I agree we should cover how to unset, but please add an info note stating that it is often wiser to leave the data you think you "don't need anymore" in the database and simply remove it from the schema, at least at first, to make absolutely sure you won't regret deleting information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

@@ -0,0 +1,262 @@
# Writing Migrations

Migrations in ApostropheCMS allow you to make targeted changes to your database, ensuring that your data stays in sync with the evolving structure of your code. They are particularly useful when new code deployments introduce changes to the underlying data model or schema, such as adding new fields, removing deprecated properties, or correcting inconsistencies in existing content. In this guide, we will walk through how to write migrations to add or remove properties from existing pieces and widgets.
Copy link
Member

Choose a reason for hiding this comment

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

Doubling down on this because my work to automatically add default values has merged to main and release day is Wednesday, so it makes sense to address it here:

If your goal is to ensure a newly-added field in the schema will be present in the database for existing documents with its default value (as specified by def, or the fallback def of the field type such as the empty string for string fields), then you do not need to write a migration. As of version 4.x, this is automatic. However if you need to transform the existing content of the database in another way, such as renaming or removing a property or transforming a number to a string, et cetera, then migrations are the right tool for you.


## Adding migrations

In ApostropheCMS, migrations are added using the [`add(name, fn)` method](/reference/modules/migration.md#add-name-fn) of the `@apostrophecms/migration` module. One common place to add these are within the `init(self)` initialization function of your module. Each migration requires a unique name and is only run **once**. ApostropheCMS tracks which migrations have already been executed, ensuring they won’t run again across restarts or deployments.
Copy link
Member

Choose a reason for hiding this comment

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

"one common place to add these is within" right? (English is weird)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. corrected.

return self.apos.migration.eachDoc({
type: 'article'
}, async (doc) => {
if (doc.featured === undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

I think a simple example like this is still good, but I would document it as a way to clean up if a string field was first added with no def and then you decide, as a one-time thing, to change the empty string to a more useful default later. Apostrophe can't do that automatically because it can't divine your intent and the empty string can be a legitimate value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

To apply pending migrations in a production environment, run:

```bash
node app @apostrophecms/migration:run
Copy link
Member

Choose a reason for hiding this comment

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

  1. The task name is @apostrophecms/migration:migrate

  2. Although migrations currently do run automatically in both development and production, it is best practice to run the @apostrophecms/migration:migrate task in production before launching the newest version of the application to serve requests. At a future time, an option to disable Apostrophe's check for needed migrations on ordinary invocations in production may be offered as an optimization.

(Yeaaaaa I just found out this is true two weeks ago sorry LOL)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Number 1 - changed. I guess I was rushing.
Number 2 - changed.


## Adding or Modifying a Property in Existing Documents

When a new property needs to be added to all instances of a document type, you can use the [`eachDoc`](/reference/modules/migration.md#async-eachdoc-criteria-limit-iterator) helper provided by the migration module. This method efficiently queries documents in your collection and allows you to update them with only the necessary changes. The `eachDoc` helper takes three parameters.
Copy link
Member

Choose a reason for hiding this comment

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

When a property of all instances of a document type needs to be changed, transformed or added in a way more complicated than setting def at the time it is first added to the code...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

await self.apos.doc.db.updateOne({
_id: doc._id
}, {
$unset: { `${dotPath.border}`: '' }
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect logic here, .border is supposed to be part of the string, not evaluated as a property of dotPath (you need to close your } sooner)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@BoDonkey
Copy link
Contributor Author

I'm wondering with the new intro if we need an example of changing a string schema field to a number?

Copy link
Member

@boutell boutell left a comment

Choose a reason for hiding this comment

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

All good, just providing you with a real version number for the line about when def beomes automatic

@@ -1,10 +1,10 @@
# Writing Migrations

Migrations in ApostropheCMS allow you to make targeted changes to your database, ensuring that your data stays in sync with the evolving structure of your code. They are particularly useful when new code deployments introduce changes to the underlying data model or schema, such as adding new fields, removing deprecated properties, or correcting inconsistencies in existing content. In this guide, we will walk through how to write migrations to add or remove properties from existing pieces and widgets.
Migrations in ApostropheCMS allow you to make targeted changes to your database, ensuring that your data stays in sync with the evolving structure of your code. If your goal is to ensure a newly-added field in the schema will be present in the database for existing documents with its default value (as specified by def, or the fallback def of the field type such as the empty string for string fields), then you do not need to write a migration. As of version 4.x, this is automatic. However, if you need to transform the existing content of the database in another way, such as renaming or removing a property or transforming a number to a string, then migrations are the right tool for you. In this guide, we will walk through how to write migrations to add or remove properties from existing pieces and widgets.
Copy link
Member

Choose a reason for hiding this comment

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

Shipping tomorrow, so you can write 4.8.0 here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

await self.apos.doc.db.updateOne({
_id: doc._id
}, {
$set: { description: 'No description available' }
$set: { copyright: '©2024 ApostropheCMS. All rights reserved.' }
Copy link
Member

Choose a reason for hiding this comment

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

Still not a frequent use case anymore (because def will automatically get propagated), but I do think we need a simple example and it's valid, so let's go with it.

@@ -249,7 +244,7 @@ module.exports = {
await self.apos.doc.db.updateOne({
_id: doc._id
}, {
$unset: { `${dotPath.border}`: '' }
$unset: { `${dotPath}.border`: '' }
Copy link
Member

Choose a reason for hiding this comment

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

... Huh. I see that your empty string "value" is consistent with how mongodb examples handle this, I guess they like a natural "falsy"/"empty" value for whatever the type was, even though the value does not matter at all with $unset. 👍

@BoDonkey BoDonkey requested a review from boutell October 2, 2024 12:16
@BoDonkey BoDonkey merged commit f9e9a91 into main Oct 2, 2024
@BoDonkey BoDonkey deleted the pro-6471-migrations branch October 2, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants