Document, verify and test every kind of Todo.target
Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.
Let's revive this really old issue from 2019.
While trying to convert Todos App into a Vue app, we keep running into issues with unknown targets of Todos. So far we had the need for two MRs: !166744 (merged), !167042 (merged) and in the past there already was #243447 (closed).
The problem is that the todo target is a polymorphic type, meaning that the TODO database table looks something like this:
| target_type | target_id | action | ...more fields | note |
|---|---|---|---|---|
| Project | 12 | 10 | Member access request on Project 12 | |
| DesignManagement::Design | 12 | 2 | Mentioned on Design 12 |
So for each ToDo we save the target_type (the rails model class name) as a String and the id of that entity. Thanks @ahegyi for making me aware: !166744 (comment 2126979204).
We don't have a definite list of what can be a target. At the moment it is:
AlertManagement::Alert
Commit
DesignManagement::Design
Epic
Issue
MergeRequest
Namespace
Project
Vulnerability
WorkItem
This places the following problems:
- New target models could be added at a moments notice, potentially breaking the GraphQL interface or other functionality or parts of the functionality if not implemented correctly.
- We really discourage polymorphic types alltogether: https://docs.gitlab.com/ee/development/database/polymorphic_associations.html because it blows up the database.
The question is where we should go from here:
- We should really make sure that we add tests, so that adding a new Todo would require the correct implementations in all places
- We should figure out a way to change the DB structure to be more efficient
Original description
Todo#target is polymorphic. Where we have polymorphic associations, we need to have some idea what kinds of operations they support. We need to clearly:
- document what kinds of things can be a todo-target
- document what interfaces a new todo-target must implement
- have a validation for Todo that the target is a valid target
- test the full suite of actions (dashboard, api) with every possible kind of target.
We should have a new todo-target factory that can generate one of each kind, and a way to generate a collection with every kind.