Write to Stages when editing a feature (#2311)

* write to stages upon editing

* docstring

* simplify test

* Update core_models.py
This commit is contained in:
Daniel Smith 2022-10-11 08:58:06 +02:00 коммит произвёл GitHub
Родитель 6111c59a35
Коммит d8b9c00927
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
5 изменённых файлов: 281 добавлений и 2 удалений

Просмотреть файл

@ -143,6 +143,71 @@ STAGE_DEP_REMOVE_CODE = 470
STAGE_ENT_ROLLOUT = 1061
# Prototype stage types for every feature type.
STAGE_TYPES_PROTOTYPE = {
FEATURE_TYPE_INCUBATE_ID: STAGE_BLINK_PROTOTYPE,
FEATURE_TYPE_EXISTING_ID: STAGE_FAST_PROTOTYPE,
FEATURE_TYPE_CODE_CHANGE_ID: None,
FEATURE_TYPE_DEPRECATION_ID: None
}
# Dev trial stage types for every feature type.
STAGE_TYPES_DEV_TRIAL = {
FEATURE_TYPE_INCUBATE_ID: STAGE_BLINK_DEV_TRIAL,
FEATURE_TYPE_EXISTING_ID: STAGE_FAST_DEV_TRIAL,
FEATURE_TYPE_CODE_CHANGE_ID: STAGE_PSA_DEV_TRIAL,
FEATURE_TYPE_DEPRECATION_ID: STAGE_DEP_DEV_TRIAL
}
# Origin trial stage types for every feature type.
STAGE_TYPES_ORIGIN_TRIAL = {
FEATURE_TYPE_INCUBATE_ID: STAGE_BLINK_ORIGIN_TRIAL,
FEATURE_TYPE_EXISTING_ID: STAGE_FAST_ORIGIN_TRIAL,
FEATURE_TYPE_CODE_CHANGE_ID: None,
FEATURE_TYPE_DEPRECATION_ID: STAGE_DEP_DEPRECATION_TRIAL
}
# Origin trial extension stage types for every feature type.
STAGE_TYPES_EXTEND_ORIGIN_TRIAL = {
FEATURE_TYPE_INCUBATE_ID: STAGE_BLINK_EXTEND_ORIGIN_TRIAL,
FEATURE_TYPE_EXISTING_ID: STAGE_FAST_EXTEND_ORIGIN_TRIAL,
FEATURE_TYPE_CODE_CHANGE_ID: None,
FEATURE_TYPE_DEPRECATION_ID: STAGE_DEP_EXTEND_DEPRECATION_TRIAL
}
# Shipping stage types for every feature type.
STAGE_TYPES_SHIPPING = {
FEATURE_TYPE_INCUBATE_ID: STAGE_BLINK_SHIPPING,
FEATURE_TYPE_EXISTING_ID: STAGE_FAST_SHIPPING,
FEATURE_TYPE_CODE_CHANGE_ID: STAGE_PSA_SHIPPING,
FEATURE_TYPE_DEPRECATION_ID: STAGE_DEP_SHIPPING
}
# Mapping of original field names to the new stage types the fields live on.
STAGE_TYPES_BY_FIELD_MAPPING = {
'finch_url': STAGE_TYPES_SHIPPING,
'experiment_goals': STAGE_TYPES_ORIGIN_TRIAL,
'experiment_risks': STAGE_TYPES_ORIGIN_TRIAL,
'experiment_extension_reason': STAGE_TYPES_EXTEND_ORIGIN_TRIAL,
'origin_trial_feedback_url': STAGE_TYPES_ORIGIN_TRIAL,
'intent_to_implement_url': STAGE_TYPES_PROTOTYPE,
'intent_to_ship_url': STAGE_TYPES_SHIPPING,
'intent_to_experiment_url': STAGE_TYPES_ORIGIN_TRIAL,
'intent_to_extend_experiment_url': STAGE_TYPES_EXTEND_ORIGIN_TRIAL,
'shipped_milestone': STAGE_TYPES_SHIPPING,
'shipped_android_milestone': STAGE_TYPES_SHIPPING,
'shipped_ios_milestone': STAGE_TYPES_SHIPPING,
'shipped_webview_milestone': STAGE_TYPES_SHIPPING,
'ot_milestone_desktop_start': STAGE_TYPES_ORIGIN_TRIAL,
'ot_milestone_desktop_end': STAGE_TYPES_ORIGIN_TRIAL,
'ot_milestone_android_start': STAGE_TYPES_ORIGIN_TRIAL,
'ot_milestone_android_end': STAGE_TYPES_ORIGIN_TRIAL,
'ot_milestone_ios_start': STAGE_TYPES_ORIGIN_TRIAL,
'ot_milestone_ios_end': STAGE_TYPES_ORIGIN_TRIAL,
'ot_milestone_webview_start': STAGE_TYPES_ORIGIN_TRIAL,
'ot_milestone_webview_end': STAGE_TYPES_ORIGIN_TRIAL,
'dt_milestone_desktop_start': STAGE_TYPES_DEV_TRIAL,
'dt_milestone_android_start': STAGE_TYPES_DEV_TRIAL,
'dt_milestone_ios_start': STAGE_TYPES_DEV_TRIAL,
'dt_milestone_webview_start': STAGE_TYPES_DEV_TRIAL
}
NO_ACTIVE_DEV = 1
PROPOSED = 2
IN_DEVELOPMENT = 3

Просмотреть файл

@ -1214,6 +1214,28 @@ class FeatureEntry(ndb.Model): # Copy from Feature
# Note: This class is not used yet.
class MilestoneSet(ndb.Model): # copy from milestone fields of Feature
"""Range of milestones during which a feature will be in a certain stage."""
# Mapping of old milestone fields to the new fields they should use
# in the MilestoneSet kind.
MILESTONE_FIELD_MAPPING = {
'shipped_milestone': 'desktop_first',
'shipped_android_milestone': 'android_first',
'shipped_ios_milestone': 'ios_first',
'shipped_webview_milestone': 'webview_first',
'ot_milestone_desktop_start': 'desktop_first',
'ot_milestone_desktop_end': 'desktop_last',
'ot_milestone_android_start': 'android_first',
'ot_milestone_android_end': 'android_last',
'ot_milestone_ios_start': 'ios_first',
'ot_milestone_ios_end': 'ios_last',
'ot_milestone_webview_start': 'webview_first',
'ot_milestone_webview_end': 'webview_last',
'dt_milestone_desktop_start': 'desktop_first',
'dt_milestone_android_start': 'android_first',
'dt_milestone_ios_start': 'ios_first',
'dt_milestone_webview_start': 'webview_first'
}
desktop_first = ndb.IntegerProperty()
desktop_last = ndb.IntegerProperty()
android_first = ndb.IntegerProperty()
@ -1249,3 +1271,9 @@ class Stage(ndb.Model):
experiment_extension_reason = ndb.TextProperty()
intent_thread_url = ndb.StringProperty()
origin_trial_feedback_url = ndb.StringProperty()
@classmethod
def get_feature_stages(cls, feature_id: int) -> dict:
"""Return a dictionary of stages associated with a given feature."""
stages = cls.query(cls.feature_id == feature_id).fetch()
return {stage.stage_type: stage for stage in stages}

Просмотреть файл

@ -361,3 +361,36 @@ class FeatureTest(testing_config.CustomTestCase):
self.assertEqual(
cached_test_feature,
actual)
class StageTest(testing_config.CustomTestCase):
def setUp(self):
self.feature_entry_1 = core_models.FeatureEntry(id=1, name='fe one',
summary='summary', category=1, impl_status_chrome=1,
standard_maturity=1, web_dev_views=1)
self.feature_entry_1.put()
stage_types = [core_enums.STAGE_DEP_PLAN, core_enums.STAGE_DEP_DEV_TRIAL,
core_enums.STAGE_DEP_DEPRECATION_TRIAL, core_enums.STAGE_DEP_SHIPPING,
core_enums.STAGE_DEP_REMOVE_CODE]
self.feature_id = self.feature_entry_1.key.integer_id()
for stage_type in stage_types:
stage = core_models.Stage(feature_id=self.feature_id, stage_type=stage_type)
stage.put()
def tearDown(self):
self.feature_entry_1.key.delete()
for stage in core_models.Stage.query().fetch():
stage.key.delete()
def test_get_feature_stages(self):
"""A dictionary with stages relevant to the feature should be present."""
stage_dict = core_models.Stage.get_feature_stages(self.feature_id)
list_stages = stage_dict.items()
expected_stage_types = {410, 430, 450, 460, 470}
# Extension stage type was not created, so it should not appear.
self.assertIsNone(stage_dict.get(451, None))
self.assertEqual(len(list_stages), 5)
for stage_type, stage in stage_dict.items():
self.assertTrue(stage_type in expected_stage_types)
self.assertEqual(stage.stage_type, stage_type)
expected_stage_types.remove(stage_type)

Просмотреть файл

@ -142,6 +142,9 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
logging.info('POST is %r', self.form)
update_items = []
stage_update_items = []
feature_type = feature.feature_type
if self.touched('spec_link'):
feature.spec_link = self.parse_link('spec_link')
update_items.append(('spec_link', self.parse_link('spec_link')))
@ -191,10 +194,14 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
if self.touched('intent_to_implement_url'):
feature.intent_to_implement_url = self.parse_link(
'intent_to_implement_url')
stage_update_items.append(('intent_to_implement_url', self.parse_link(
'intent_to_implement_url')))
if self.touched('intent_to_ship_url'):
feature.intent_to_ship_url = self.parse_link(
'intent_to_ship_url')
stage_update_items.append(('intent_to_ship_url',
self.parse_link('intent_to_ship_url')))
if self.touched('ready_for_trial_url'):
feature.ready_for_trial_url = self.parse_link(
@ -203,14 +210,20 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
if self.touched('intent_to_experiment_url'):
feature.intent_to_experiment_url = self.parse_link(
'intent_to_experiment_url')
stage_update_items.append(('intent_to_experiment_url',
self.parse_link('intent_to_experiment_url')))
if self.touched('intent_to_extend_experiment_url'):
feature.intent_to_extend_experiment_url = self.parse_link(
'intent_to_extend_experiment_url')
stage_update_items.append(('intent_to_extend_experiment_url',
self.parse_link('intent_to_extend_experiment_url')))
if self.touched('origin_trial_feedback_url'):
feature.origin_trial_feedback_url = self.parse_link(
'origin_trial_feedback_url')
stage_update_items.append(('origin_trial_feedback_url',
self.parse_link('origin_trial_feedback_url')))
if self.touched('anticipated_spec_changes'):
feature.anticipated_spec_changes = self.form.get(
@ -220,6 +233,7 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
if self.touched('finch_url'):
feature.finch_url = self.parse_link('finch_url')
stage_update_items.append(('finch_url', self.parse_link('finch_url')))
if self.touched('i2e_lgtms'):
feature.i2e_lgtms = self.split_emails('i2e_lgtms')
@ -231,38 +245,58 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
# TODO(jrobbins): Consider supporting milestones that are not ints.
if self.touched('shipped_milestone'):
feature.shipped_milestone = self.parse_int('shipped_milestone')
stage_update_items.append(('shipped_milestone',
self.parse_int('shipped_milestone')))
if self.touched('shipped_android_milestone'):
feature.shipped_android_milestone = self.parse_int(
'shipped_android_milestone')
stage_update_items.append(('shipped_android_milestone',
self.parse_int('shipped_android_milestone')))
if self.touched('shipped_ios_milestone'):
feature.shipped_ios_milestone = self.parse_int('shipped_ios_milestone')
stage_update_items.append(('shipped_ios_milestone',
self.parse_int('shipped_ios_milestone')))
if self.touched('shipped_webview_milestone'):
feature.shipped_webview_milestone = self.parse_int(
'shipped_webview_milestone')
stage_update_items.append(('shipped_webview_milestone',
self.parse_int('shipped_webview_milestone')))
if self.touched('ot_milestone_desktop_start'):
feature.ot_milestone_desktop_start = self.parse_int(
'ot_milestone_desktop_start')
stage_update_items.append(('ot_milestone_desktop_start',
self.parse_int('ot_milestone_desktop_start')))
if self.touched('ot_milestone_desktop_end'):
feature.ot_milestone_desktop_end = self.parse_int(
'ot_milestone_desktop_end')
stage_update_items.append(('ot_milestone_desktop_end',
self.parse_int('ot_milestone_desktop_end')))
if self.touched('ot_milestone_android_start'):
feature.ot_milestone_android_start = self.parse_int(
'ot_milestone_android_start')
stage_update_items.append(('ot_milestone_android_start',
self.parse_int('ot_milestone_android_start')))
if self.touched('ot_milestone_android_end'):
feature.ot_milestone_android_end = self.parse_int(
'ot_milestone_android_end')
stage_update_items.append(('ot_milestone_android_end',
self.parse_int('ot_milestone_android_end')))
if self.touched('ot_milestone_webview_start'):
feature.ot_milestone_webview_start = self.parse_int(
'ot_milestone_webview_start')
stage_update_items.append(('ot_milestone_webview_start',
self.parse_int('ot_milestone_webview_start')))
if self.touched('ot_milestone_webview_end'):
feature.ot_milestone_webview_end = self.parse_int(
'ot_milestone_webview_end')
stage_update_items.append(('ot_milestone_webview_end',
self.parse_int('ot_milestone_webview_end')))
if self.touched('requires_embedder_support'):
feature.requires_embedder_support = (
@ -278,18 +312,26 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
if self.touched('dt_milestone_desktop_start'):
feature.dt_milestone_desktop_start = self.parse_int(
'dt_milestone_desktop_start')
stage_update_items.append(('dt_milestone_desktop_start',
self.parse_int('dt_milestone_desktop_start')))
if self.touched('dt_milestone_android_start'):
feature.dt_milestone_android_start = self.parse_int(
'dt_milestone_android_start')
stage_update_items.append(('dt_milestone_android_start',
self.parse_int('dt_milestone_android_start')))
if self.touched('dt_milestone_ios_start'):
feature.dt_milestone_ios_start = self.parse_int(
'dt_milestone_ios_start')
stage_update_items.append(('dt_milestone_ios_start',
self.parse_int('dt_milestone_ios_start')))
if self.touched('dt_milestone_webview_start'):
feature.dt_milestone_webview_start = self.parse_int(
'dt_milestone_webview_start')
stage_update_items.append(('dt_milestone_webview_start',
self.parse_int('dt_milestone_webview_start')))
if self.touched('flag_name'):
feature.flag_name = self.form.get('flag_name')
@ -339,6 +381,7 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
if self.touched('feature_type'):
feature.feature_type = int(self.form.get('feature_type'))
update_items.append(('feature_type', int(self.form.get('feature_type'))))
feature_type = int(self.form.get('feature_type'))
# intent_stage can be be set either by <select> or a checkbox
if self.touched('intent_stage'):
@ -479,13 +522,19 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
update_items.append(('feature_notes', self.form.get('comments')))
if self.touched('experiment_goals'):
feature.experiment_goals = self.form.get('experiment_goals')
stage_update_items.append(('experiment_goals',
self.form.get('experiment_goals')))
if self.touched('experiment_timeline'):
feature.experiment_timeline = self.form.get('experiment_timeline')
if self.touched('experiment_risks'):
feature.experiment_risks = self.form.get('experiment_risks')
stage_update_items.append(('experiment_risks',
self.form.get('experiment_risks')))
if self.touched('experiment_extension_reason'):
feature.experiment_extension_reason = self.form.get(
'experiment_extension_reason')
stage_update_items.append(('experiment_extension_reason',
self.form.get('experiment_extension_reason')))
if self.touched('ongoing_constraints'):
feature.ongoing_constraints = self.form.get('ongoing_constraints')
update_items.append(('ongoing_constraints',
@ -503,9 +552,55 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
for field, value in update_items:
setattr(feature_entry, field, value)
feature_entry.put()
# Write changes made to the corresponding stage type.
if stage_update_items:
self.update_stage_fields(feature_id, feature_type, stage_update_items)
# Remove all feature-related cache.
rediscache.delete_keys_with_prefix(core_models.feature_cache_prefix())
redirect_url = '/guide/edit/' + str(key.integer_id())
return self.redirect(redirect_url)
def update_stage_fields(self, feature_id, feature_type, update_items) -> None:
# Get all existing stages associated with the feature.
stages = core_models.Stage.get_feature_stages(feature_id)
for field, value in update_items:
# Determine the stage type that the field should change on.
stage_type = core_enums.STAGE_TYPES_BY_FIELD_MAPPING[field][feature_type]
# If this feature type does not have this field, skip it
# (e.g. developer-facing code changes cannot have origin trial fields).
if stage_type is None:
continue
stage = stages.get(stage_type, None)
# If a stage of this type does not exist for this feature, create it.
if stage is None:
stage = core_models.Stage(feature_id=feature_id, stage_type=stage_type)
stage.put()
stages[stage_type] = stage
# Change the field based on the field type.
# If this field changing is a milestone, change it in the
# MilestoneSet entity.
if field in core_models.MilestoneSet.MILESTONE_FIELD_MAPPING:
milestone_field = (
core_models.MilestoneSet.MILESTONE_FIELD_MAPPING[field])
milestoneset_entity = getattr(stage, 'milestones')
# If the MilestoneSet entity has not been initiated, create it.
if milestoneset_entity is None:
milestoneset_entity = core_models.MilestoneSet()
setattr(milestoneset_entity, milestone_field, value)
stage.milestones = milestoneset_entity
# If the field starts with "intent_", it should modify the
# more general "intent_thread_url" field.
elif field.startswith('intent_'):
setattr(stage, 'intent_thread_url', value)
# Otherwise, replace field value with attribute of the same field name.
else:
setattr(stage, field, value)
# Write to all the stages.
for stage in stages.values():
stage.put()

Просмотреть файл

@ -107,12 +107,27 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase):
standard_maturity=1, web_dev_views=1, impl_status_chrome=1)
self.feature_entry_1.put()
feature_id = self.feature_1.key.integer_id()
# Shipping stage is not created, and should be created on edit.
stage_types = [core_models.STAGE_BLINK_INCUBATE,
core_models.STAGE_BLINK_PROTOTYPE,
core_models.STAGE_BLINK_DEV_TRIAL,
core_models.STAGE_BLINK_EVAL_READINESS,
core_models.STAGE_BLINK_ORIGIN_TRIAL,
core_models.STAGE_BLINK_EXTEND_ORIGIN_TRIAL]
for stage_type in stage_types:
stage = core_models.Stage(feature_id=feature_id, stage_type=stage_type)
stage.put()
self.request_path = ('/guide/stage/%d/%d' % (
self.feature_1.key.integer_id(), self.stage))
self.handler = guide.FeatureEditHandler()
def tearDown(self):
self.feature_1.key.delete()
self.feature_entry_1.key.delete()
for stage in core_models.Stage.query().fetch():
stage.key.delete()
def test_touched(self):
"""We can tell if the user meant to edit a field."""
@ -166,13 +181,31 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase):
def test_post__normal_valid(self):
"""Allowed user can edit a feature."""
testing_config.sign_in('user1@google.com', 1234567890)
# Fields changed.
form_fields = ('category, name, summary, shipped_milestone, '
'intent_to_experiment_url, experiment_risks, experiment_reason, '
'origin_trial_feedback_url, intent_to_ship_url')
# Expected stage change items.
new_shipped_milestone = '84'
new_intent_to_experiment_url = 'https://example.com/intent'
new_experiment_risks = 'Some pretty risky business'
new_experiment_extension_reason = 'It would be fun'
new_origin_trial_feedback_url = 'https://example.com/ot_intent'
new_intent_to_ship_url = 'https://example.com/shipping'
with test_app.test_request_context(
self.request_path, data={
'form_fields': 'category, name, summary, shipped_milestone',
'form_fields': form_fields,
'category': '2',
'name': 'Revised feature name',
'summary': 'Revised feature summary',
'shipped_milestone': '84',
'shipped_milestone': new_shipped_milestone,
'intent_to_experiment_url': new_intent_to_experiment_url,
'experiment_risks': new_experiment_risks,
'experiment_extension_reason': new_experiment_extension_reason,
'origin_trial_feedback_url': new_origin_trial_feedback_url,
'intent_to_ship_url': new_intent_to_ship_url
}):
actual_response = self.handler.process_post_data(
self.feature_1.key.integer_id(), self.stage)
@ -194,3 +227,28 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase):
self.assertEqual(2, revised_entry.category)
self.assertEqual('Revised feature name', revised_entry.name)
self.assertEqual('Revised feature summary', revised_entry.summary)
# Ensure changes were also made to Stage entities
stages = core_models.Stage.get_feature_stages(
self.feature_1.key.integer_id())
self.assertEqual(len(stages.keys()), 7)
origin_trial_stage = stages.get(150)
ot_extension_stage = stages.get(151)
# Stage for shipping should have been created.
shipping_stage = stages.get(160)
self.assertIsNotNone(origin_trial_stage)
self.assertIsNotNone(shipping_stage)
self.assertIsNotNone(ot_extension_stage)
# Check that correct stage fields were changed.
self.assertEqual(origin_trial_stage.experiment_risks,
new_experiment_risks)
self.assertEqual(origin_trial_stage.intent_thread_url,
new_intent_to_experiment_url)
self.assertEqual(origin_trial_stage.origin_trial_feedback_url,
new_origin_trial_feedback_url)
self.assertEqual(ot_extension_stage.experiment_extension_reason,
new_experiment_extension_reason)
self.assertEqual(shipping_stage.milestones.desktop_first,
int(new_shipped_milestone))
self.assertEqual(shipping_stage.intent_thread_url,
new_intent_to_ship_url)