fix: restore ability to create status change documents

[rjsparks]

Fixes #5962

[codecov]

Codecov Report

Merging #5963 (b65a318) into main (09f3477) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head b65a318 differs from pull request most recent head 5710b7a. Consider uploading reports for the commit 5710b7a to get more accurate results

@@            Coverage Diff             @@
##             main    #5963      +/-   ##
==========================================
- Coverage   88.67%   88.64%   -0.04%     
==========================================
  Files         288      288              
  Lines       40001    40003       +2     
==========================================
- Hits        35471    35459      -12     
- Misses       4530     4544      +14     

Impacted Files	Coverage Δ
ietf/doc/utils.py	87.15% <100.00%> (+0.01%)	⬆️
ietf/doc/views_status_change.py	93.38% <100.00%> (ø)
ietf/submit/forms.py	79.02% <100.00%> (+0.03%)	⬆️

... and 5 files with indirect coverage changes

[pselkirk]

As a purely stylistic matter, I tend to prefer "truthy" tests like if status_change.ad vs. if status_change.ad is not None or {% if not doc.ad %} vs. {% if doc.ad is None %}.

That said, it appears to do what it says on the label.

[rjsparks]

As a purely stylistic matter, I tend to prefer "truthy" tests

I've been getting pressure to make more explicit checks, particularly in templates, when the real point of consideration is whether the thing is None or not. In this particular case, I don't think it makes a difference, but if someone configured an AD with a str that returned "", there are other kinds of conditionals or use of {{ ad }} that would likely do the wrong thing.

[jennifer-richards]

I've been getting pressure to make more explicit checks, particularly in templates, when the real point of consideration is whether the thing is None or not.

As a source of this pressure, I prefer it because it distinguishes the absence of value from the possible presence of a value that happens to evaluate as False. It's largely stylistic, but anecdotally I've run into more bugs caused by a surprising False value than by inappropriately testing for is None.

It's a little tricky in templates sometimes - Django represents the absence of a value in some of its fields (like CharField) as an empty string rather than None, so it's impractical to be too dogmatic.

[jennifer-richards]

In #5962, you said

The tests have not exposed this, so they need to be inspected across the entire status-change workflow.

Are you satisfied with having looked over that for this PR or should we plan that inspection separately?

[rjsparks]

Are you satisfied with having looked over (the tests) or should we plan that inspection separately?

I'm not really satisfied, but I did make a quick pass through looking for case-sensitivity troubles. I think this particular fail was a late surprise because of the way we test forms (poking hand-written POST values in that may not actually reflect what a browser would have provided exercising the defined form). So I think it's enough of an edge that we move on, and watch for a framework that works that would allow more of an integrated test of the use of the form.

[jennifer-richards]

One suggested syntax change to consider, but looks good to me.

[jennifer-richards]

Parentheses might be in order here - this is if A or (B and C): I think, which makes sense but it'd help readability to make the intent more explicit