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

Generate seeds for HABTM associations #80

Merged
merged 13 commits into from
Jun 18, 2024

Conversation

jfoo1984
Copy link
Contributor

@jfoo1984 jfoo1984 commented May 30, 2024

This PR addresses #34. The change add seeds for join tables for an HABTM association by iterating over all HABTM associations in a model.

@jfoo1984
Copy link
Contributor Author

I haven't had a chance to look at updating the specs. I did try to run the migrator spec, but it seemed to be broken due to other reasons.

@jfoo1984
Copy link
Contributor Author

So even though this allows us to generate a seed with that implicit model, it seems like we can't actually run that generated seed successfully. I'm getting the following error

private constant Citation::HABTM_PathogenListVersions referenced

NameError: private constant Citation::HABTM_PathogenListVersions referenced

@jfoo1984 jfoo1984 changed the title Check that registered model has ID to support implicit HABTM models Generate seeds for HABTM associations May 31, 2024
@jfoo1984
Copy link
Contributor Author

jfoo1984 commented May 31, 2024

@pboling This branch is now working for me. Some points to consider though

  • The generated seed generates the HABTM association in both direction, though each statement has a check to make sure the association is not added if it already exists. We could probably improve this by creating a list of HABTM associations and pruning off duplicates
  • Are there cases where we might want duplicated HABTM associations?
  • Should we add a option / var to control whether we check for HABTM associations?

Update: I'm planning to look into consolidating adding associations into a single statement, and also using sets to avoid generating associations both ways

@jfoo1984
Copy link
Contributor Author

jfoo1984 commented Jun 3, 2024

Addressed issues mentioned above

* The generated seed generates the HABTM association in both direction, though each statement has a check to make sure the association is not added if it already exists.  We could probably improve this by creating a list of HABTM associations and pruning off duplicates

https://github.com/pboling/seed_migration/pull/80/files#diff-6652e1848792ce36ff90d5730fcc4de2252024499451356ce8764d918c9e4614R283-R299 adds a step to go through all registered models, and use a Set to store only unique associations.

Should we add a option / var to control whether we check for HABTM associations?

This seems unnecessary, as these associations are only added if both models are registered. But it would not be too difficult to add this config option if needed.

I'm planning to look into consolidating adding associations into a single statement, and also using sets to avoid generating associations both ways.

This has been added, the seed generates statements to add necessary HABTM association ids in a single statement.

@pboling
Copy link
Owner

pboling commented Jun 18, 2024

If you have CI working in your fork, please send a PR with those fixes too! 🗡️

@pboling pboling merged commit e919b6b into pboling:master Jun 18, 2024
0 of 8 checks passed
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