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

import default data population #982

Merged
merged 21 commits into from
Aug 28, 2015
Merged

import default data population #982

merged 21 commits into from
Aug 28, 2015

Conversation

stopfstedt
Copy link
Member

fixes #892

@stopfstedt
Copy link
Member Author

still WIP as unit tests and MeSH fixtures are still missing. it works tho.

@@ -0,0 +1,34 @@
LOCK TABLES `competency` WRITE;
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing all fo these .sql files were committed accidentally? Otherwise I don't know what they are for?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch. they are left over from an earlier, aborted attempt to run queries against the db via doctrine.
removing them...

@stopfstedt
Copy link
Member Author

added tests, this is good to go (unless travis sez otherwise). willl deal with mesh another day...


// honor the given entity identifiers.
// @link http://www.ens.ro/2012/07/03/symfony2-doctrine-force-entity-id-on-persist/
$metadata = $manager->getClassMetaData(get_class($entity));
Copy link
Member

Choose a reason for hiding this comment

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

There is a force ID option in the entity managers:
https://github.com/ilios/ilios/blob/master/src/Ilios/CoreBundle/Entity/Manager/AamcPcrsManager.php#L57

It may be too much work to inject our custom managers, but maybe that is what the $manager is. I'm not sure.

I'm also worried that we're using a different pattern in these two places. I have no idea what the correct path might be so I'm good with this one, just a comment.

Last, I think you can call these two metadata lines a single time before the loop as opposed to on each iteration. There may be a slight performance bump in doing that, or maybe it will break everything.

@jrjohnson
Copy link
Member

Looks good; couple of comments which you can ignore.
@saschaben - there is a broader question of what default data we should load for a new install. Do we want to continue providing a set of UCSF schools and departments?

@saschaben
Copy link
Member

I think we should continue to provide the default set of schools (SOM, SOP, SOD, SON) but the departments is an open question. We don't really leverage them in any way currently, so it doesn't really offer much to other schools in way of example.

@jrjohnson
Copy link
Member

:+1

@stopfstedt stopfstedt assigned dartajax and unassigned jrjohnson Aug 28, 2015
dartajax added a commit that referenced this pull request Aug 28, 2015
@dartajax dartajax merged commit fdffb45 into ilios:master Aug 28, 2015
@stopfstedt stopfstedt deleted the 892_import_default_data_task branch August 28, 2015 22:09
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.

Create data loading fixtures
4 participants