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.

Sign up for GitHub

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

Jump to bottom

New: Migration Scripts (fixes #89) #90

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

New: Migration Scripts (fixes #89) #90

wants to merge 6 commits into from

Conversation

joe-replin
Copy link

New

Testing

Test using this branch of the Adapt Framework which includes the scripts that handle the migration.
adaptlearning/adapt_framework#3633

@joe-replin joe-replin self-assigned this Jan 17, 2025
joe-allen-89
joe-allen-89 requested changes Jan 30, 2025
View reviewed changes
Copy link
Contributor

@joe-allen-89 joe-allen-89 left a comment

Choose a reason for hiding this comment

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

Just a few changes that have come up from suggestions and a few version number changes.

joe-replin and others added 3 commits February 10, 2025 14:48
@joe-replin @joe-allen-89
Apply suggestions from code review
4756c4e 
Committing review suggestions.

Co-authored-by: joe-allen-89 <[email protected]>
@joe-replin @joe-allen-89
Apply suggestions from code review
b9a3072 
whereContent return corrections

Co-authored-by: joe-allen-89 <[email protected]>
@joe-replin
Update v2.js
baa13b7
chris-steele
chris-steele requested changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@chris-steele chris-steele left a comment

Choose a reason for hiding this comment

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

Just a few pointers, but style and format really good and consistent @joe-replin

migrations/v3.js Outdated
/**
* * Add JSON field to component and set attribute.
*/
mutateContent('adapt-contrib-assessmentResults - add assessmentResult._routeToAssessment', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

_routeToAssessment was added from v2.3.1 and v2.4.0; consider splitting this into own describe.

Also move _routeToAssessment to the _retry object

Copy link
Author

Choose a reason for hiding this comment

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

Great finds, thank you.

migrations/v3.js Outdated
@@ -0,0 +1,46 @@
import { describe, whereContent, whereFromPlugin, mutateContent, checkContent, updatePlugin } from 'adapt-migrations';

describe('adapt-contrib-assessmentResults - v2.3.0 > v3.0.0', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The global ariaRegion was added between v2.4.0 and v3.0.0 but is missing in this script

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for catching that.

migrations/v3.js Outdated
/**
* * Add JSON field to component and set attribute.
*/
mutateContent('adapt-contrib-assessmentResults - add assessmentResult._resetType', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

_resetType was added from v2.3.1 and v2.4.0; consider splitting this into own describe.

Copy link
Author

Choose a reason for hiding this comment

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

Wow. that was buried down deep. Good find.

migrations/v5.js Outdated
@@ -0,0 +1,36 @@
import { describe, whereContent, whereFromPlugin, mutateContent, checkContent, updatePlugin } from 'adapt-migrations';

describe('adapt-contrib-assessmentResults - v3.0.0 > v5.2.0', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

global retryText was added between v5.1.1 and v5.1.2

_completionBody default changed between v5.2.0 and v5.2.1

migrations/v5.js
/**
* * Add field to each item in a JSON array and set blank.
*/
mutateContent('adapt-contrib-assessmentResults - add assessmentResultInstance._bands.feedbackNotFinal attribute', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

feedbackNotFinal was added between v5.1.7 and v5.2.0

chris-steele
chris-steele requested changes Feb 20, 2025
View reviewed changes
Copy link
Contributor

@chris-steele chris-steele left a comment

Choose a reason for hiding this comment

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

It's nearly there, just a few comments @joe-replin

migrations/v2.js
@@ -57,3 +51,48 @@ describe('adapt-contrib-assessmentResults - v2.0.3 > v2.3.0', async () => {

updatePlugin('adapt-contrib-assessmentResults - update to v2.3.0', { name: 'adapt-contrib-assessmentResults', version: '2.3.0', framework: '>=2.1.0' });
});

describe('adapt-contrib-assessmentResults - v2.3.0 > v2.4.0', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: these changes came in v2.3.1 😄

migrations/v2.js

mutateContent('adapt-contrib-assessmentResults - add assessmentResult._retry._routeToAssessment', async () => {
assessmentResults.forEach(assessmentResult => {
assessmentResult._retry.forEach(item => {
Copy link
Contributor

Choose a reason for hiding this comment

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

_retry is an object

migrations/v2.js

checkContent('adapt-contrib-assessmentResults - check assessmentResult._retry._routeToAssessment atrribute', async () => {
const isValid = assessmentResults.every(assessmentResult =>
assessmentResult._routeToAssessment.every(item =>
Copy link
Contributor

Choose a reason for hiding this comment

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

_routeToAssessment was a boolean added to the _retry object

migrations/v2.js
checkContent('adapt-contrib-assessmentResults - check assessmentResult._retry._routeToAssessment atrribute', async () => {
const isValid = assessmentResults.every(assessmentResult =>
assessmentResult._routeToAssessment.every(item =>
item && item.feedbackNotFinal === false
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this is here?

migrations/v5.js
@@ -1,18 +1,71 @@
import { describe, whereContent, whereFromPlugin, mutateContent, checkContent, updatePlugin } from 'adapt-migrations';

describe('adapt-contrib-assessmentResults - v3.0.0 > v5.2.0', async () => {
describe('adapt-contrib-assessmentResults - v3.0.0 > v5.1.2', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: global retryText was added in v5.1.1; consider replacing v3.0.0 with v5.1.1 for consistency

migrations/v5.js
updatePlugin('adapt-contrib-assessmentResults - update to v5.1.2', { name: 'adapt-contrib-assessmentResults', version: '5.1.2', framework: '>=5.19.1' });
});

describe('adapt-contrib-assessmentResults - v5.1.2 > v5.1.7', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this describe should be removed as you have provided the corrected version below?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliverfoster oliverfoster oliverfoster left review comments

@chris-steele chris-steele chris-steele requested changes

@joe-allen-89 joe-allen-89 Awaiting requested review from joe-allen-89

Assignees

@joe-replin joe-replin

Labels
enhancement
Projects
adapt_framework: The TODO Board
Status: Needs Reviewing
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Migration Scripts
4 participants
@joe-replin @chris-steele @oliverfoster @joe-allen-89