-
Notifications
You must be signed in to change notification settings - Fork 285
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
Make todo.Task swappable #70
Comments
@shacker fyi, since this is a blocker for me, I'll get started on it now. |
@Fandekasp Since the goal here is to trigger task changes when something elsewhere in the project changes, I think a more flexible (and less invasive) solution would be to take advantage of Django's native signals system.
That would give you or others the flexibility to create additional signals handlers (or arguments to a single handler) with limitless flexibility, no need for model changes, and no need for another dependency (I try to keep the number of required dependencies to a minimum). Then other parts of the app could trigger signals on more than just model changes - they could respond to REST inputs, javascript events, model save() triggers, or anything at all. Can you make an argument in favor of model changes over using signals? Let me know what you think |
@shacker Thanks for the suggestion. I do not see how I can build my feature on signals alone. Maybe you'll have good insight.
As you can see, I need a ManyToOne relationship (foreignkey) between a Task and a Page (in the next release, we'll handle similar workflow with different models, which is why I wrote the contentype generic relation in the example). |
You can do anything from your custom signals. I've created a sample pull request demonstrating how the system could be used. This is very bare bones but demonstrates that the flexibility is unlimited: I really think this is the way to go. Less invasive, more flexible, more extensible, more portable to other use cases. In fact... I need to think about this a bit more, but I'm not even sure django-todo needs to provide anything in this department. Any project can define its own signal handlers and receivers in whatever way makes sense for them. But maybe there is some core stuff (like the complete toggle) we should provide as a reference. |
Your mention of needing "a hook to auto-close the title task" is a good example of a need that's specific to your use case, rather than a general use case. Easily provided by signals. As to your need for a db relation between Task and Page, that is something you can create from within your project. |
@shacker if I understand properly, your thinking goes as follow:
This doesn't sound optimal :( I'll spend more time clarifying my understanding of django signals, wagtail hooks, and Django good patterns. |
More specifically, fine to create those relationships, but you should create them from within your own project. Modifying core todo models is a pretty tectonic shift and un-needed by anyone else until now. Django projects take a million different forms. The best goal of this app is to stay vanilla and let people integrate it however they need to.
Sure. Or just simple FKs or M2Ms between your models and Task or TaskList. Whatever you need!
Absolutely. Then you can always write whatever code you need to do whatever you need without needing to modify any models or migrate anything, or to require migrations in a 3rd-party app! The optimal design pattern is that each app should limit its scope as much as possible. "Small pieces, loosely joined" (the Unix philosophy). Emphasis on "loosely joined." It's what prevents software from becoming spaghetti. |
@shacker fair enough. Thanks for your support. |
Usecase: Extend the Task model to add a generic foreignkey to another model.
This will allow us then to set triggers to complete a task when an action is performed on its related model.
Concrete example: Link a wagtail page to a task when creating it, then auto-complete the task when the wagtail Page status field has been changed.
3rd-part code using todo:
todo suggested modification using swapper
If you agree with this feature request, I can do a PR for it tomorrow (japan timezone)
The text was updated successfully, but these errors were encountered: