Bug#754413: add a description field to keywords
On Fri, 17 Jul 2015, Christophe Siraut wrote:
> Adding a Keyword description field with data works fine, please review
> the attached patch.
I'll take a look at that later. But a few comments are below already.
> The keywords modification form seems broken: here boxes do not shown
> checked for already active subscription keywords; do you remember it
> working recently?
What do you mean? You have no keywords checked at all? Or you only see
a subset of keywords?
I just tried on tracker.debian.org:
- when I click on "Modify keywords" next to an email, then I get nothing
checked
- when I click on "Modify" next to a source package, then I always get
all the default keywords selected, and you're right that it does not
match the set of actual keywords (checked with a package where I had
only the "vcs" keyword activated, namely "tracker.debian.org" :))
- when I click on "Modify" in a "Team subscription", there I get the
correct set of keywords, but the modify button is still hidden
in the expandable part...
=> there's definitely something fishy, but are you sure that the
regression is not cause by your move of the "Modify" button because
strangely it seems to work for the team subscription.
=> we should harmonize the button to say "Modify keywords" in all cases
=> for the keywords at the email level, I'm not sure why it would be empty
by default. It would seem natural to also have the default keywords
checked when there's no user-customize keywords at the email level.
> I do not get the point of making several requests using javascript in
> order to compose that form, see accounts/static/js/profile.js Why do not
> we rely on standard django form facilities?
It's dating back to quite some time so I don't know. If you want to
rewrite it in a cleaner way, by all means go ahead!
> --- a/distro_tracker/accounts/templates/accounts/subscriptions.html
> +++ b/distro_tracker/accounts/templates/accounts/subscriptions.html
> @@ -60,7 +60,7 @@
> <div class="default-keywords" style="display: none;" id="default-keywords-{{ forloop.counter }}">
> <ul>
> {% for keyword in email.default_keywords.all %}
> - <li class="keyword">{{ keyword }}</li>
> + <li title="{{ keyword.description }}" class="keyword">{{ keyword }}</li>
> {% endfor %}
> </ul>
> </div>
In the popup I would like the description to be immediately visible, so
not hidden in a mouse-over title, we have enough width for this.
> @@ -0,0 +1,43 @@
> +# -*- coding: utf-8 -*-
> +from __future__ import unicode_literals
> +
> +from django.db import migrations
> +
> +
> +def describe_keywords(apps, schema_editor):
> + Keyword = apps.get_model('core', 'Keyword')
> + Keyword.objects.filter(name='default').update(description=' all the other \
> +mails (those which aren\'t automatic)')
Should be rewritten with a data structure and a loop:
keywords = {
'default': 'All other discussions',
[...]
}
for keyword, description in keywords.items():
Keywords.objects.filter(name=keyword).update(description=description)
Also please use the descriptions we had here:
https://packages.qa.debian.org/cgi-bin/pts.cgi?package=dpkg&email=foof@bar&what=advanced
(Modulo the keywords that we did rename)
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/
Reply to: