Review Request 111751: Refresh TaskStatusItemModel when a Node changes

[Friedrich W. H. Kossebau]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111751/#review36677
-----------------------------------------------------------


Fix would work, but... I am not happy with it, sorry. I hope the reasoning makes sense to you.
If so, then please consider updating the patch with something as proposed.
If not, please tell.


plan/libs/models/kpttaskstatusmodel.cpp
<http://git.reviewboard.kde.org/r/111751/#comment27074>

    If calling "refresh()" at the end anyway, then all the logic with the row before and emitting dataChanged is unneeded, from what I understand of the rest of the code.
    
    Ideally the "TaskStatusItemModel::slotNodeChanged(...)" method would have some additional logic which checks if the task is still in the same group. Possibly there could be a private enum listing all the groups ("TaskStatus {TaskStarted, TaskRunning, TaskUpcoming, TaskNotStarted }") and a separate, private method which estimates the group for a given task ("TaskStatus TaskStatusItemModel::statusOfNode( Node *node )"). The latter would have the code to estimate the group just taken from "TaskStatusItemModel::refresh()" which would after that then use "TaskStatusItemModel::statusOfNode(...)" instead.
    With that "TaskStatusItemModel::slotNodeChanged( Node *node )" could use "TaskStatusItemModel::statusOfNode(...)" to see if the new status matches the old one known, and in the case it does not go and call "refresh()", but in all other cases just emit dataChanged() as before.
    To get the proper group via the enum it needs to be ensured that the order of the groups in "QList <KPlato::NodeMap* > m_top" matches the value of the status in the enum, so "m_top[status]" will get it.
    
    Got what I mean?


- Friedrich W. H. Kossebau


On July 27, 2013, 8:58 p.m., Alvaro Soliverez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111751/
> -----------------------------------------------------------
> 
> (Updated July 27, 2013, 8:58 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> When updating progress of a task, the Task Status view does not refresh accordingly. If the task was already in the view (it shows tasks only within a timeframe or tasks in progress), it will not change groups if the task changes status. If the task was not in the view, and for example, it is set to started, it will not show up in the view.
> 
> call refresh() after slotNodeChanged()
> 
> 
> This addresses bug 322897.
>     http://bugs.kde.org/show_bug.cgi?id=322897
> 
> 
> Diffs
> -----
> 
>   plan/libs/models/kpttaskstatusmodel.cpp 6257ea3 
> 
> Diff: http://git.reviewboard.kde.org/r/111751/diff/
> 
> 
> Testing
> -------
> 
> Tested with a live file, now it refreshes correctly in both cases
> 
> 
> Thanks,
> 
> Alvaro Soliverez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130728/606bbd31/attachment.htm>