512618 - Bugzilla::Bug::choices should return Field::Choice objects, not just values

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

I'd like to start using choices(), more, everywhere, because it allows us to centralize "what values this user can set on this bug" in the backend code instead of the templates. However, right now field.html.tmpl expects Field::Choice objects, while choices() returns just values. So choices() should return Field::Choice objects instead, and also should really only load legal_values when they are asked for.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

So, it turns out that there are very few things actually still using choices(). The [% BLOCK select %] in bug/edit.html.tmpl is now only used for per-product fields, and then choices() is also used to get valid resolutions. So I now only include information for those fields, in choices(), for now.

Comment 2

[Frédéric Buclin]

Comment on attachment 396648 [details] [diff] [review]
v1

(In reply to comment #0)
> I'd like to start using choices(), more, everywhere

I rather want to kill choices() entirely. We don't need it, really. Components, versions and milestones can already be obtained by [% bug.product.obj.$foo %] which has the same effect as [% bug.choices.$foo %]. You should fix [% BLOCK select %] to stop using choices().

This leaves us with a single place using bug.choices.product, which you introduced, and a single place using choices.resolution, which would partially be fixed by bug 106589. The logic about MOVED can remain where it is now.

Comment 3

[Max Kanat-Alexander]

Comment on attachment 396648 [details] [diff] [review]
v1

(In reply to comment #2)
> I rather want to kill choices() entirely. We don't need it, really. 

  Well, *after* this bug, I'd be happy to get rid of choices(), if you want. I actually think it's somewhat useful given the current architecture of the code. I used to agree with you entirely, and I was actually originally going to kill choices() instead of just re-work it, but I found that the architecture, for now, was nicer with choices() than without.

  Also, this blocks a really important other enhancement, and it's also a nice cleanup of the existing code.

Comment 4

[Frédéric Buclin]

Comment on attachment 396648 [details] [diff] [review]
v1

So be sure to file a bug about killing choices() entirely. But for now, fix BLOCK select to use bug.product_obj.$selname instead of bug.choices.$selname, and remove the corresponding entries from choices().

Comment 5

[Max Kanat-Alexander]

Attachment: v2

Okay, here you go.

Comment 6

[Frédéric Buclin]

Comment on attachment 403714 [details] [diff] [review]
v2

>Index: template/en/default/bug/knob.html.tmpl

>+        [% get_resolution(r.name) FILTER html %]</option>

It conflicts with bug 512606, which I just reviewed and which removes get_resolution(). Could you unbitrot it and resubmit?

Comment 7

[Frédéric Buclin]

(In reply to comment #6)
> It conflicts with bug 512606

I meant bug 512623.

Comment 8

[Max Kanat-Alexander]

Attachment: v3

Okay, unbitrotted.

Comment 9

[Max Kanat-Alexander]

Attachment: v4

It turns out that product_obj.${selname} doesn't match the field names. I've gone back to using choices instead, because that makes things much simpler in the templates (it actually matches up against the field names). At some point this will all be obsoleted by using bug/field.html.tmpl instead, and you can file a bug to do that if you'd like.

Comment 10

[Frédéric Buclin]

Comment on attachment 403904 [details] [diff] [review]
v4

r=LpSolit

Comment 11

[Max Kanat-Alexander]

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.289; previous revision: 1.288
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.166; previous revision: 1.165
done
Checking in template/en/default/bug/knob.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/knob.html.tmpl,v  <--  knob.html.tmpl
new revision: 1.44; previous revision: 1.43
done