-
Notifications
You must be signed in to change notification settings - Fork 36
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
Add new tables and fields for MOTs #5008
Conversation
This should be fixing #4981 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Minor things that'll be nice to resolve (in comments)
Do you expect multiple schema changes to be made on this concept? Something to consider is that each subsequent change will require a migration. I think it will better that the changes get applied in production inside one migration. If we merge this in production as it is then,
- Each schema change will be a migration, increasing number of migrations, more number of things that can go wrong when MOT gets merged to production
- If there is a hot bug fix needed, it will be a mess to do it properly if partial MOT is inside production at any point.
I'll recommend keeping all the changes in this branch. Yes, subsequent changes will still require a migration. But, before release, we can make merge those migrations into one migration. So, in realty, only one migration will run for production users after release.
TLDR: Consider not merging into production (even after 7.9.6 gets released), and make this branch the base for all MOT development so migrations can be squashed into one.
Ya, I too feel a little uneasy about the number of Django migrations needed for 7.9.7. I was thinking about solving multiple issues in one branch, or at least just merge issue into a 7-9-7-temp branch. Also, we don't want to be merging this code into production before 7.9.6 is released. The problem would then be that we would have to manually make database changes on the test-panel so that testers could try out the branches, so for testers it would be better with Django migrations. |
Relationship(name='createdbyagent', type='many-to-one', required=False, relatedModelName='Agent', column='CreatedByAgent_ID'), | ||
Relationship(name='modifiedbyagent', type='many-to-one', required=False, relatedModelName='Agent', column='ModifiedByAgent_ID') | ||
Relationship(name='collection', type='many-to-one', required=False, relatedModelName='Collection', column='CollectionID'), | ||
Relationship(name='specifyUser', type='many-to-one', required=True, relatedModelName='SpecifyUser', column='SpecifyUse_ID'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo here? SpecifyUse_ID
. Also does it need an underscore or does it have to be removed like the other ID columns?
Closing this PR, being replaced with #5032 |
@acwhite211, canyon close the associated branch too? |
Fixes #4981
Add the new CollectionObjectType table to the specify models, database, and datamodel.
Checklist
and self-explanatory (or properly documented)
Testing instructions