[gtg-refactor] Failure adding subtask

See a possible fix in the attached branch.

I've not been involved in the refactoring so I'll just drop my thoughts:

That certainly fixes it, but I have two questions:
 - I remember Lionel saying "always pass ids as arguments, never objects except when needed". Shouldn't the tid be passed instead?
 - the add_parent declaration in task.py has a comment "Take a tid object as parameter", but it really wants a task object.

Indeed, this should be maybe refactorised more deeply so that add_parent takes the id as argument.

On Mon, Mar 01, 2010 at 09:24:55AM -0000, Luca Invernizzi wrote:
> I've not been involved in the refactoring so I'll just drop my thoughts:
>
> That certainly fixes it, but I have two questions:
> - I remember Lionel saying "always pass ids as arguments, never objects except when needed". Shouldn't the tid be passed instead?

Yeah, but making the fix this way was a bit too advanced for me.

> - the add_parent declaration in task.py has a comment "Take a tid object as parameter", but it really wants a task object.

I think this is the real bug here. My "fix" just works around this
problem, but maybe helps confirm this as the issue.

Bryce

The usage of add_parent was indeed inconsistent. Fixing this showed a new bug ( #529941)

I'm reading your commit (but I didn't tested). I'm a bit annoyed because now it means that Task and TreeNode have different specifications for the add_parent method, which is bad because Task extends TreeNode.

I think that add_parent should be also fixed in TreeNode and thus everywhere it is used.

I'll have a look.
On Mon, Mar 01, 2010 at 10:31:47AM -0000, Lionel Dricot wrote:
> I'm reading your commit (but I didn't tested). I'm a bit annoyed because
> now it means that Task and TreeNode have different specifications for
> the add_parent method, which is bad because Task extends TreeNode.
>
> I think that add_parent should be also fixed in TreeNode and thus
> everywhere it is used.
>
> --
> [gtg-refactor] Failure adding subtask
> https://bugs.launchpad.net/bugs/529746
> You received this bug notification because you are a bug assignee.
>
> Status in Getting Things GNOME!: Fix Committed
>
> Bug description:
> This is a regression on the gtg-refactor branch.
> Steps to reproduce:
>
> 1. Launch gtg via ./scripts/debug.sh
> 2. Select 'Getting started with GTG'
> 3. Create a new subtask (Ctrl+Shift+N)
>
> A new task is added visible from the taskbrowser, but it's not set as a subtask.
>
> This error is printed:
>
> Traceback (most recent call last):
> File "/home/bryce/src/gtg/gtg-refactor/GTG/taskbrowser/browser.py", line 1203, in on_add_subtask
> task.add_parent(uid)
> File "/home/bryce/src/gtg/gtg-refactor/GTG/core/task.py", line 392, in add_parent
> added = TreeNode.add_parent(self,parent)
> File "/home/bryce/src/gtg/gtg-refactor/GTG/core/tree.py", line 281, in add_parent
> id = par.get_id()
> AttributeError: 'str' object has no attribute 'get_id'
>
>

You're right, Lionel. I believe I fixed that in the latest commit

Excellent !

Also, I'm very pleased to see that there was no need for the object itself in the Tree function. The object reference has virtually disappeared from there, which is good :-)