-
Notifications
You must be signed in to change notification settings - Fork 182
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
Allow dynamic model registration #140
base: master
Are you sure you want to change the base?
Conversation
thank you for this. Can you please add at least one test that utilizes this? |
@jheld I had trouble testing this, as the lazy invocation for the dynamic model happens when the settings are loaded. So Instead I've opted to pre-register the models in Let me know if this is good enough or if you have a better idea on how to proceed. |
@Etenil I appreciate the effort you've put in. We might merge as is. I had a feeling this might happen. I think I got around it with the delta/changed fields setting by instead of importing the setting variable directly, doing a getattr on settings when doing the logic. Would that work for you? I think override_settings still works in that case -- and I think/hope I created a test case/altered a test case for it. If that doesn't work/taking too much time to implement, we'll merge as is. |
@jheld I'm not really sure how to do what you mentioned, would you have an example? |
Yes. I think I have both a test case (and supporting code in the library which is being tested) for the delta/changed field on/off flag. The setting that's being tested is in the README. It was I think one of the last betas of the current official release. If you have trouble finding it let me know I can add more detail to here. |
@Etenil can you rebase? We may not require an additional test, but want to make sure things are still passing. |
The built-in
User
model can't be fetched by aapps.get_model()
nor given as class as they would go in a recursive import.This allows to do the following instead: