From f99d66bd5c8b80c68f30cdb09796eba0cb318423 Mon Sep 17 00:00:00 2001 From: Jason Robbins Date: Thu, 22 Jun 2023 09:48:50 -0700 Subject: [PATCH] Add fields for finch flag name and non-finch justification. (#3108) --- api/api_specs.py | 4 ++- api/converters.py | 2 ++ api/converters_test.py | 2 ++ .../elements/chromedash-feature-filter.js | 2 ++ client-src/elements/form-definition.js | 4 +++ client-src/elements/form-field-specs.js | 29 ++++++++++++++++++- internals/core_models.py | 2 ++ internals/data_types.py | 6 ++-- internals/search_fulltext.py | 2 ++ internals/search_queries.py | 3 ++ pages/guide.py | 2 ++ .../test_html_rendering.html | 14 ++++++++- templates/blink/intent_to_implement.html | 17 ++++++++++- 13 files changed, 83 insertions(+), 6 deletions(-) diff --git a/api/api_specs.py b/api/api_specs.py index a513fb86..65725582 100644 --- a/api/api_specs.py +++ b/api/api_specs.py @@ -14,7 +14,7 @@ # limitations under the License. # Data type for lists defining field data type information. -FIELD_INFO_DATA_TYPE = list[tuple[str, str]] +FIELD_INFO_DATA_TYPE = list[tuple[str, str]] # List with fields that can be edited on feature create/update # and their data types. @@ -47,6 +47,8 @@ FEATURE_FIELD_DATA_TYPES: FIELD_INFO_DATA_TYPE = [ ('ff_views_link', 'str'), ('ff_views_notes', 'str'), ('flag_name', 'str'), + ('finch_name', 'str'), + ('non_finch_justification', 'str'), ('impl_status_chrome', 'int'), ('initial_public_proposal_url', 'str'), ('intent_stage', 'int'), diff --git a/api/converters.py b/api/converters.py index 1124b302..883d3fe4 100644 --- a/api/converters.py +++ b/api/converters.py @@ -272,6 +272,8 @@ def feature_entry_to_json_verbose( 'screenshot_links': fe.screenshot_links or [], 'breaking_change': fe.breaking_change, 'flag_name': fe.flag_name, + 'finch_name': fe.finch_name, + 'non_finch_justification': fe.non_finch_justification, 'ongoing_constraints': fe.ongoing_constraints, 'motivation': fe.motivation, 'devtrial_instructions': fe.devtrial_instructions, diff --git a/api/converters_test.py b/api/converters_test.py index 1e99fd37..93866b51 100644 --- a/api/converters_test.py +++ b/api/converters_test.py @@ -351,6 +351,8 @@ class FeatureConvertersTest(testing_config.CustomTestCase): 'feature_notes': 'notes', 'ff_views': 5, 'flag_name': None, + 'finch_name': None, + 'non_finch_justification': None, 'initial_public_proposal_url': None, 'interop_compat_risks': None, 'measurement': None, diff --git a/client-src/elements/chromedash-feature-filter.js b/client-src/elements/chromedash-feature-filter.js index b428cffb..2497980c 100644 --- a/client-src/elements/chromedash-feature-filter.js +++ b/client-src/elements/chromedash-feature-filter.js @@ -95,6 +95,8 @@ const QUERIABLE_FIELDS = [ // 'requires_embedder_support': Feature.requires_embedder_support, {name: 'browsers.chrome.flag_name', display: 'Flag name', type: TEXT_TYPE}, + {name: 'browsers.chrome.finch_name', display: 'Finch name', type: TEXT_TYPE}, + // 'browsers.chrome.non_finch_justification': Feature.non_finch_justification, // 'all_platforms': Feature.all_platforms, // 'all_platforms_descr': Feature.all_platforms_descr, // 'wpt': Feature.wpt, diff --git a/client-src/elements/form-definition.js b/client-src/elements/form-definition.js index 6742d3f1..fb1cab10 100644 --- a/client-src/elements/form-definition.js +++ b/client-src/elements/form-definition.js @@ -262,6 +262,8 @@ const FLAT_DEV_TRIAL_FIELDS = { name: 'Implementation in Chromium', fields: [ 'flag_name', + 'finch_name', + 'non_finch_justification', 'dt_milestone_desktop_start', 'dt_milestone_android_start', 'dt_milestone_ios_start', @@ -493,6 +495,8 @@ const DEPRECATION_DEV_TRIAL_FIELDS = { name: 'Implementation in Chromium', fields: [ 'flag_name', + 'finch_name', + 'non_finch_justification', 'dt_milestone_desktop_start', 'dt_milestone_android_start', 'dt_milestone_ios_start', diff --git a/client-src/elements/form-field-specs.js b/client-src/elements/form-field-specs.js index 5cce6cf4..bec54b30 100644 --- a/client-src/elements/form-field-specs.js +++ b/client-src/elements/form-field-specs.js @@ -1261,7 +1261,34 @@ export const ALL_FIELDS = { required: false, label: 'Flag name', help_text: html` - Name of the flag on chrome://flags that enables this feature.`, + Name of the flag on chrome://flags that allows a web developer to + enable this feature in their own browser to try it out. + E.g., "storage-buckets". These are defined in about_flags.cc.`, + }, + + 'finch_name': { + type: 'input', + attrs: TEXT_FIELD_ATTRS, + required: false, + label: 'Finch feature name', + help_text: html` + String name of the base::Feature defined via the + BASE_FEATURE macro in your feature implementation + code. E.g., "StoragBuckets". These names are used + in runtime_enabled_features.json5 and finch GCL files`, + }, + + 'non_finch_justification': { + type: 'textarea', + required: false, + label: 'Non-finch justification', + help_text: html` + The Flag Guarding Guidelines require new features to have + a finch flag. If your feature does not have a finch flag, explain why.`, }, 'prefixed': { diff --git a/internals/core_models.py b/internals/core_models.py index f017608b..06b0c9b1 100644 --- a/internals/core_models.py +++ b/internals/core_models.py @@ -93,6 +93,8 @@ class FeatureEntry(ndb.Model): # Copy from Feature # Implementation in Chrome impl_status_chrome = ndb.IntegerProperty(required=True, default=NO_ACTIVE_DEV) flag_name = ndb.StringProperty() + finch_name = ndb.StringProperty() + non_finch_justification = ndb.TextProperty() ongoing_constraints = ndb.TextProperty() # Topic: Adoption (reviewed by API Owners. Auto-approved gate later?) diff --git a/internals/data_types.py b/internals/data_types.py index 8c0c6032..2e1b45c0 100644 --- a/internals/data_types.py +++ b/internals/data_types.py @@ -33,13 +33,13 @@ class StageDict(TypedDict): # Dev trial specific fields. announcement_url: str | None - + # Origin trial specific fields. experiment_goals: str | None experiment_risks: str | None extensions: list[StageDict] # type: ignore origin_trial_feedback_url: str | None - + # Trial extension specific fields. ot_stage_id: int | None experiment_extension_reason: str | None @@ -192,6 +192,8 @@ class VerboseFeatureDict(TypedDict): # Implementation in Chrome flag_name: str | None + finch_name: str | None + non_finch_justification: str | None ongoing_constraints: str | None # Topic: Adoption diff --git a/internals/search_fulltext.py b/internals/search_fulltext.py index 9c7842a7..fe3b37d2 100644 --- a/internals/search_fulltext.py +++ b/internals/search_fulltext.py @@ -64,6 +64,8 @@ def get_strings(fe : FeatureEntry) -> list[str]: # TODO: impl_status_Chrome strings.append(fe.flag_name) + strings.append(fe.finch_name) + strings.append(fe.non_finch_justification) strings.append(fe.ongoing_constraints) strings.append(fe.motivation) diff --git a/internals/search_queries.py b/internals/search_queries.py index ba6d5b9f..97955669 100644 --- a/internals/search_queries.py +++ b/internals/search_queries.py @@ -225,6 +225,9 @@ QUERIABLE_FIELDS: dict[str, Property] = { 'browsers.chrome.status': FeatureEntry.impl_status_chrome, 'browsers.chrome.flag_name': FeatureEntry.flag_name, + 'browsers.chrome.finch_name': FeatureEntry.finch_name, + 'browsers.chrome.non_finch_justification': + FeatureEntry.non_finch_justification, 'ongoing_constraints': FeatureEntry.ongoing_constraints, 'motivation': FeatureEntry.motivation, diff --git a/pages/guide.py b/pages/guide.py index 76efd747..6a406d6f 100644 --- a/pages/guide.py +++ b/pages/guide.py @@ -178,6 +178,8 @@ class FeatureEditHandler(basehandlers.FlaskHandler): ('requires_embedder_support', 'bool'), ('devtrial_instructions', 'link'), ('flag_name', 'str'), + ('finch_name', 'str'), + ('non_finch_justification', 'str'), ('owner', 'emails'), ('editors', 'emails'), ('cc_recipients', 'emails'), diff --git a/pages/testdata/intentpreview_test/test_html_rendering.html b/pages/testdata/intentpreview_test/test_html_rendering.html index 42919fff..cae606ad 100644 --- a/pages/testdata/intentpreview_test/test_html_rendering.html +++ b/pages/testdata/intentpreview_test/test_html_rendering.html @@ -246,9 +246,21 @@ No -

Flag name

+

Flag name on chrome://flags

None +

Finch feature name

+None + + + +

Non-finch justification

+ None + + + + +

Requires code in //chrome?

False diff --git a/templates/blink/intent_to_implement.html b/templates/blink/intent_to_implement.html index 56cbaa44..b12d0b50 100644 --- a/templates/blink/intent_to_implement.html +++ b/templates/blink/intent_to_implement.html @@ -195,9 +195,24 @@ >{{feature.devtrial_instructions}} {% endif %} -

Flag name

+

Flag name on chrome://flags

{{feature.flag_name}} +

Finch feature name

+{{feature.finch_name}} + +{% if feature.non_finch_justification %} +

Non-finch justification

+

{{feature.non_finch_justification|urlize}}

+{% else %} + {% if not feature.finch_name %} +

Non-finch justification

+ None + {% endif %} +{% endif %} + + +

Requires code in //chrome?

{{feature.requires_embedder_support}}