-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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: