diff --git a/api/stages_api.py b/api/stages_api.py index 09dfaa55..9c838061 100644 --- a/api/stages_api.py +++ b/api/stages_api.py @@ -109,22 +109,17 @@ class StagesAPI(basehandlers.APIHandler): setattr(stage.milestones, field, body[field]) # Keep the gate type that might need to be created for the stage type. - gate_type: int | None = None + gate_type: int | None = stage_helpers.get_gate_for_stage(feature_type, s_type) # Update type-specific fields. - if s_type == core_enums.STAGE_TYPES_DEV_TRIAL[feature_type]: # pragma: no cover - gate_type = core_enums.GATE_API_PROTOTYPE if s_type == core_enums.STAGE_TYPES_ORIGIN_TRIAL[feature_type]: self._add_given_stage_vals(stage, body, self.OT_FIELDS) - gate_type = core_enums.GATE_API_ORIGIN_TRIAL if s_type == core_enums.STAGE_TYPES_EXTEND_ORIGIN_TRIAL[feature_type]: self._add_given_stage_vals(stage, body, self.OT_EXTENSION_FIELDS) - gate_type = core_enums.GATE_API_EXTEND_ORIGIN_TRIAL if s_type == core_enums.STAGE_TYPES_SHIPPING[feature_type]: # pragma: no cover self._add_given_stage_vals(stage, body, self.SHIPPING_FIELDS) - gate_type = core_enums.GATE_API_SHIP if s_type == core_enums.STAGE_TYPES_ROLLOUT[feature_type]: # pragma: no cover self._add_given_stage_vals(stage, body, self.ENTERPRISE_FIELDS) diff --git a/client-src/elements/chromedash-add-stage-dialog.js b/client-src/elements/chromedash-add-stage-dialog.js index 3dafbf32..e9290e31 100644 --- a/client-src/elements/chromedash-add-stage-dialog.js +++ b/client-src/elements/chromedash-add-stage-dialog.js @@ -7,11 +7,14 @@ let addStageDialogEl; let currentFeatureId; -export async function openAddStageDialog(featureId, featureType) { - if (!addStageDialogEl || currentFeatureId !== featureId) { +export async function openAddStageDialog(featureId, featureType, onSubmitCustomHandler) { + if (!addStageDialogEl || + currentFeatureId !== featureId || + onSubmitCustomHandler !== addStageDialogEl.onSubmitCustomHandler) { addStageDialogEl = document.createElement('chromedash-add-stage-dialog'); addStageDialogEl.featureId = featureId; addStageDialogEl.featureType = featureType; + addStageDialogEl.onSubmitCustomHandler = onSubmitCustomHandler; document.body.appendChild(addStageDialogEl); await addStageDialogEl.updateComplete; } @@ -26,6 +29,7 @@ class ChromedashAddStageDialog extends LitElement { featureId: {type: Number}, featureType: {type: Number}, canSubmit: {type: Boolean}, + onSubmitCustomHandler: {type: Function}, }; } @@ -33,6 +37,7 @@ class ChromedashAddStageDialog extends LitElement { super(); this.featureId = 0; this.featureType = 0; + this.onSubmitCustomHandler = null; this.canSubmit = false; } @@ -78,6 +83,12 @@ class ChromedashAddStageDialog extends LitElement { } handleStageCreate() { + if (this.onSubmitCustomHandler) { + this.onSubmitCustomHandler({stage_type: Number(this.getStageSelectValue())}); + this.onSubmitCustomHandler = null; + this.shadowRoot.querySelector('sl-dialog').hide(); + return; + } window.csClient.createStage(this.featureId, {stage_type: this.getStageSelectValue()}) .then(() => { this.shadowRoot.querySelector('sl-dialog').hide(); diff --git a/client-src/elements/chromedash-form-field.js b/client-src/elements/chromedash-form-field.js index 471d88b8..10ef9b4a 100644 --- a/client-src/elements/chromedash-form-field.js +++ b/client-src/elements/chromedash-form-field.js @@ -16,6 +16,7 @@ export class ChromedashFormField extends LitElement { loading: {type: Boolean}, fieldProps: {type: Object}, forEnterprise: {type: Boolean}, + stageType: {type: Number | undefined}, componentChoices: {type: Object}, // just for the blink component select field }; } @@ -29,6 +30,7 @@ export class ChromedashFormField extends LitElement { this.disabled = false; this.loading = false; this.forEnterprise = false; + this.stageType = undefined; this.componentChoices = {}; } @@ -102,7 +104,7 @@ export class ChromedashFormField extends LitElement { // form field name can be specified in form-field-spec to match DB field name let fieldName = this.fieldProps.name || this.name; if (STAGE_SPECIFIC_FIELDS.has(fieldName) && this.stageId) { - fieldName = `${fieldName}__${this.stageId}`; + fieldName = this.stageType ? `${fieldName}__${this.stageId}__${this.stageType}__create` : `${fieldName}__${this.stageId}`; } // choices can be specified in form-field-spec or fetched from API const choices = this.fieldProps.choices || this.componentChoices; diff --git a/client-src/elements/chromedash-form-field_test.js b/client-src/elements/chromedash-form-field_test.js index 3cf95d2d..ae6a2f4b 100644 --- a/client-src/elements/chromedash-form-field_test.js +++ b/client-src/elements/chromedash-form-field_test.js @@ -101,4 +101,32 @@ describe('chromedash-form-field', () => { assert.include(renderElement.innerHTML, 'multiple'); assert.include(renderElement.innerHTML, 'cleareable'); }); + + it('renders a stage\'s field with the right form field name', async () => { + const component = await fixture( + html` + + `); + assert.exists(component); + assert.instanceOf(component, ChromedashFormField); + const fieldRow = component.renderRoot.querySelector('tr'); + assert.exists(fieldRow); + + const renderElement = component.renderRoot; + assert.include(renderElement.innerHTML, 'name="rollout_platforms__1"'); + }); + + it('renders a stage\'s field to be created with the right form field name', async () => { + const component = await fixture( + html` + + `); + assert.exists(component); + assert.instanceOf(component, ChromedashFormField); + const fieldRow = component.renderRoot.querySelector('tr'); + assert.exists(fieldRow); + + const renderElement = component.renderRoot; + assert.include(renderElement.innerHTML, 'name="rollout_platforms__10__11__create"'); + }); }); diff --git a/client-src/elements/chromedash-guide-editall-page.js b/client-src/elements/chromedash-guide-editall-page.js index 0bbdf74b..ce62e5f5 100644 --- a/client-src/elements/chromedash-guide-editall-page.js +++ b/client-src/elements/chromedash-guide-editall-page.js @@ -43,6 +43,7 @@ export class ChromedashGuideEditallPage extends LitElement { loading: {type: Boolean}, appTitle: {type: String}, nextPage: {type: String}, + nextStageToCreateId: {type: Number}, }; } @@ -55,6 +56,7 @@ export class ChromedashGuideEditallPage extends LitElement { this.nextPage = ''; this.previousStageTypeRendered = 0; this.sameTypeRendered = 0; + this.nextStageToCreateId = 0; } connectedCallback() { @@ -218,6 +220,7 @@ export class ChromedashGuideEditallPage extends LitElement { @@ -298,6 +301,7 @@ export class ChromedashGuideEditallPage extends LitElement { getAllStageIds() { const stageIds = []; this.feature.stages.forEach(feStage => { + if (feStage.to_create) return; stageIds.push(feStage.id); // Check if any trial extension exist, and collect their IDs as well. const extensions = feStage.extensions || []; @@ -310,11 +314,21 @@ export class ChromedashGuideEditallPage extends LitElement { const text = this.feature.is_enterprise_feature ? 'Add Step': 'Add Stage'; return html` + () => openAddStageDialog(this.feature.id, this.feature.feature_type_int, this.AddNewStageToCreate.bind(this))}"> ${text} `; } + AddNewStageToCreate(stageType) { + this.feature.stages.push({ + ...stageType, + to_create: true, + id: ++this.nextStageToCreateId}); + this.feature.stages = this.feature.stages + .sort((a, b) => a.stage_type_int - b.stage_type_int); + this.feature = {...this.feature}; + } + renderForm() { const formattedFeature = formatFeatureForEdit(this.feature); const stageIds = this.getAllStageIds(); diff --git a/internals/stage_helpers.py b/internals/stage_helpers.py index f2f0afcf..fa82bddc 100644 --- a/internals/stage_helpers.py +++ b/internals/stage_helpers.py @@ -24,8 +24,13 @@ from internals.core_enums import ( STAGE_TYPES_DEV_TRIAL, STAGE_TYPES_ORIGIN_TRIAL, STAGE_TYPES_EXTEND_ORIGIN_TRIAL, - STAGE_TYPES_SHIPPING) + STAGE_TYPES_SHIPPING, + GATE_API_SHIP, + GATE_API_EXTEND_ORIGIN_TRIAL, + GATE_API_ORIGIN_TRIAL, + GATE_API_PROTOTYPE) from internals.core_models import FeatureEntry, MilestoneSet, Stage +from internals.review_models import Gate # Type return value of get_stage_info_for_templates() @@ -38,6 +43,36 @@ class StageTemplateInfo(TypedDict): should_render_mstone_table: bool should_render_intents: bool +def create_feature_stage(feature_id: int, feature_type: int, stage_type: int) -> Stage: + # Create the stage. + stage = Stage(feature_id=feature_id, stage_type=stage_type) + stage.put() + + # If we should create a gate and this is a stage that requires a gate, + # create it. + gate_type = get_gate_for_stage(feature_type, stage_type) + if gate_type is not None: + gate = Gate(feature_id=feature_id, stage_id=stage.key.id(), gate_type=gate_type, + state=Gate.PREPARING) + gate.put() + + return stage + +def get_gate_for_stage(feature_type, s_type) -> int | None: + # Update type-specific fields. + if s_type == STAGE_TYPES_DEV_TRIAL[feature_type]: # pragma: no cover + return GATE_API_PROTOTYPE + + if s_type == STAGE_TYPES_ORIGIN_TRIAL[feature_type]: + return GATE_API_ORIGIN_TRIAL + + if s_type == STAGE_TYPES_EXTEND_ORIGIN_TRIAL[feature_type]: + return GATE_API_EXTEND_ORIGIN_TRIAL + + if s_type == STAGE_TYPES_SHIPPING[feature_type]: # pragma: no cover + return GATE_API_SHIP + return None + def get_feature_stages(feature_id: int) -> dict[int, list[Stage]]: """Return a dictionary of stages associated with a given feature.""" diff --git a/internals/stage_helpers_test.py b/internals/stage_helpers_test.py index 84c24514..460728e6 100644 --- a/internals/stage_helpers_test.py +++ b/internals/stage_helpers_test.py @@ -23,13 +23,14 @@ class StageHelpersTest(testing_config.CustomTestCase): def setUp(self): self.feature_entry_1 = FeatureEntry(id=1, name='fe one', - summary='summary', category=1, impl_status_chrome=1, + summary='summary', category=1, impl_status_chrome=1, feature_type=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() + self.feature_type = self.feature_entry_1.feature_type for stage_type in stage_types: stage = Stage(feature_id=self.feature_id, stage_type=stage_type) stage.put() @@ -50,4 +51,33 @@ class StageHelpersTest(testing_config.CustomTestCase): for stage_type, stages_list in stage_dict.items(): self.assertTrue(stage_type in expected_stage_types) self.assertEqual(stages_list[0].stage_type, stage_type) - expected_stage_types.remove(stage_type) \ No newline at end of file + expected_stage_types.remove(stage_type) + + def test_create_feature_stage(self): + """A dictionary with stages relevant to the feature should be present.""" + stage_dict = stage_helpers.get_feature_stages(self.feature_id) + list_stages = stage_dict.items() + expected_stage_types = {410, 430, 450, 460, 470} + self.assertEqual(len(list_stages), 5) + for stage_type, stages_list in stage_dict.items(): + self.assertTrue(stage_type in expected_stage_types) + self.assertEqual(stages_list[0].stage_type, stage_type) + expected_stage_types.remove(stage_type) + + stage_helpers.create_feature_stage( + self.feature_id, + self.feature_type, + core_enums.STAGE_ENT_ROLLOUT) + stage_helpers.create_feature_stage( + self.feature_id, + self.feature_type, + core_enums.STAGE_ENT_SHIPPED) + stage_dict = stage_helpers.get_feature_stages(self.feature_id) + list_stages = stage_dict.items() + expected_stage_types = {410, 430, 450, 460, 470, 1061, 1070} + self.assertEqual(len(list_stages), 7) + for stage_type, stages_list in stage_dict.items(): + self.assertTrue(stage_type in expected_stage_types) + self.assertEqual(stages_list[0].stage_type, stage_type) + expected_stage_types.remove(stage_type) + diff --git a/pages/guide.py b/pages/guide.py index 85a442c2..8b8ffe25 100644 --- a/pages/guide.py +++ b/pages/guide.py @@ -424,7 +424,7 @@ class FeatureEditHandler(basehandlers.FlaskHandler): if stage_ids: stage_ids_list = [int(id) for id in stage_ids.split(',')] self.update_stages_editall( - fe.feature_type, stage_ids_list, changed_fields, form_fields) + feature_id, fe.feature_type, stage_ids_list, changed_fields, form_fields) # If a stage_id is supplied, we make changes to only that specific stage. elif stage_id: for field, field_type in self.STAGE_FIELDS: @@ -443,7 +443,7 @@ class FeatureEditHandler(basehandlers.FlaskHandler): extension_stage_ids = self.form.get('extension_stage_ids') if extension_stage_ids: stage_ids_list = [int(id) for id in extension_stage_ids.split(',')] - self.update_stages_editall(fe.feature_type, stage_ids_list, changed_fields, form_fields) + self.update_stages_editall(feature_id, fe.feature_type, stage_ids_list, changed_fields, form_fields) # Update metadata fields. now = datetime.now() if self.form.get('accurate_as_of'): @@ -571,12 +571,28 @@ class FeatureEditHandler(basehandlers.FlaskHandler): def update_stages_editall( self, + feature_id: int, feature_type: int, stage_ids: list[int], changed_fields: list[tuple[str, Any, Any]], form_fields: list[str]) -> None: """Handle the updates for stages on the edit-all page.""" + id_to_field_suffix = {} + ids_created = set() + for field in self.form.keys(): + if not field.endswith('__create'): + continue + [name, id, stage_type, suffix] = field.split('__') + if stage_type is None: + continue + if id not in ids_created: + ids_created.add(id) + stage = stage_helpers.create_feature_stage(feature_id, feature_type, int(stage_type)) + id_to_field_suffix[stage.key.id()] = f'{id}__{stage_type}__create' + stage_ids.append(stage.key.id()) + for id in stage_ids: + suffix = id_to_field_suffix.get(id, id) stage = Stage.get_by_id(id) if not stage: self.abort(404, msg=f'No stage {id} found') @@ -584,12 +600,12 @@ class FeatureEditHandler(basehandlers.FlaskHandler): for field, field_type in self.STAGE_FIELDS: # To differentiate stages that have the same fields, the stage ID # is appended to the field name with 2 underscores. - field_with_id = f'{field}__{id}' - if not self.touched(field_with_id, form_fields): + field_with_suffix = f'{field}__{suffix}' + if not self.touched(field_with_suffix, form_fields): continue new_field_name = self.RENAMED_FIELD_MAPPING.get(field, field) old_val = getattr(stage, new_field_name) - new_val = self._get_field_val(field_with_id, field_type) + new_val = self._get_field_val(field_with_suffix, field_type) setattr(stage, new_field_name, new_val) if old_val != new_val: changed_fields.append((field, old_val, new_val)) @@ -613,20 +629,20 @@ class FeatureEditHandler(basehandlers.FlaskHandler): milestone_fields = self.SHIPPING_MILESTONE_FIELDS # Determine if 'intent_thread_url' field needs to be changed. - intent_field_with_id = f'{intent_thread_field}__{id}' - if intent_thread_field and self.touched(intent_field_with_id, form_fields): + intent_field_with_suffix = f'{intent_thread_field}__{suffix}' + if intent_thread_field and self.touched(intent_field_with_suffix, form_fields): old_val = stage.intent_thread_url - new_val = self._get_field_val(intent_field_with_id, 'link') + new_val = self._get_field_val(intent_field_with_suffix, 'link') if old_val != new_val: changed_fields.append((intent_thread_field, old_val, new_val)) setattr(stage, 'intent_thread_url', new_val) for field, milestone_field in milestone_fields: - field_with_id = f'{field}__{id}' - if not self.touched(field_with_id, form_fields): + field_with_suffix = f'{field}__{suffix}' + if not self.touched(field_with_suffix, form_fields): continue old_val = None - new_val = self._get_field_val(field_with_id, 'int') + new_val = self._get_field_val(field_with_suffix, 'int') milestoneset_entity = stage.milestones milestoneset_entity = getattr(stage, 'milestones') if milestoneset_entity is None: diff --git a/pages/guide_test.py b/pages/guide_test.py index ec5a1ba1..599eb9d4 100644 --- a/pages/guide_test.py +++ b/pages/guide_test.py @@ -224,6 +224,14 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase): new_origin_trial_feedback_url = 'https://example.com/ot_intent' new_intent_to_ship_url = 'https://example.com/shipping' + # Expected stage created + new_rollout_milestone = 50 + new_rollout_details = 'Details' + new_rollout_impact = 3 + new_second_rollout_milestone = 55 + new_second_rollout_details = 'Details 1' + new_second_rollout_impact = 1 + with test_app.test_request_context( self.request_path, data={ 'stages': '30,50,60', @@ -237,7 +245,14 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase): 'experiment_risks__50': new_experiment_risks, 'origin_trial_feedback_url__50': new_origin_trial_feedback_url, 'intent_to_ship_url__60': new_intent_to_ship_url, + 'rollout_milestone__1__1061__create': new_rollout_milestone, + 'rollout_details__1__1061__create': new_rollout_details, + 'rollout_impact__1__1061__create': new_rollout_impact, + 'rollout_milestone__2__1061__create': new_second_rollout_milestone, + 'rollout_details__2__1061__create': new_second_rollout_details, + 'rollout_impact__2__1061__create': new_second_rollout_impact, 'feature_type': '1' + # TODO ad to creates }): actual_response = self.handler.process_post_data( feature_id=self.fe_1.key.integer_id()) @@ -257,7 +272,7 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase): # Ensure changes were made to Stage entities. stages = stage_helpers.get_feature_stages( self.fe_1.key.integer_id()) - self.assertEqual(len(stages.keys()), 6) + self.assertEqual(len(stages.keys()), 7) dev_trial_stage = stages.get(130) origin_trial_stages = stages.get(150) # Stage for shipping should have been created. @@ -277,6 +292,14 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase): int(new_shipped_milestone)) self.assertEqual(shipping_stages[0].intent_thread_url, new_intent_to_ship_url) + # Check that rollout stages are created + rollout_stages = stages.get(1061) + self.assertEqual(rollout_stages[0].rollout_milestone, new_rollout_milestone) + self.assertEqual(rollout_stages[0].rollout_details, new_rollout_details) + self.assertEqual(rollout_stages[0].rollout_impact, new_rollout_impact) + self.assertEqual(rollout_stages[1].rollout_milestone, new_second_rollout_milestone) + self.assertEqual(rollout_stages[1].rollout_details, new_second_rollout_details) + self.assertEqual(rollout_stages[1].rollout_impact, new_second_rollout_impact) def test_post__normal_valid_single_stage(self): """Allowed user can edit a feature."""