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

Extending Target Model with user defined target data #851

Closed
3 of 5 tasks
jchate6 opened this issue Feb 23, 2024 · 1 comment · Fixed by #885
Closed
3 of 5 tasks

Extending Target Model with user defined target data #851

jchate6 opened this issue Feb 23, 2024 · 1 comment · Fixed by #885
Assignees
Labels
enhancement New feature or request

Comments

@jchate6
Copy link
Contributor

jchate6 commented Feb 23, 2024

Include user defined abstract base classes using model inheritance to extend the Target Model. The purpose here is to replace Target_Extras with a more sustainable system

  • Build user target Model in setup.py
  • Extend Target Model with user defined target data
  • Document
  • test
  • Create Target_Extras migration management command.
@github-project-automation github-project-automation bot moved this to Triage in TOM Toolkit Feb 23, 2024
@jchate6 jchate6 added the enhancement New feature or request label Feb 23, 2024
@jchate6 jchate6 moved this from Triage to Staged in TOM Toolkit Feb 23, 2024
@jchate6 jchate6 moved this from Staged to In progress in TOM Toolkit Mar 7, 2024
@jchate6
Copy link
Contributor Author

jchate6 commented Mar 12, 2024

Setback

The initial attempts at this won't work.
I think we need to re-think our approach.
The linked branch does contain code to allow any data added to Target to be displayed on the Target Detail page and in the update and create forms. It's unclear if this will be useful for future progress.

1st attempt: abstract base class

Incorporating an abstract base class into the existing Target model in tom_targets results in migrations that are incompatible with future updates to tom_targets. These migrations will be made in the tom_targets app itself and will be removed each time tom-toolkit is updated with a new version. This could be made to work potentially by not incorporating pre-made tom_target migrations into tom_base and requiring users to manage.py makemigrations and manage.py migrate each time they updated the base tomtoolkit, but this seems like too great a burden to place on the user.

2nd attempt: Multi-table inheritance

Using Multi-table inheritance to build the Target model with a direct link to a user defined model with its own table (and its own migration directory) still requires a migration to tom_target to incorporate the automatically defined parent link. This parent link has a few issues:

  1. We would have to enforce a name for the user defined target model, and this would be limited to only a single model that we support. This isn't terrible, since in theory, this would just be the connection point from "target" to "user_target" and further extensions would be possible by inheriting "user_target" from other models, but it's not the original structure we had in mind.
  2. Filling in this parent link does not work for existing DB, since existing rows will not have a pk to reference in the new table. This might be able to be accomplished through a manual migration, but again suggests that we might want to re-evaluate to find a better way.
  3. This overwrites the existing target.id with target.userdefinedtarget_ptr which breaks most of the code in tomtoolkit. Again, this can probably be avoided with a manual migration, or pre-defined parent link, but our goal here is simplify extensions to the target model, not complicate them.

The ultimate result is that while this method may work well for brand new DBs that have no existing data, they don't upgrade existing tomtoolkits very well.

Next Steps

I think the next step is to re-evaluate a plan forward from here. Up until now we have been trying to dynamically build a Target Table using user defined fields, but it might be better to have a user defined model that inherits from a tomtoolkit defined "BaseTarget" that then gets interpreted as Target by the rest of the toolkit.

@jchate6 jchate6 moved this from In progress to Triage in TOM Toolkit Mar 12, 2024
@jchate6 jchate6 moved this from Triage to Backlog in TOM Toolkit Mar 15, 2024
@jchate6 jchate6 moved this from Backlog to Staged in TOM Toolkit Mar 20, 2024
@jchate6 jchate6 moved this from Staged to In progress in TOM Toolkit Mar 29, 2024
@jchate6 jchate6 linked a pull request Apr 15, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from In progress to Closed in TOM Toolkit Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant