Skip to content

Add bare bone test for migration to 154 to 155 #24496

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

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Apr 24, 2025

See discussion at #24494 (comment)

Problem: How do we know the migration from model version 154 to 155 works, in particular because the module for the entities changed in 155?

Proposed solution: Add unit tests to exercise the 154 -> 155 migration

We don't even have to merge this, might be enough to just see them green in CI and make a note.

@dangermattic
Copy link
Collaborator

dangermattic commented Apr 24, 2025

2 Warnings
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Apr 24, 2025

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number27499
VersionPR #24496
Bundle IDorg.wordpress.alpha
Commit2a17e73
Installation URL7kvips8m2c8tg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Apr 24, 2025

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number27499
VersionPR #24496
Bundle IDcom.jetpack.alpha
Commit2a17e73
Installation URL7lkjp4q4tb7e8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@mokagio mokagio force-pushed the mokagio/test-154-155-migration branch from 3ff73ba to 050734f Compare April 24, 2025 06:36
mokagio added 4 commits April 24, 2025 16:43
First, convert all entities to "Global Namespace" with

```
sed -i '' -E 's/representedClassName="(WordPress)?(\.)?([^"]+)"/representedClassName="\3"/g' \
  Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress\ *.xcdatamodel/contents
```

Then, set the entities that need the "Current Product Module", using
#24494 as the
source for which to pick

```
sed -i '' -E 's/representedClassName="(ManagedAccountSettings|BlogAuthor|BloggingPrompt|BloggingPromptSettings|BloggingPromptSettingsReminderDays|BlogSettings|DiffAbstractValue|DiffContentValue|DiffTitleValue|ManagedDomain|ManagedPerson|Plan|PlanFeature|PlanGroup|PublicizeConnection|PublicizeService|ReaderCard|Revision|RevisionDiff|Role)"/representedClassName=".\1"/g' \
  Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress\ *.xcdatamodel/contents
```

Notice that
`Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress 155.xcdatamodel/contents`
did not change, which is a point in favor of showing this scripted
change being correct.
@@ -814,7 +814,7 @@
<attribute name="slug" attributeType="String" syncable="YES"/>
<relationship name="blog" maxCount="1" deletionRule="Nullify" destinationEntity="Blog" inverseName="roles" inverseEntity="Blog" syncable="YES"/>
</entity>
<entity name="SharingButton" representedClassName="WordPress.SharingButton" syncable="YES">
<entity name="SharingButton" representedClassName="SharingButton" syncable="YES">
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no . in the SharingButton class name, and a few other entities. Is that expected?

Base automatically changed from task/wpdata-fdsaf to mokagio/wordpressdata-target-xcdatamodeld-workbench-2-move-files April 24, 2025 13:44
@mokagio
Copy link
Contributor Author

mokagio commented Apr 30, 2025

Closing after @kean's input here

The first thing the app does is migrate from the production model (154). It never loads the represented entities in memory. It uses NSManagedObject and KVC for migration. It's impossible to retroactively change the models and there is also no need to. The app has representedClassName values in the previous models with class name that no longer even exist in the codebase – it's what the migration is for.

👋

@mokagio mokagio closed this Apr 30, 2025
@mokagio mokagio deleted the mokagio/test-154-155-migration branch April 30, 2025 05:03
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.

5 participants