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

Support Scan Plugins Models & Migrations #63

Closed
kudorori opened this issue Sep 28, 2018 · 12 comments
Closed

Support Scan Plugins Models & Migrations #63

kudorori opened this issue Sep 28, 2018 · 12 comments

Comments

@kudorori
Copy link

I hope I can support the models and migrations in the plugin. I can open a PR if needed.

@bayssmekanique
Copy link
Collaborator

Can you be a bit more specific about what you are looking to be able to do?

The plugin does currently support using models and performing migrations. Are you looking for a more integrated approach?

@kudorori
Copy link
Author

kudorori commented Oct 2, 2018

I hope he can support the models and migration folders in the plugin.

@bayssmekanique
Copy link
Collaborator

Model and Migrations folders are both supported through this plugin. The directory for both of these is not automatically generated, which might be what your request is, but the support is there.

@kudorori
Copy link
Author

kudorori commented Oct 2, 2018

Maybe my expression is not very good

Is he able to find models and migrations for other plugins?

├── models
├── migrations
├── plugins               
│   ├── user-plugin
│   │    ├── models        <= here
│   │    ├── migrations   <= here

@bayssmekanique
Copy link
Collaborator

AH! I see what you are saying and I can see the use cases.

My concern with directly pulling in models and migrations from the node_modules directory would be in overriding models without breaking the way sequelize handles defining models (and I don't want to break sequelize functionality).

I think the best approach might be to have the plugin inject models and migrations into the project during installation. The plugin would not have to "eject" the entire model, it could just be a hook that gets ejected, but that would at least give an easy point for a dev to override the model or migrations.

Example:

// migrations/20181002162429-example-plugin.js
const migration = require('example-ah-plugin').migration
module.exports = migration
// models/examplePlugin.js
const models = require('example-ah-plugin').models
module.exports = models

@evantahler, any insights you can provide? Has something similar come up with other plugins previously?

@evantahler
Copy link
Member

I would suggest looking at how Rails handles plugins with migrations for inspiration. We would need a mechanism which allows developers to opt-into a plugin's migrations, rather than have them run automatically.

If we made it so that migrations directories could have sub-folders, we could just symlink in the plugin's migrations?

@kudorori
Copy link
Author

kudorori commented Oct 3, 2018

This is just my idea

//config/plugins.js

// You can also toggle on or off sections of a plugin to include (default true for all sections):
return {
  'userPlugin': {
    path: __dirname + '/../node_modules/userPlugin',
    actions: true,
    tasks: true,
    initializers: true,
    servers: true,
    cli: true,
    public: true,
    sequelize: true    // <=  here
  }
}

@bayssmekanique
Copy link
Collaborator

bayssmekanique commented Oct 3, 2018

@kudorori, that would certainly work for the configuration portion.

The migrations might be able to stay in place if sequelize/umzug#106 can get resolved, otherwise we'll either have to do symlinks (which as a Windows user I have a strong distrust of) or running umzug for each plugin directory (which I'm not entirely certain will even work).

We perform the model loading ourselves, so that's easy enough to override to look into the plugins directories and find the models that need to be pulled in.

The only thing I'm not seeing a solution for is overriding the plugin. I know most plugins don't have extensive overriding short of configuration, so I might be overthinking this a bit. I'm thinking of the use-case of an ACL plugin that would have a core User model that would likely need to be extended to meet the application needs. Would the solution be to ship the plugin without a User model and have the plugin assume the User model will be supplied by the end-user and attempt to make associations with the model during initialization?

@agmcleod
Copy link
Contributor

sort of similar, i wonder if it might be possible to update ah-sequelize so it accepts a config folder (defaulting to project root) for where to find models and migrations? Im potentially looking at building a repo that has two separate action hero backends in it, but shares models. Two services that connect to a single db.

@bayssmekanique
Copy link
Collaborator

I'll work on a fix this weekend.

@bayssmekanique
Copy link
Collaborator

I've pushed some changes to the issue-63 branch. This essentially just adds a modelsDir and migrationsDir config parameter which can be used to overide the location of both using either a path or an array of paths relative to the project root. This should solve @agmcleod's need, and while it's less ideal for seamless plugin support it enables that capability.

The one caveat to this approach is that each migrationsDir path will be processed in full before moving on to the next dir location, so migrations can not rely on migrations from other directories. This shouldn't be a problem for plugins, but it's worth mentioning. @evantahler, any thoughts?

evantahler added a commit that referenced this issue Dec 4, 2018
@bayssmekanique
Copy link
Collaborator

Issued closed by PR #65.

Solution is to use modelDir and migrationDir properties on the configuration object to add additional models or migrations.

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

No branches or pull requests

4 participants