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

Task assignee issue #19

Open
jordotech opened this issue Oct 6, 2020 · 6 comments
Open

Task assignee issue #19

jordotech opened this issue Oct 6, 2020 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@jordotech
Copy link
Contributor

I am currently adding assignees to a Task object via a post_save signal by matching the task.name against a string. Is it possible to add an assignee from within a workflow method?

@codingjoe codingjoe added the enhancement New feature or request label Oct 6, 2020
@codingjoe
Copy link
Owner

Hi @jordotech,

Thanks for reaching out. You make an excellent point. I honestly just added the field for future usage, but haven't come around to actually implement user assignment and something resembling a task inbox.

Let's talk possible solutions before I go ahead an implement something. I'd really like to get your input on the ideas I have for this.

So, each task creates the next tasks after executing itself successfully. How that is done depends on whether the next task implements a create_task factory method. Take the Join tasks, it needs to create a task slightly differently, as it performs a get_or_create since the task might already exist and is waiting for all incoming edges to complete.

Essentially, you could provide create a create_task method and provide assignees like so:

class MyCustomUpdateView(tasks.UpdateView):
    def create_task(self, workflow):
        return workflow.task_set.create(
            name=self.name,
            type=self.type,
            completed=None,
            asignees=[],  # add your user here
        )

However, there are a couple of problems, that need to be address for this to be viable solution:

  1. This method should maybe receive a request as the context, if possible.
  2. As an instance method, we could potentially experience state leakage.
Regarding 1

Let's say you have a scenario, where you want to assign the next task the same user that currently completes the task at hand. You could get the last task owner form the previous task, but that is not given to the create_task method. We could change that, but there could be other scenarios where the request is very handy.

However, not all tasks are created from a human task. They could be created by a machine tasks. Not having a request would be more consistent. I'd opt for now having a request here.

Regarding 2

Event thought in my example the create_task method is on a class-based view, the method is not called within a view's context. It should probably be a class method, to avoid state leakage. Moreover, I guess it would be counterintuitive a view method, where you can't access self.request or self.kwargs. However, for that to work, we probably need to implement something like a as_task method, to set custom attributes when declaring the workflow. Yet, that would be a breaking change.

I hope my answer wasn't too long. I just wanted to give you all the info you need.

BTW, thanks for becoming a sponsor <3 It really makes a difference, if only emotionally.

Best,
Joe

@codingjoe codingjoe self-assigned this Oct 6, 2020
@jordotech
Copy link
Contributor Author

I agree with you on point 1, I would opt to not using the request object there. For what it's worth, the following is working quite nicely for me and does not introduce breaking changes.

Model

I made a new model where you configure users to task names:

class TaskAssignee(TimeStampedModel):
    role = models.CharField(
        "Role Name",
        max_length=50,
        help_text="Ex. 'Final Approver', 'Preliminary Approver'",
    )
    task_id = models.CharField(
        "Task ID",
        max_length=50,
        help_text="The machine name of the human workflow task that will be assigned to this approver.",
    )
    content_type = models.ForeignKey(
        "contenttypes.ContentType",
        on_delete=models.CASCADE,
        null=True,
        blank=True,
        limit_choices_to=workflow_state_subclasses,
        related_name="joeflow_assignee_set",
    )
   # todo: Add FK to content type
    user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE,)

    class Meta:
        verbose_name = _("Task Assignee")
        verbose_name_plural = _("Task Assignees")

    def __str__(self):
        return f"{self.role}"

The problem with the above is that I'm manually entering the task method name into the admin form which is not very elegant. Maybe the field could be a choices field that pulls options from a new setting

JOEFLOW_ASSIGNABLE_TASKS = ["prelim_approve", "final_approve"]

Signal

Then a post_save signal that adds the assignees at Task creation if there's a match.

@receiver(post_save, sender=Task)
def assign_task(sender, instance, created, raw, using, update_fields, **kwargs):
        if created:
        if instance.type == 'human':
            task_content_type = ContentType.objects.get_for_model(instance.workflow)
            for approver in Approver.objects.filter(task_id=instance.name, content_type=task_content_type):
                instance.assignees.add(approver.user)

Now I have proper "My Tasks" dashboards with pending/complete tasks.

The main problem I have faced with joeflow is referencing the Task object that is associated with a machine task.

For example, I have not been able to implement the following (simplified) workflow. The idea is to email someone that they've been assigned and provide a link to the next UpdateView but those Task objects aren't created yet and I'm unable to reference the Task object from within the machine task method... hence the signal code.

class ApproveWorkflow(Workflow):

    # fields...

    start = tasks.Start()

    prelim_approve = UpdateView(fields=["approved"])

    def has_approval(self):
        if self.approved:
            return [self.end]

    final_approve = ProtectedUpdateView(fields=["final_approval"])  
    
    def assign_task(self):
        # Cannot assign b/c next Task is not created yet
        pass
    
    def notify_assignee(self):
        # send email to assignee with link to next human step
        pass

    def end(self):
        pass

    edges = (
        (start, assign_task),
        (assign_task, notify_assignee),
        (notify_assignee, approve),
        (approve, has_approval),
        (has_approval, end),
    )

@codingjoe
Copy link
Owner

Hi @jordotech,

Thanks for your patience. I needed a little to find time for a thorough response.

No request. Great, then that's how we will build it.

Tasks assignment is a tricky thing, I agree. Especially if you have want to assign a task that does not exist yet.
Let me try to understand better, what you want to achieve. You are looking to build a view, where people can assign a task to someone, right? Would it be sufficient to have a task inbox, where people find their own and unassigned tasks? It seems you also want to notify assignees, after they have been assigned a task right?

I would probably separate the two scenarios. One where tasks get auto-assigned and one where tasks get assigned by humans.

For the latter a simple task create method should be sufficient. For the further we will need two views. One that resembles a tasks inbox and one that handles reassignments and notifications. If a user can reassign a tasks or not could be restricted by permissions. However, those should probably be per process not per task. A task could have a has_perm function to decide on a per task base if a user has the permission to reassign a task.

If that worked for you, I would go ahead and try to implement a solution over the weekend and add documentation on how to extend the default behavior.

Best,
Joe

codingjoe added a commit that referenced this issue Oct 18, 2020
@codingjoe
Copy link
Owner

@jordotech I worked on a draft this weekend on how to do task assignment. I'd really like your feedback. I will try to find time soon to also develop a proper task inbox.

@jordotech
Copy link
Contributor Author

Thanks for that! I just reviewed the additions, looking good.

Regarding the UpdateWithPrevUserView:
That human form not have any inputs, correct? Would it not be better to present a select input of all available assignees? The form could initialize with the prev_task.completed_by_user (if prev_task supplied). I think it would cover both scenarios. But yea, this is an important addition to the package, well done.

For reference, this is a flowchart of the real workflow I've developed with joeflow:
IT Request Diagram

In this scenario there are 2 Django users who are always auto-assigned as "preliminary approver" and "final approver".

codingjoe added a commit that referenced this issue Nov 3, 2020
codingjoe added a commit that referenced this issue Nov 3, 2020
@codingjoe
Copy link
Owner

Yes, I too thought of a select input, similar to how issues and pull-requests are assigned here on GitHub. I am finishing up the create_task part now and release it. The whole inbox and reassignment part, should go in a separate PR, to keep change sets small.

codingjoe added a commit that referenced this issue Nov 3, 2020
codingjoe added a commit that referenced this issue Nov 3, 2020
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
None yet
Development

No branches or pull requests

2 participants