From fb78d47bb1c8fc35d7d57704f4175a4f5ebae03a Mon Sep 17 00:00:00 2001 From: Daniel LaLiberte Date: Wed, 6 Dec 2023 17:29:32 +0000 Subject: [PATCH] Show dialog when preflight not needed. --- client-src/elements/chromedash-app.js | 1 - .../elements/chromedash-intent-dialog.js | 12 +- .../elements/chromedash-intent-dialog_test.js | 355 ++++++++++++++++++ .../elements/chromedash-process-overview.js | 21 +- 4 files changed, 377 insertions(+), 12 deletions(-) create mode 100644 client-src/elements/chromedash-intent-dialog_test.js diff --git a/client-src/elements/chromedash-app.js b/client-src/elements/chromedash-app.js index 1db248593404..c9f7c506a365 100644 --- a/client-src/elements/chromedash-app.js +++ b/client-src/elements/chromedash-app.js @@ -312,7 +312,6 @@ class ChromedashApp extends LitElement { this.currentPage = ctx.path; this.hideSidebar(); }); - // page('/admin/features/launch/:featureId(\\d+)/:intentStage(\\d+)?intent=1') page('/guide/new', (ctx) => { if (!this.setupNewPage(ctx, 'chromedash-guide-new-page')) return; if (ctx.querystring.search('loginStatus=False') == -1) { diff --git a/client-src/elements/chromedash-intent-dialog.js b/client-src/elements/chromedash-intent-dialog.js index 009bf456532a..b90075a7c677 100644 --- a/client-src/elements/chromedash-intent-dialog.js +++ b/client-src/elements/chromedash-intent-dialog.js @@ -1249,13 +1249,17 @@ after that, you're free to ship your feature. return html`

Finch feature name

- ${finchName} + ${finchName ?? 'None'} +

+ ${this.feature.non_finch_justification ? - html`

+ html`

Non-finch justification

-

${this.feature.non_finch_justification}

` : +

${this.feature.non_finch_justification}

+

+ ` : !finchName ? - html`

+ html`

Non-finch justification

None` : nothing} diff --git a/client-src/elements/chromedash-intent-dialog_test.js b/client-src/elements/chromedash-intent-dialog_test.js new file mode 100644 index 000000000000..49b117b97c87 --- /dev/null +++ b/client-src/elements/chromedash-intent-dialog_test.js @@ -0,0 +1,355 @@ +// // # Copyright 2020 Google Inc. +// // # +// // # Licensed under the Apache License, Version 2.0 (the "License") +// // # you may not use this file except in compliance with the License. +// // # You may obtain a copy of the License at +// // # +// // # http://www.apache.org/licenses/LICENSE-2.0 +// // # +// // # Unless required by applicable law or agreed to in writing, software +// // # distributed under the License is distributed on an "AS IS" BASIS, +// // # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// // # See the License for the specific language governing permissions and +// // # limitations under the License. + +// // import testing_config # Must be imported before the module under test. + +// // from unittest import mock + +// // import os +// // import flask +// // import werkzeug +// // import html5lib +// // import settings + +// import {html} from 'lit'; +// import {assert, fixture} from '@open-wc/testing'; +// import {ChromeDashIntentDialog, openIntentDialog} from './chromedash-intent-dialog'; +// import '../js-src/cs-client'; +// import sinon from 'sinon'; + +// // from google.cloud import ndb # type: ignore +// // from pages import intentpreview +// // from internals import core_enums +// // from internals.core_models import FeatureEntry, MilestoneSet, Stage + +// // test_app = flask.Flask(__name__, +// // template_folder=settings.get_flask_template_path()) + +// // # Load testdata to be used across all of the CustomTestCases +// // TESTDATA = testing_config.Testdata(__file__) + +// // class IntentEmailPreviewHandlerTest(testing_config.CustomTestCase): + +// describe('chromedash-intent-dialog', () => { + +// // def setUp(self): +// // self.feature_1 = FeatureEntry( +// // name='feature one', summary='sum', owner_emails=['user1@google.com'], +// // category=1, intent_stage=core_enums.INTENT_IMPLEMENT, +// // feature_type=0) +// // self.feature_1.put() +// // # Write stages for the feature. +// // stage_types = [110, 120, 130, 140, 150, 151, 160, 1061] +// // for s_type in stage_types: +// // s = Stage(feature_id=self.feature_1.key.integer_id(), stage_type=s_type, +// // milestones=MilestoneSet(desktop_first=1, +// // android_first=1, desktop_last=2), +// // intent_thread_url=f'https://example.com/{s_type}') +// // # Add stage-specific fields based on the stage ID. +// // # 150 is the ID associated with the origin trial stage for feature type 0. +// // if s_type == 150: +// // s.experiment_goals = 'goals' +// // s.experiment_risks = 'risks' +// // s.announcement_url = 'https://example.com/announce' +// // # 151 is the stage ID associated with the origin trial extension. +// // elif s_type == 151: +// // s.experiment_extension_reason = 'reason' +// // # 151 is the ID associated with the shipping stage. +// // elif s_type == 160: +// // s.finch_url = 'https://example.com/finch' +// // s.put() + +// // self.request_path = '/admin/features/launch/%d/%d?intent' % ( +// // core_enums.INTENT_SHIP, self.feature_1.key.integer_id()) +// // self.handler = intentpreview.IntentEmailPreviewHandler() + +// beforeEach(async () => { + +// }); + + +// // def tearDown(self): +// // self.feature_1.key.delete() + +// afterEach(async () => { + +// }); + + +// // def test_get__anon(self): +// // """Anon cannot view this preview features, gets redirected to login.""" +// // testing_config.sign_out() +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // actual_response = self.handler.get_template_data(feature_id=feature_id) +// // self.assertEqual('302 FOUND', actual_response.status) + +// it('should redirect anonymous user to login', async () => { +// // testing_config.sign_out() +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // actual_response = self.handler.get_template_data(feature_id=feature_id) +// // self.assertEqual('302 FOUND', actual_response.status) +// }); + +// // def test_get__no_existing(self): +// // """Trying to view a feature that does not exist gives a 404.""" +// // testing_config.sign_in('user1@google.com', 123567890) +// // bad_feature_id = self.feature_1.key.integer_id() + 1 +// // with test_app.test_request_context(self.request_path): +// // with self.assertRaises(werkzeug.exceptions.NotFound): +// // self.handler.get_template_data(feature_id=bad_feature_id) +// it('should throw 404 if feature does not exist', async () => { +// // testing_config.sign_in('user1@google.com', 123567890) +// // bad_feature_id = self.feature_1.key.integer_id() + 1 +// // with test_app.test_request_context(self.request_path): +// // with self.assertRaises(werkzeug.exceptions.NotFound): +// // self.handler.get_template_data(feature_id=bad_feature_id) +// }); + + +// // def test_get__no_stage_specified(self): +// // """Allowed user can preview intent email for a feature using an old URL.""" +// // request_path = ( +// // '/admin/features/launch/%d?intent' % self.feature_1.key.integer_id()) +// // testing_config.sign_in('user1@google.com', 123567890) +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // actual_data = self.handler.get_template_data(feature_id=feature_id) +// // self.assertIn('feature', actual_data) +// // self.assertEqual('feature one', actual_data['feature']['name']) + +// it('should preview intent email for a feature using an old URL', async () => { +// // request_path = ( +// // '/admin/features/launch/%d?intent' % self.feature_1.key.integer_id()) +// // testing_config.sign_in('user1@google.com', 123567890) +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // actual_data = self.handler.get_template_data(feature_id=feature_id) +// // self.assertIn('feature', actual_data) +// // self.assertEqual('feature one', actual_data['feature']['name']) +// }); + +// // def test_get__normal(self): +// // """Allowed user can preview intent email for a feature.""" +// // testing_config.sign_in('user1@google.com', 123567890) +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // actual_data = self.handler.get_template_data(feature_id=feature_id) +// // self.assertIn('feature', actual_data) +// // self.assertEqual('feature one', actual_data['feature']['name']) + +// it('should preview intent email for a feature', async () => { +// // testing_config.sign_in('user1@google.com', 123567890) +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // actual_data = self.handler.get_template_data(feature_id=feature_id) +// // self.assertIn('feature', actual_data) +// // self.assertEqual('feature one', actual_data['feature']['name']) +// }); + +// // def test_get_page_data__implement(self): +// // """page_data has correct values.""" +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // page_data = self.handler.get_page_data( +// // feature_id, self.feature_1, core_enums.INTENT_IMPLEMENT) +// // self.assertEqual( +// // 'http://localhost/feature/%d' % feature_id, +// // page_data['default_url']) +// // self.assertEqual( +// // ['motivation'], +// // page_data['sections_to_show']) +// // self.assertEqual( +// // 'Intent to Prototype', +// // page_data['subject_prefix']) + +// it('should get page data', async () => { +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // page_data = self.handler.get_page_data( +// // feature_id, self.feature_1, core_enums.INTENT_IMPLEMENT) +// // self.assertEqual( +// // 'http://localhost/feature/%d' % feature_id, +// // page_data['default_url']) +// // self.assertEqual( +// // ['motivation'], +// // page_data['sections_to_show']) +// // self.assertEqual( +// // 'Intent to Prototype', +// // page_data['subject_prefix']) +// }); + +// // def test_get_page_data__ship(self): +// // """page_data has correct values.""" +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // page_data = self.handler.get_page_data( +// // feature_id, self.feature_1, core_enums.INTENT_SHIP) +// // self.assertEqual( +// // 'http://localhost/feature/%d' % feature_id, +// // page_data['default_url']) +// // self.assertIn('ship', page_data['sections_to_show']) +// // self.assertEqual( +// // 'Intent to Ship', +// // page_data['subject_prefix']) + +// it('should get page data', async () => { +// // feature_id = self.feature_1.key.integer_id() +// // with test_app.test_request_context(self.request_path): +// // page_data = self.handler.get_page_data( +// // feature_id, self.feature_1, core_enums.INTENT_SHIP) +// // self.assertEqual( +// // 'http://localhost/feature/%d' % feature_id, +// // page_data['default_url']) +// // self.assertIn('ship', page_data['sections_to_show']) +// // self.assertEqual( +// // 'Intent to Ship', +// // page_data['subject_prefix']) +// }); + +// // def test_compute_subject_prefix__incubate_new_feature(self): +// // """We offer users the correct subject line for each intent stage.""" +// // self.assertEqual( +// // 'Intent stage "None"', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_NONE)) + +// // self.assertEqual( +// // 'Intent stage "Start incubating"', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_INCUBATE)) + +// // self.assertEqual( +// // 'Intent to Prototype', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_IMPLEMENT)) + +// // self.assertEqual( +// // 'Ready for Developer Testing', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_EXPERIMENT)) + +// // self.assertEqual( +// // 'Intent stage "Evaluate readiness to ship"', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_IMPLEMENT_SHIP)) + +// // self.assertEqual( +// // 'Intent to Experiment', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_EXTEND_TRIAL)) + +// // self.assertEqual( +// // 'Intent to Ship', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_SHIP)) + +// // self.assertEqual( +// // 'Intent to Extend Deprecation Trial', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_REMOVED)) + +// // self.assertEqual( +// // 'Intent stage "Shipped"', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_SHIPPED)) + +// // self.assertEqual( +// // 'Intent stage "Parked"', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_PARKED)) + + +// // def test_compute_subject_prefix__deprecate_feature(self): +// // """We offer users the correct subject line for each intent stage.""" +// // self.feature_1.feature_type = core_enums.FEATURE_TYPE_DEPRECATION_ID +// // self.assertEqual( +// // 'Intent stage "None"', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_NONE)) + +// // self.assertEqual( +// // 'Intent to Deprecate and Remove', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_INCUBATE)) + +// // self.assertEqual( +// // 'Request for Deprecation Trial', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_EXTEND_TRIAL)) + +// // def test_compute_subject_prefix__PSA_feature(self): +// // """We offer users the correct subject line for each intent stage.""" +// // self.feature_1.feature_type = core_enums.FEATURE_TYPE_CODE_CHANGE_ID +// // self.assertEqual( +// // 'Web-Facing Change PSA', +// // self.handler.compute_subject_prefix( +// // self.feature_1, core_enums.INTENT_SHIP)) + +// // class IntentEmailPreviewTemplateTest(testing_config.CustomTestCase): + +// // HANDLER_CLASS = intentpreview.IntentEmailPreviewHandler + +// // def setUp(self): +// // super(IntentEmailPreviewTemplateTest, self).setUp() +// // self.feature_1 = FeatureEntry( +// // name='feature one', summary='sum', owner_emails=['user1@google.com'], +// // category=1, intent_stage=core_enums.INTENT_IMPLEMENT) +// // # Hardcode the key for the template test +// // self.feature_1.key = ndb.Key('FeatureEntry', 234) +// // self.feature_1.wpt = True +// // self.feature_1.wpt_descr = 'We love WPT!' +// // self.feature_1.put() + +// // self.request_path = '/admin/features/launch/%d/%d?intent' % ( +// // core_enums.INTENT_SHIP, self.feature_1.key.integer_id()) +// // self.handler = self.HANDLER_CLASS() +// // self.feature_id = self.feature_1.key.integer_id() + +// // testing_config.sign_in('user1@google.com', 123567890) +// // with test_app.test_request_context(self.request_path): +// // self.template_data = self.handler.get_template_data( +// // feature_id=self.feature_id) +// // page_data = self.handler.get_page_data( +// // self.feature_id, self.feature_1, core_enums.INTENT_IMPLEMENT) + +// // self.template_data.update(page_data) +// // self.template_data['nonce'] = 'fake nonce' +// // template_path = self.handler.get_template_path(self.template_data) +// // self.full_template_path = os.path.join(template_path) + +// // self.maxDiff = None + +// // def tearDown(self): +// // self.feature_1.key.delete() +// // testing_config.sign_out() + +// // def test_html_rendering(self): +// // """We can render the template with valid html.""" +// // with test_app.test_request_context(self.request_path): +// // actual_data = self.handler.get_template_data(feature_id=self.feature_id) +// // actual_data.update(self.handler.get_common_data()) +// // actual_data['nonce'] = 'fake nonce' +// // actual_data['xsrf_token'] = '' +// // actual_data['xsrf_token_expires'] = 0 + +// // template_text = self.handler.render( +// // actual_data, self.full_template_path) +// // testing_config.sign_out() +// // parser = html5lib.HTMLParser(strict=True) +// // document = parser.parse(template_text) +// // # TESTDATA.make_golden(template_text, 'test_html_rendering.html') +// // self.assertMultiLineEqual( +// // TESTDATA['test_html_rendering.html'], template_text) diff --git a/client-src/elements/chromedash-process-overview.js b/client-src/elements/chromedash-process-overview.js index 7aa4dc33dbeb..97facfed9602 100644 --- a/client-src/elements/chromedash-process-overview.js +++ b/client-src/elements/chromedash-process-overview.js @@ -5,7 +5,7 @@ import { somePendingPrereqs, somePendingGates, } from './chromedash-preflight-dialog'; -// import {openIntentDialog} from './chromedash-intent-dialog'; +import {openIntentDialog} from './chromedash-intent-dialog'; import {findProcessStage} from './utils'; import {SHARED_STYLES} from '../css/shared-css.js'; @@ -161,11 +161,17 @@ export class ChromedashProcessOverview extends LitElement { return (viewedIncomingStageIndex > featureStageIndex); } + openIntentDialog(action, stage, feStage) { + openIntentDialog( + this.feature, this.progress, this.process, action, + stage, feStage, this.featureGates); + } + renderAction(action, stage, feStage) { const label = action.name; - const url = action.url - .replace('{feature_id}', this.feature.id) - .replace('{outgoing_stage}', stage.outgoing_stage); + // const url = action.url + // .replace('{feature_id}', this.feature.id) + // .replace('{outgoing_stage}', stage.outgoing_stage); const checkCompletion = () => { if (somePendingPrereqs(action, this.progress) || @@ -176,9 +182,10 @@ export class ChromedashProcessOverview extends LitElement { this.featureGates); return; } else { - // Act like user clicked left button to go to the draft email window. - const draftWindow = window.open(url, '_blank'); - draftWindow.focus(); + // // Act like user clicked left button to go to the draft email window. + // const draftWindow = window.open(url, '_blank'); + // draftWindow.focus(); + this.openIntentDialog(action, stage, feStage); } }; return html`