fix: keep draft-iesg state on expiration. Update action holders. - #8321
Conversation
|
Keeping at draft as #8212 fix isn't complete and to investigate if we can remediate docs recently moved to dead just because of expiry. |
There was a problem hiding this comment.
One minor change suggested (getting the system Person by name instead of pk, though if you want to start a change in our practice that's fine).
More significantly, it looks to me like you might be trying to check that notification emails aren't being sent in a way that I don't think tests that. This might be my misunderstanding. If you are trying to check that, I think you need to test that the skip_community_list_notification flag is set on the event you save. We already have a test that proves the flag works as intended. However, getting the event instance might be a bit involved.
| d.latest_event(StateDocEvent).desc, | ||
| "IESG state changed to <b>I-D Exists</b> from Dead", | ||
| ) | ||
| self.assertEqual(len(outbox), 0) |
There was a problem hiding this comment.
If this is meant to test that community notifications aren't sent, additional work will be needed because the signal handler doesn't actually send messages when running tests. (I'm not that's what you're checking)
|
|
||
|
|
||
| def repair_dead_on_expire(): | ||
| by = Person.objects.get(pk=1) # a.k.a. "(System)" |
There was a problem hiding this comment.
We canonically use Person.objects.get(user__username="(System)"). Better to stick to that?
There was a problem hiding this comment.
looks like it's mostly (name="(System)") rather than user__username?
There was a problem hiding this comment.
Er, yes, name. Sorry, I don't know why I thought I saw that.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8321 +/- ##
=======================================
Coverage 88.72% 88.73%
=======================================
Files 310 312 +2
Lines 40882 40910 +28
=======================================
+ Hits 36273 36301 +28
Misses 4609 4609 ☔ View full report in Codecov by Sentry. |
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
Looks good.
An alternative to counting events and manipulating indexes would be to call mock_notify.reset_mock() before calling your method under test. That'll empty call_args and reset called to False. Purely optional though.
| empty_outbox() | ||
| notified_during_factory_work = mock_notify.call_count | ||
| for call_args in mock_notify.call_args_list: | ||
| e = call_args.args[0] |
There was a problem hiding this comment.
Wow! I had missed that call_args now has args and kwargs properties! That is so much better than call_args[0][0].
2 participants
Fixes #5560 and #8212