[Ovirt-devel] [PATCH] Add task list to dashboard

[Scott Seago]

David Lutterkort wrote:
>
>   
>>> Not directly a comment about this patch: it would be cleaner if we had a
>>> TaskController with an index action that accepts parameters to restrict
>>> the list of tasks to a certain object, e.g. a hw pool or a vm pool.
>>>
>>> The implementation with TaskActions and the index -> show_tasks -> show
>>> call sequence seems a little roundabout.
>>>
>>>   
>>>       
>> I tried to pull all of the generic stuff (that applies to more than one 
>> of the 3 controllers that have task lists) into the task_actions module 
>> which is included by the controllers that have task lists. I suppose we 
>> could have a separate controller with 3 different actions  relating to 
>> dashboard, vm, and hw pool, but it doesn't really match the model we're 
>> already using for VM pool and HW pool controllers -- where one 
>> controller handles the various UI tabs for an object type. So the bits 
>> common to all task lists are already pulled out into task_actions -- and 
>> on the view side in task/_show and task/_grid.
>>     
>
> What I specifically object to is the TaskActions module. Simply for data
> retrieval, the way to think about this is that there is a collection of
> tasks that gets filtered by various criteria - some of them
> user-supplied, and some of them context-dependant.
>
> I would simply have a TaskController with only an index action; to
> display tasks for a VM pool, pass in a vm_pool_id request param, for a
> hardware pool a hw_pool_id etc.
>
>   
And a username for the dashboard task list -- i.e. for the dashboard we 
want all tasks initiated by a given user.
> Regardless of how this is done for the WUI, for the API, we'll need a
> TaskController to pull down lists of tasks.
>
>   
My concern is that this will be a little more convoluted to put all of 
this stuff in a single TaskController, since the behavior is slightly 
different in the three contexts. For the VM pool case, for example, we 
don't have a list of types to filter on (since they're only VM tasks). 
For the dashboard one, the query itself is different, since instead of 
calling (vm,hw)pool.tasks we have to call Task.find(...) with the 
username set in the conditions clause.

Also for the WUI we're handling the permissions, etc. at the pool 
controller.

On the other hand having an index api method on the Task controller 
makes perfect sense for the API case -- maybe the way to accomodate both 
uses would be to simply also include TaskActions in the task controller 
and make TaskController.index call the right  methods to return exactly 
what you need.
>>  Some of the roundabout 
>> nature here has to do with the way the flexigrid setup works -- we need 
>> one action to generate the page itself -- with navigation, filters, 
>> etc., and another action to return the json array for the table content.
>>     
>
> To be clear: I was only talking about how the json array is generated.
>
>   
And it may be that we can find a way to put the json array stuff all 
into the TaskController but leave the rest of it where it is...

Scott