feat: Reclassify nomcom feedback

[pselkirk]

Fixes #4669

[codecov]

Codecov Report

Merging #6002 (764a8f7) into main (416ffb0) will decrease coverage by 0.01%.
Report is 4 commits behind head on main.
The diff coverage is 85.10%.

@@            Coverage Diff             @@
##             main    #6002      +/-   ##
==========================================
- Coverage   88.67%   88.66%   -0.01%     
==========================================
  Files         290      290              
  Lines       40195    40226      +31     
==========================================
+ Hits        35642    35667      +25     
- Misses       4553     4559       +6     

Files Changed	Coverage Δ
ietf/nomcom/views.py	92.57% <84.44%> (-0.33%)	⬇️
ietf/name/models.py	100.00% <100.00%> (ø)
ietf/nomcom/forms.py	95.39% <100.00%> (ø)

... and 3 files with indirect coverage changes

[richsalz]

So this adds a "reclassify" action, that puts already-classified feedback back in the work queue. That's a reasonable approach. This looks fine to me.

[jennifer-richards]

I think the UI needs to indicate that it's changed from "viewing" to "reclassifying". As it is, it's hard to see that the "Reclassify feedback" menu item actually had any effect or that clicking on the individual items will do other than view the feedback.

I wonder if a button on each individual feedback item's view page would be easier to implement in a way that's clearly visible to the user. I.e., open the "View feedback" tab, click the feedback item, then click the "reclassify" button on it to go to a confirmation view.

If you stick with the batch reclassify mode, there are also some glitches on the view_feedback_nominee.html template - the feedback type tabs don't seem to do anything. That should be cleaned up.

[pselkirk]

Robert asked for an "overcome by events" feedback type, which was one of the motivators for this feature, but I feel like it's orthogonal to the actual functionality of reclassification, and might be better handled in a separate ticket.

[rjsparks]

It's what things will often be reclassified to.
I'd like to see it as part of this PR, but a companion that merged near the same time would be sufficient

[jennifer-richards]

I have a number of small things, plus a couple non-trivial requests. One (the HTTP_REFERER business) I think needs to be sorted out now, the other (using a ModelForm) could wait if you're not enthusiastic to jump on it.

I like the "Reclassify" button format, but I'm wondering whether we really need the reclassify mode at all. Is there any reason not to, say, shrink the button a little and perhaps relocate it, then just always have it present? I think that would do away with the need to deal with the link in the base template and eliminate the need that led to the HTTP_REFERER business.

[jennifer-richards]

While you're in the area, would you mind cleaning up the indentation / line breaks here?

[jennifer-richards]

Although it's done a few times in this app (and too much elsewhere in the datatracker), I think we should get away from directly accessing request parameters. I think that a ModelForm on Feedback set only to modify the type field would be preferable. With a ClassifyFeedbackForm (and possibly a couple subclasses with different querysets for the type options), the various places where the code currently decides what types to allow could be consolidated.

If you're not enthusiastic to drag this PR out further, I don't object to making that a code cleanup project for later.

[pselkirk]

I like the "Reclassify" button format, but I'm wondering whether we really need the reclassify mode at all. Is there any reason not to, say, shrink the button a little and perhaps relocate it, then just always have it present?

My feeling was that reclassification would be uncommon enough that a) there should be an explicit action to initiate it, and b) having the button there for the 99.99% of the time that you're not reclassifying would be visual clutter.

OTOH, I'm not the target user. @richsalz @martinthomson

[martinthomson]

I'd have to see this in action to be able to reach any sort of conclusion.

[richsalz]

It's easy to ignore a button if it's always there. If it comes and goes, that's a problem.

[jennifer-richards]

My feeling was that reclassification would be uncommon enough that a) there should be an explicit action to initiate it, and b) having the button there for the 99.99% of the time that you're not reclassifying would be visual clutter.

I agree it'll likely be uncommon, but I think that's a reason to keep it closer at hand in the usual view. With the button always present, when the chair comes across a piece of feedback that's incorrectly classified they won't have to remember to look through the "chair tasks" menu and then find the feedback again.

I think shrinking the button will reduce the clutter to a tolerable level. I can imagine we eventually associate a pulldown menu with each feedback item that has the reclassify view as one of a few options (but, emphatically, am not proposing that right now!).

As a bonus it'll simplify the views that are doing double-duty here.

[jennifer-richards]

Looks good to me!

Your worry about the "Reclassify" being a little obtrusive (even with the small button) is a reasonable one, but I think this is a good starting point. If it annoys the chair, we can restyle the view after we have some experience with how this is used.

[richsalz]

We don't need "button flavor of the month" :) This is good. Thanks! And future nomcom chairs will thank you, too.