Skip to content

New: customscriptsdir added to migrations grunt task (fixes #3677) #3680

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

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

Conversation

joe-allen-89
Copy link
Contributor

@joe-allen-89 joe-allen-89 commented May 1, 2025

Adding the customscriptdir is to allow a custom folder to be specified, this allows us to load custom migration scripts for the authoring tool.

Changed the fileNameIncludes matching from .includes() to RegExp, this is to make it more accurate when targetting a specific file. For example when using .includes() the flag --file=text will also pull through textInput.

#3677

Fix

  • match fileNameIncludes changed to RegExp

New

@oliverfoster
Copy link
Member

oliverfoster commented May 1, 2025

minimatch(filePath, '**/' + fileNameIncludes)

This should do what you're doing with the regex. It just uses globs to define the regex. I'm not clear on how the behaviour is changed.

https://github.com/isaacs/minimatch

The use of includes as well as glob matching is by design. It was originally just includes https://github.com/adaptlearning/adapt-migrations/blame/831052bb4b3bf06ed8bb4d0e8b57961fa6e282aa/examples/migrations.js#L55 and Joe R had gotten used to it. I replaced it with the globs check because it made more sense to me and then Joe R requested a reversion, so I implemented both. You should be able to write text/ or adapt-contrib-text if you want to run only text component scripts.

--file=text will also pull through textInput.

I think this is acceptable. We're specify files/folder names with --file, not explicit components. In the same way --file=assessment would also include assessmentResults.

The code for the custom folder looks good.

@joe-allen-89 joe-allen-89 moved this from New to Assigned in adapt_framework: The TODO Board May 6, 2025
@joe-allen-89
Copy link
Contributor Author

My thought process was to avoid any potential issue with naming custom tasks in the authoring tool. Regex change reveretd.

@oliverfoster
Copy link
Member

There is currently no API for getting all of the existing tasks and running a specific one (other than --file). If you need the AAT to be able do that then you can add new stuff in here for that specific purpose?

grunt migration:gettasks to output JSON or such?

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@cahirodoherty-learningpool cahirodoherty-learningpool moved this from Assigned to Needs Reviewing in adapt_framework: The TODO Board Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Reviewing
Development

Successfully merging this pull request may close these issues.

Add the ability to pass in a folder containing migration scripts.
3 participants