/trunk/apps/dash/templates/dash/table.html (Diff revision 2)

I'm unsure about doing this.

Cell should always be a tuple when it is a link where the first item is the link itself, and the second item is link text.

Another option would be to always have cell be a tuple where the second item would be empty if cell was not a link.

Overall, I prefer the first way but would like some input.

/trunk/apps/dash/sections.py (Diff revision 3)

since items already come out of a project

items = get_visible(user, project, self.model)

is it necessary to #get_row for item _and_ project?

1. Ted Tate 9 months, 1 week ago (October 20th, 2009, 12:45 a.m.)

   Nope. I must have added it in during an earlier iteration of this patch and forgot to remove it.

/trunk/apps/dash/templates/dash/table.html (Diff revision 3)

to weight in, if a cell is not a link, then we shouldn't construct <a> tags.

1. Ted Tate 9 months, 1 week ago (October 20th, 2009, 12:45 a.m.)

   Agreed. I think I have this one fairly sorted now.

/trunk/apps/dash/tests/sections.doctest (Diff revision 3)

but "project" is instance.project. The new argument doesn't appear to be used.

1. Ted Tate 9 months, 1 week ago (October 20th, 2009, 12:46 a.m.)

   Agreed.

/trunk/apps/tickets/dash_data.py (Diff revision 3)

instead of project.slug, can't we use ticket.project.slug ?

Some suggestions.

/trunk/apps/dash/sections.py (Diff revision 3)

You are not using project in self.get_row. Why did you add that parameter?

1. Ted Tate 9 months, 1 week ago (October 20th, 2009, 12:25 a.m.)

   Loss of sanity I assume.
   
   Fixed.

/trunk/apps/dash/templates/dash/table.html (Diff revision 3)

{% for link,text in row %}
  {% if link %}
    <a href={{ link }}>{{ text|default:"-" }}</a>
  {% else %}
    {{ text|default:"-" }}
{% endif %}

row will be a generator(something iterable) in which the first element can be a link or None and the second element is the text.

1. Ted Tate 9 months, 1 week ago (October 19th, 2009, 9:08 p.m.)

   Ahhh, the solution I was looking for. I like this much more than the way I was doing it.

/trunk/apps/dash/tests/sections.doctest (Diff revision 3)

You should include a get_absolute_url (http://docs.djangoproject.com/en/dev/ref/models/instances/#django.db.models.Model.get_absolute_url) method in our FakeEvent model.

from itertools import izip_longest

row_texts = (instance.title, ...)
link = intance.get_absolute_url()
# Fills with None.
return izip_longest([link], row_texts)

1. Ted Tate 9 months, 1 week ago (October 20th, 2009, 12:38 a.m.)

   So I just discovered that Python 2.5 does not include izip_longest (just izip). I don't think it should be a big issue if we just use zip here for the time being. The only issue is we are being a bit verbose when only having links in the first columns of our table.

2. Zuzel Vera Pacheco 9 months, 1 week ago (October 20th, 2009, 4:13 a.m.)

   Note the import line (from itertools import izip_longest). itertools is a useful module :)

3. Ted Tate 9 months, 1 week ago (October 20th, 2009, 10:38 a.m.)

   Sorry for being unclear. izip_longest was added in the Python 2.6 version of itertools.
   
   Python 2.5.2 (r252:60911, Oct  5 2008, 19:24:49) 
   ...
   >>> import itertools
   >>> dir(itertools)
   ['__doc__', '__file__', '__name__', 'chain', 'count', 'cycle', 'dropwhile', 'groupby', 'ifilter', 'ifilterfalse', 'imap', 'islice', 'izip', 'repeat', 'starmap', 'takewhile', 'tee']
   

/trunk/apps/tickets/dash_data.py (Diff revision 3)

ticket url is: ticket.get_absolute_url()

Take a look at apps/tickets/models.py line 51

1. Ted Tate 9 months, 1 week ago (October 20th, 2009, 12:38 a.m.)

   Fixed.

/trunk/apps/tickets/dash_data.py (Diff revision 3)

from itertools import izip_longest

row_texts = (ticket.number, ...)
link = ticket.get_absolute_url()
# Fills with None.
return izip_longest([link], row_texts)

1. Ted Tate 9 months, 1 week ago (October 20th, 2009, 12:39 a.m.)

   Same comment as above. Currently replaced with this:
   
   links = ticket.get_absolute_url(), None, None, None
   text = ticket.number, ticket.owner, ticket.subject,  formatted_date
   return zip(links,text)

Looks ok :) I will update my diff after your commit so there are no conflicts. -- Waiting for your review.

/trunk/apps/tickets/dash_data.py (Diff revision 4)

This import line is not necessary (you can remove it before the commit, no need to update the diff on review board)