-
Notifications
You must be signed in to change notification settings - Fork 378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: remove "AD is watching" state #7960
Merged
rjsparks
merged 12 commits into
ietf-tools:main
from
jennifer-richards:remove-ad-is-watching-state
Sep 24, 2024
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
b9f6c95
feat: remove "AD is watching" state
jennifer-richards e089a60
chore: update names.json
jennifer-richards 52701ba
refactor: use idexists state, not dead
jennifer-richards 3f32c81
fix: remove guidance to use watching state
jennifer-richards cce67bd
chore: remove references to 'watching' state
jennifer-richards 6e1e851
feat: remove create_in_state from edit_info view
jennifer-richards fb0a7ec
test: update test
jennifer-richards ab88f49
style: Black + move class closer to use
jennifer-richards 37040fc
Merge branch 'main' into remove-ad-is-watching-state
jennifer-richards f7e8251
refactor: remove lint
jennifer-richards b290071
fix: restore missing admonition
jennifer-richards ae1b50f
ci: Merge branch 'main' into remove-ad-is-watching-state
rjsparks File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
121 changes: 121 additions & 0 deletions
121
ietf/doc/migrations/0024_remove_ad_is_watching_states.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
# Copyright The IETF Trust 2024, All Rights Reserved | ||
|
||
from django.db import migrations | ||
|
||
|
||
def get_helper(DocHistory, RelatedDocument, RelatedDocHistory, DocumentAuthor, DocHistoryAuthor): | ||
"""Dependency injection wrapper""" | ||
|
||
def save_document_in_history(doc): | ||
"""Save a snapshot of document and related objects in the database. | ||
Local copy of ietf.doc.utils.save_document_in_history() to avoid depending on the | ||
code base in a migration. | ||
""" | ||
|
||
def get_model_fields_as_dict(obj): | ||
return dict((field.name, getattr(obj, field.name)) | ||
for field in obj._meta.fields | ||
if field is not obj._meta.pk) | ||
|
||
# copy fields | ||
fields = get_model_fields_as_dict(doc) | ||
fields["doc"] = doc | ||
fields["name"] = doc.name | ||
|
||
dochist = DocHistory(**fields) | ||
dochist.save() | ||
|
||
# copy many to many | ||
for field in doc._meta.many_to_many: | ||
if field.remote_field.through and field.remote_field.through._meta.auto_created: | ||
hist_field = getattr(dochist, field.name) | ||
hist_field.clear() | ||
hist_field.set(getattr(doc, field.name).all()) | ||
|
||
# copy remaining tricky many to many | ||
def transfer_fields(obj, HistModel): | ||
mfields = get_model_fields_as_dict(item) | ||
# map doc -> dochist | ||
for k, v in mfields.items(): | ||
if v == doc: | ||
mfields[k] = dochist | ||
HistModel.objects.create(**mfields) | ||
|
||
for item in RelatedDocument.objects.filter(source=doc): | ||
transfer_fields(item, RelatedDocHistory) | ||
|
||
for item in DocumentAuthor.objects.filter(document=doc): | ||
transfer_fields(item, DocHistoryAuthor) | ||
|
||
return dochist | ||
|
||
return save_document_in_history | ||
|
||
|
||
def forward(apps, schema_editor): | ||
"""Mark watching draft-iesg state unused after removing it from Documents""" | ||
StateDocEvent = apps.get_model("doc", "StateDocEvent") | ||
Document = apps.get_model("doc", "Document") | ||
State = apps.get_model("doc", "State") | ||
StateType = apps.get_model("doc", "StateType") | ||
Person = apps.get_model("person", "Person") | ||
|
||
save_document_in_history = get_helper( | ||
DocHistory=apps.get_model("doc", "DocHistory"), | ||
RelatedDocument=apps.get_model("doc", "RelatedDocument"), | ||
RelatedDocHistory=apps.get_model("doc", "RelatedDocHistory"), | ||
DocumentAuthor=apps.get_model("doc", "DocumentAuthor"), | ||
DocHistoryAuthor=apps.get_model("doc", "DocHistoryAuthor"), | ||
) | ||
|
||
draft_iesg_state_type = StateType.objects.get(slug="draft-iesg") | ||
idexists_state = State.objects.get(type=draft_iesg_state_type, slug="idexists") | ||
watching_state = State.objects.get(type=draft_iesg_state_type, slug="watching") | ||
system_person = Person.objects.get(name="(System)") | ||
|
||
# Remove state from documents that currently have it | ||
for doc in Document.objects.filter(states=watching_state): | ||
assert doc.type_id == "draft" | ||
doc.states.remove(watching_state) | ||
doc.states.add(idexists_state) | ||
e = StateDocEvent.objects.create( | ||
type="changed_state", | ||
by=system_person, | ||
doc=doc, | ||
rev=doc.rev, | ||
desc=f"{draft_iesg_state_type.label} changed to <b>{idexists_state.name}</b> from {watching_state.name}", | ||
state_type=draft_iesg_state_type, | ||
state=idexists_state, | ||
) | ||
doc.time = e.time | ||
doc.save() | ||
save_document_in_history(doc) | ||
assert not Document.objects.filter(states=watching_state).exists() | ||
|
||
# Mark state as unused | ||
watching_state.used = False | ||
watching_state.save() | ||
|
||
|
||
def reverse(apps, schema_editor): | ||
"""Mark watching draft-iesg state as used | ||
Does not try to re-apply the state to Documents modified by the forward migration. This | ||
could be done in theory, but would either require dangerous history rewriting or add a | ||
lot of history junk. | ||
""" | ||
State = apps.get_model("doc", "State") | ||
StateType = apps.get_model("doc", "StateType") | ||
State.objects.filter( | ||
type=StateType.objects.get(slug="draft-iesg"), slug="watching" | ||
).update(used=True) | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
("doc", "0023_bofreqspamstate"), | ||
] | ||
|
||
operations = [migrations.RunPython(forward, reverse)] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ | |
WriteupDocEvent, DocRelationshipName, IanaExpertDocEvent ) | ||
from ietf.doc.utils import get_tags_for_stream_id, create_ballot_if_not_open | ||
from ietf.doc.views_draft import AdoptDraftForm | ||
from ietf.name.models import StreamName, DocTagName, RoleName | ||
from ietf.name.models import DocTagName, RoleName | ||
from ietf.group.factories import GroupFactory, RoleFactory | ||
from ietf.group.models import Group, Role | ||
from ietf.person.factories import PersonFactory, EmailFactory | ||
|
@@ -471,69 +471,61 @@ def test_edit_telechat_date(self): | |
self.assertIn("may not leave enough time", get_payload_text(outbox[-1])) | ||
|
||
def test_start_iesg_process_on_draft(self): | ||
|
||
draft = WgDraftFactory( | ||
name="draft-ietf-mars-test2", | ||
group__acronym='mars', | ||
group__acronym="mars", | ||
intended_std_level_id="ps", | ||
authors=[Person.objects.get(user__username='ad')], | ||
) | ||
url = urlreverse('ietf.doc.views_draft.edit_info', kwargs=dict(name=draft.name)) | ||
authors=[Person.objects.get(user__username="ad")], | ||
) | ||
|
||
url = urlreverse("ietf.doc.views_draft.edit_info", kwargs=dict(name=draft.name)) | ||
login_testing_unauthorized(self, "secretary", url) | ||
|
||
# normal get | ||
r = self.client.get(url) | ||
self.assertEqual(r.status_code, 200) | ||
q = PyQuery(r.content) | ||
self.assertEqual(len(q('form select[name=intended_std_level]')), 1) | ||
self.assertEqual("", q('form textarea[name=notify]')[0].value.strip()) | ||
self.assertEqual(len(q("form select[name=intended_std_level]")), 1) | ||
self.assertEqual("", q("form textarea[name=notify]")[0].value.strip()) | ||
|
||
# add | ||
events_before = draft.docevent_set.count() | ||
events_before = list(draft.docevent_set.values_list("id", flat=True)) | ||
mailbox_before = len(outbox) | ||
|
||
ad = Person.objects.get(name="Areað Irector") | ||
|
||
r = self.client.post(url, | ||
dict(intended_std_level=str(draft.intended_std_level_id), | ||
ad=ad.pk, | ||
create_in_state=State.objects.get(used=True, type="draft-iesg", slug="watching").pk, | ||
notify="[email protected]", | ||
telechat_date="", | ||
)) | ||
r = self.client.post( | ||
url, | ||
dict( | ||
intended_std_level=str(draft.intended_std_level_id), | ||
ad=ad.pk, | ||
notify="[email protected]", | ||
telechat_date="", | ||
), | ||
) | ||
self.assertEqual(r.status_code, 302) | ||
|
||
draft = Document.objects.get(name=draft.name) | ||
self.assertEqual(draft.get_state_slug("draft-iesg"), "watching") | ||
self.assertEqual(draft.get_state_slug("draft-iesg"), "pub-req") | ||
self.assertEqual(draft.get_state_slug("draft-stream-ietf"), "sub-pub") | ||
self.assertEqual(draft.ad, ad) | ||
self.assertTrue(not draft.latest_event(TelechatDocEvent, type="scheduled_for_telechat")) | ||
self.assertEqual(draft.docevent_set.count(), events_before + 4) | ||
self.assertCountEqual(draft.action_holders.all(), [draft.ad]) | ||
events = list(draft.docevent_set.order_by('time', 'id')) | ||
self.assertEqual(events[-4].type, "started_iesg_process") | ||
self.assertEqual(len(outbox), mailbox_before+1) | ||
self.assertTrue('IESG processing' in outbox[-1]['Subject']) | ||
self.assertTrue('draft-ietf-mars-test2@' in outbox[-1]['To']) | ||
|
||
# Redo, starting in publication requested to make sure WG state is also set | ||
draft.set_state(State.objects.get(type_id='draft-iesg', slug='idexists')) | ||
draft.set_state(State.objects.get(type='draft-stream-ietf',slug='writeupw')) | ||
draft.stream = StreamName.objects.get(slug='ietf') | ||
draft.action_holders.clear() | ||
draft.save_with_history([DocEvent.objects.create(doc=draft, rev=draft.rev, type="changed_stream", by=Person.objects.get(user__username="secretary"), desc="Test")]) | ||
r = self.client.post(url, | ||
dict(intended_std_level=str(draft.intended_std_level_id), | ||
ad=ad.pk, | ||
create_in_state=State.objects.get(used=True, type="draft-iesg", slug="pub-req").pk, | ||
notify="[email protected]", | ||
telechat_date="", | ||
)) | ||
self.assertEqual(r.status_code, 302) | ||
draft = Document.objects.get(name=draft.name) | ||
self.assertEqual(draft.get_state_slug('draft-iesg'),'pub-req') | ||
self.assertEqual(draft.get_state_slug('draft-stream-ietf'),'sub-pub') | ||
self.assertTrue( | ||
not draft.latest_event(TelechatDocEvent, type="scheduled_for_telechat") | ||
) | ||
# check that the expected events were created (don't insist on ordering) | ||
self.assertCountEqual( | ||
draft.docevent_set.exclude(id__in=events_before).values_list("type", flat=True), | ||
[ | ||
"changed_action_holders", # action holders set to AD | ||
"changed_document", # WG state set to sub-pub | ||
"changed_document", # AD set | ||
"changed_document", # state change notice email set | ||
"started_iesg_process", # IESG state is now pub-req | ||
], | ||
) | ||
self.assertCountEqual(draft.action_holders.all(), [draft.ad]) | ||
self.assertEqual(len(outbox), mailbox_before + 1) | ||
self.assertTrue("IESG processing" in outbox[-1]["Subject"]) | ||
self.assertTrue("draft-ietf-mars-test2@" in outbox[-1]["To"]) | ||
|
||
def test_edit_consensus(self): | ||
draft = WgDraftFactory() | ||
|
@@ -750,10 +742,6 @@ def test_expire_drafts(self): | |
|
||
self.assertEqual(len(list(get_expired_drafts())), 1) | ||
|
||
draft.set_state(State.objects.get(used=True, type="draft-iesg", slug="watching")) | ||
|
||
self.assertEqual(len(list(get_expired_drafts())), 1) | ||
|
||
draft.set_state(State.objects.get(used=True, type="draft-iesg", slug="iesg-eva")) | ||
|
||
self.assertEqual(len(list(get_expired_drafts())), 0) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cringing at the html tags, but keeping the set of things to mutate later regular was a good call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I share the cringe