diff --git a/api/channels_api.py b/api/channels_api.py index 6ad7cd3152da..7a5dda3c5dd4 100644 --- a/api/channels_api.py +++ b/api/channels_api.py @@ -23,9 +23,6 @@ import settings -SCHEDULE_CACHE_TIME = 60 * 60 # 1 hour - - def construct_chrome_channels_details(): omaha_data = fetchchannels.get_omaha_data() channels = {} @@ -34,71 +31,36 @@ def construct_chrome_channels_details(): for v in win_versions: channel = v['channel'] major_version = int(v['version'].split('.')[0]) - channels[channel] = fetch_chrome_release_info(major_version) + channels[channel] = fetchchannels.fetch_chrome_release_info(major_version) channels[channel]['version'] = major_version # Adjust for the brief period after next miletone gets promted to stable/beta # channel and their major versions are the same. if channels['stable']['version'] == channels['beta']['version']: new_beta_version = channels['stable']['version'] + 1 - channels['beta'] = fetch_chrome_release_info(new_beta_version) + channels['beta'] = fetchchannels.fetch_chrome_release_info(new_beta_version) channels['beta']['version'] = new_beta_version if channels['beta']['version'] == channels['dev']['version']: new_dev_version = channels['beta']['version'] + 1 - channels['dev'] = fetch_chrome_release_info(new_dev_version) + channels['dev'] = fetchchannels.fetch_chrome_release_info(new_dev_version) channels['dev']['version'] = new_dev_version # In the situation where some versions are in a gap between # stable and beta, show one as 'stable_soon'. if channels['stable']['version'] + 1 < channels['beta']['version']: stable_soon_version = channels['stable']['version'] + 1 - channels['stable_soon'] = fetch_chrome_release_info(stable_soon_version) + channels['stable_soon'] = fetchchannels.fetch_chrome_release_info(stable_soon_version) channels['stable_soon']['version'] = stable_soon_version return channels -def fetch_chrome_release_info(version): - key = 'chromerelease|%s' % version - - data = rediscache.get(key) - if data is None: - url = ('https://chromiumdash.appspot.com/fetch_milestone_schedule?' - 'mstone=%s' % version) - result = requests.get(url, timeout=60) - if result.status_code == 200: - try: - logging.info( - 'result.content is:\n%s', result.content[:settings.MAX_LOG_LINE]) - result_json = json.loads(result.content) - if 'mstones' in result_json: - data = result_json['mstones'][0] - del data['owners'] - del data['feature_freeze'] - del data['ldaps'] - rediscache.set(key, data, time=SCHEDULE_CACHE_TIME) - except ValueError: - pass # Handled by next statement - - if not data: - data = { - 'stable_date': None, - 'earliest_beta': None, - 'latest_beta': None, - 'mstone': version, - 'version': version, - } - # Note: we don't put placeholder data into redis. - - return data - - def construct_specified_milestones_details(start, end): channels = {} win_versions = list(range(start,end+1)) for milestone in win_versions: - channels[milestone] = fetch_chrome_release_info(milestone) + channels[milestone] = fetchchannels.fetch_chrome_release_info(milestone) return channels diff --git a/api/channels_api_test.py b/api/channels_api_test.py index 259dd17b3016..1526d08dbe05 100644 --- a/api/channels_api_test.py +++ b/api/channels_api_test.py @@ -28,7 +28,7 @@ def setUp(self): self.handler = channels_api.ChannelsAPI() self.request_path = '/api/v0/channels' - @mock.patch('api.channels_api.fetch_chrome_release_info') + @mock.patch('internals.fetchchannels.fetch_chrome_release_info') @mock.patch('internals.fetchchannels.get_omaha_data') def test_construct_chrome_channels_details( self, mock_get_omaha_data, mock_fetch_chrome_release_info): @@ -93,7 +93,7 @@ def fcri(version): self.maxDiff = None self.assertEqual(expected, actual) - @mock.patch('api.channels_api.fetch_chrome_release_info') + @mock.patch('internals.fetchchannels.fetch_chrome_release_info') @mock.patch('internals.fetchchannels.get_omaha_data') def test_construct_chrome_channels_details__beta_promotion( self, mock_get_omaha_data, mock_fetch_chrome_release_info): @@ -142,7 +142,7 @@ def fcri(version): self.maxDiff = None self.assertEqual(expected, actual) - @mock.patch('api.channels_api.fetch_chrome_release_info') + @mock.patch('internals.fetchchannels.fetch_chrome_release_info') @mock.patch('internals.fetchchannels.get_omaha_data') def test_construct_chrome_channels_details__dev_promotion( self, mock_get_omaha_data, mock_fetch_chrome_release_info): @@ -192,62 +192,7 @@ def fcri(version): self.maxDiff = None self.assertEqual(expected, actual) - @mock.patch('requests.get') - def test_fetch_chrome_release_info__found(self, mock_requests_get): - """We can get channel data from the chromiumdash app.""" - mock_requests_get.return_value = testing_config.Blank( - status_code=200, - content=json.dumps({ - 'mstones': [{ - 'owners': 'ignored', - 'feature_freeze': 'ignored', - 'ldaps': 'ignored', - 'everything else': 'kept', - }], - })) - - actual = channels_api.fetch_chrome_release_info(90) - - self.assertEqual( - {'everything else': 'kept'}, - actual) - - @mock.patch('requests.get') - def test_fetch_chrome_release_info__not_found(self, mock_requests_get): - """If chromiumdash app does not have the data, use a placeholder.""" - mock_requests_get.return_value = testing_config.Blank( - status_code=404, content='') - - actual = channels_api.fetch_chrome_release_info(91) - - self.assertEqual( - {'stable_date': None, - 'earliest_beta': None, - 'latest_beta': None, - 'mstone': 91, - 'version': 91, - }, - actual) - - @mock.patch('requests.get') - def test_fetch_chrome_release_info__error(self, mock_requests_get): - """We can get channel data from the chromiumdash app.""" - mock_requests_get.return_value = testing_config.Blank( - status_code=200, - content='{') - - actual = channels_api.fetch_chrome_release_info(90) - - self.assertEqual( - {'stable_date': None, - 'earliest_beta': None, - 'latest_beta': None, - 'mstone': 90, - 'version': 90, - }, - actual) - - @mock.patch('api.channels_api.fetch_chrome_release_info') + @mock.patch('internals.fetchchannels.fetch_chrome_release_info') def test_construct_specified_milestones_details( self, mock_fetch_chrome_release_info): mstone_data = { diff --git a/api/converters_test.py b/api/converters_test.py index 55e709de0ef4..b36ac7a48aef 100644 --- a/api/converters_test.py +++ b/api/converters_test.py @@ -298,7 +298,7 @@ def test_feature_entry_to_json_verbose__normal(self): 'is_released': True, 'category': 'Web Components', 'category_int': 1, - 'feature_type': 'New feature incubation', + 'feature_type': 'New or changed feature', 'feature_type_int': 0, 'is_enterprise_feature': False, 'intent_stage': 'Start prototyping', diff --git a/api/feature_latency_api.py b/api/feature_latency_api.py index eaf5741fc9a9..13d649d0321a 100644 --- a/api/feature_latency_api.py +++ b/api/feature_latency_api.py @@ -21,6 +21,7 @@ from api import channels_api from framework import basehandlers +from framework import permissions from internals import stage_helpers from internals.core_enums import * from internals.core_models import FeatureEntry, Stage @@ -53,6 +54,7 @@ def get_date_range( return start_date, end_date + @permissions.require_create_feature def do_get(self, **kwargs): """Calculate feature latency for features in a date range. diff --git a/api/feature_latency_api_test.py b/api/feature_latency_api_test.py index e76aad1fb903..465b298b9ba7 100644 --- a/api/feature_latency_api_test.py +++ b/api/feature_latency_api_test.py @@ -24,6 +24,7 @@ from api import feature_latency_api from internals.core_enums import * from internals.core_models import FeatureEntry, Stage, MilestoneSet +from internals import user_models test_app = flask.Flask(__name__) @@ -45,6 +46,10 @@ def make_feature(name, created_tuple, status, shipped): class FeatureLatencyAPITest(testing_config.CustomTestCase): def setUp(self): + self.app_admin = user_models.AppUser(email='admin@example.com') + self.app_admin.is_admin = True + self.app_admin.put() + self.handler = feature_latency_api.FeatureLatencyAPI() self.request_path = '/api/v0/feature-latency' @@ -64,6 +69,8 @@ def setUp(self): 'launched after end', (2023, 9, 29), ENABLED_BY_DEFAULT, 125) def tearDown(self): + testing_config.sign_out() + self.app_admin.key.delete() kinds: list[ndb.Model] = [FeatureEntry, Stage] for kind in kinds: for entity in kind.query(): @@ -85,8 +92,23 @@ def test_get_date_range__specified(self): datetime(2023, 12, 24)), actual) + def test_do_get__anon(self): + """Only users who can create features can view reports.""" + testing_config.sign_out() + with test_app.test_request_context(self.request_path): + with self.assertRaises(werkzeug.exceptions.Forbidden): + self.handler.do_get() + + def test_do_get__rando(self): + """Only users who can create features can view reports.""" + testing_config.sign_in('someone@example.com', 1232345) + with test_app.test_request_context(self.request_path): + with self.assertRaises(werkzeug.exceptions.Forbidden): + self.handler.do_get() + def test_do_get__normal(self): """It finds some feature launches to include and excludes others.""" + testing_config.sign_in('admin@example.com', 123567890) path = self.request_path + '?startAt=2023-01-01&endAt=2024-01-01' with test_app.test_request_context(path): actual = self.handler.do_get() @@ -103,6 +125,7 @@ def test_do_get__normal(self): def test_do_get__zero_results(self): """There were no test launches in this range.""" + testing_config.sign_in('admin@example.com', 123567890) path = self.request_path + '?startAt=2020-01-01&endAt=2020-03-01' with test_app.test_request_context(path): actual = self.handler.do_get() diff --git a/api/features_api.py b/api/features_api.py index e94484a7da36..55306069c26e 100644 --- a/api/features_api.py +++ b/api/features_api.py @@ -35,6 +35,7 @@ from internals import feature_helpers from internals import search from internals import search_fulltext +from internals import stage_helpers from internals.user_models import AppUser import settings @@ -91,13 +92,14 @@ def do_search(self): sort_spec = self.request.args.get('sort') num = self.get_int_arg('num', search.DEFAULT_RESULTS_PER_PAGE) start = self.get_int_arg('start', 0) + name_only = self.get_bool_arg('name_only', False) show_enterprise = ( 'feature_type' in user_query or self.get_bool_arg('showEnterprise')) try: - features_on_page, total_count = search.process_query( + features_on_page, total_count = search.process_query_using_cache( user_query, sort_spec=sort_spec, show_unlisted=show_unlisted_features, - show_enterprise=show_enterprise, start=start, num=num) + show_enterprise=show_enterprise, start=start, num=num, name_only=name_only) except ValueError as err: self.abort(400, msg=str(err)) @@ -179,9 +181,9 @@ def _patch_update_stages( self, stage_changes_list: list[dict[str, Any]], changed_fields: CHANGED_FIELDS_LIST_TYPE - ) -> bool: + ) -> list[Stage]: """Update stage fields with changes provided in the PATCH request.""" - stages: list[Stage] = [] + stages_to_store: list[Stage] = [] for change_info in stage_changes_list: stage_was_updated = False # Check if valid ID is provided and fetch stage if it exists. @@ -222,29 +224,32 @@ def _patch_update_stages( stage.milestones = milestones if stage_was_updated: - stages.append(stage) + stages_to_store.append(stage) # Save all of the updates made. - if stages: - ndb.put_multi(stages) + if stages_to_store: + ndb.put_multi(stages_to_store) - # Return a boolean representing if any changes were made to any stages. - return len(stages) > 0 + # Return the list of modified stages. + return stages_to_store def _patch_update_special_fields( self, feature: FeatureEntry, feature_changes: dict[str, Any], - has_updated: bool + has_updated: bool, + updated_stages: list[Stage], + changed_fields: CHANGED_FIELDS_LIST_TYPE, ) -> None: """Handle any special FeatureEntry fields.""" now = datetime.now() - # Handle accuracy notifications if this is an accuracy verification request. + # Set accurate_as_of if this is an accuracy verification request. if 'accurate_as_of' in feature_changes: feature.accurate_as_of = now feature.outstanding_notifications = 0 has_updated = True + # Set enterprise first notification milestones. if is_update_first_notification_milestone(feature, feature_changes): feature.first_enterprise_notification_milestone = int(feature_changes['first_enterprise_notification_milestone']) has_updated = True @@ -254,6 +259,32 @@ def _patch_update_special_fields( if should_remove_first_notice_milestone(feature, feature_changes): feature.first_enterprise_notification_milestone = None + # If a shipping stage was edited, set shipping_year based on milestones. + updated_shipping_stages = [ + us for us in updated_stages + if us.stage_type in STAGE_TYPES_SHIPPING.values()] + if updated_shipping_stages: + existing_shipping_stages = ( + stage_helpers.get_all_shipping_stages_with_milestones( + feature_id=feature.key.integer_id())) + shipping_stage_dict = { + es.key.integer_id(): es + for es in existing_shipping_stages + } + shipping_stage_dict.update({ + uss.key.integer_id(): uss + for uss in updated_shipping_stages + }) + earliest = stage_helpers.find_earliest_milestone( + list(shipping_stage_dict.values())) + if earliest: + year = stage_helpers.look_up_year(earliest) + if year != feature.shipping_year: + changed_fields.append(('shipping_year', feature.shipping_year, year)) + feature.shipping_year = year + has_updated = True + + # If changes were made, set the feature.updated timestamp. if has_updated: user_email = self.get_current_user().email() feature.updater_email = user_email @@ -263,10 +294,11 @@ def _patch_update_feature( self, feature: FeatureEntry, feature_changes: dict[str, Any], - has_updated: bool, + updated_stages: list[Stage], changed_fields: CHANGED_FIELDS_LIST_TYPE ) -> None: """Update feature fields with changes provided in the PATCH request.""" + has_updated = len(updated_stages) > 0 for field, field_type in api_specs.FEATURE_FIELD_DATA_TYPES: if field not in feature_changes: continue @@ -276,7 +308,8 @@ def _patch_update_feature( changed_fields.append((field, old_value, new_value)) has_updated = True - self._patch_update_special_fields(feature, feature_changes, has_updated) + self._patch_update_special_fields( + feature, feature_changes, has_updated, updated_stages, changed_fields) feature.put() def do_patch(self, **kwargs): @@ -298,14 +331,15 @@ def do_patch(self, **kwargs): return redirect_resp changed_fields: CHANGED_FIELDS_LIST_TYPE = [] - has_updated = self._patch_update_stages(body['stages'], changed_fields) + updated_stages = self._patch_update_stages(body['stages'], changed_fields) self._patch_update_feature( - feature, body['feature_changes'], has_updated, changed_fields) + feature, body['feature_changes'], updated_stages, changed_fields) notifier_helpers.notify_subscribers_and_save_amendments( feature, changed_fields, notify=True) # Remove all feature-related cache. - rediscache.delete_keys_with_prefix(FeatureEntry.feature_cache_prefix()) + rediscache.delete_keys_with_prefix(FeatureEntry.DEFAULT_CACHE_KEY) + rediscache.delete_keys_with_prefix(FeatureEntry.SEARCH_CACHE_KEY) # Update full-text index. if feature: search_fulltext.index_feature(feature) @@ -329,7 +363,8 @@ def do_delete(self, **kwargs) -> dict[str, str]: self.abort(403) feature.deleted = True feature.put() - rediscache.delete_keys_with_prefix(FeatureEntry.feature_cache_prefix()) + rediscache.delete_keys_with_prefix(FeatureEntry.DEFAULT_CACHE_KEY) + rediscache.delete_keys_with_prefix(FeatureEntry.SEARCH_CACHE_KEY) # Write for new FeatureEntry entity. feature_entry: FeatureEntry | None = ( diff --git a/api/features_api_test.py b/api/features_api_test.py index 06e1b5811220..b8525ea19645 100644 --- a/api/features_api_test.py +++ b/api/features_api_test.py @@ -154,8 +154,9 @@ def tearDown(self): entity.key.delete() testing_config.sign_out() - rediscache.delete_keys_with_prefix('features|*') - rediscache.delete_keys_with_prefix('FeatureEntries|*') + rediscache.delete_keys_with_prefix('features') + rediscache.delete_keys_with_prefix('FeatureEntries') + rediscache.delete_keys_with_prefix('FeatureNames') def test_get__all_listed(self): """Get all features that are listed.""" @@ -169,6 +170,21 @@ def test_get__all_listed(self): self.assertEqual('feature two', actual['features'][0]['name']) self.assertEqual('feature one', actual['features'][1]['name']) + def test_get__all_listed_feature_names(self): + """Get all feature names that are listed.""" + url = self.request_path + '?name_only=true' + with test_app.test_request_context(url): + actual = self.handler.do_get() + + # Comparing only the total number of features and names, + # as it only returns feature names. + self.assertEqual(2, len(actual['features'])) + self.assertEqual(2, actual['total_count']) + self.assertEqual(2, len(actual['features'][0])) + self.assertEqual('feature two', actual['features'][0]['name']) + self.assertEqual(2, len(actual['features'][1])) + self.assertEqual('feature one', actual['features'][1]['name']) + def test_get__all_listed__pagination(self): """Get a pagination page features that are listed.""" # User wants only 1 result, starting at index 0 @@ -210,6 +226,45 @@ def test_get__all_listed__pagination(self): self.assertEqual(0, len(actual['features'])) self.assertEqual(2, actual['total_count']) + # User wants only 1 result, starting at index 0 + url = self.request_path + '?num=1&name_only=true' + with test_app.test_request_context(url): + actual = self.handler.do_get() + self.assertEqual(1, len(actual['features'])) + self.assertEqual(2, actual['total_count']) + self.assertEqual('feature two', actual['features'][0]['name']) + + # User wants only 1 result, starting at index 1 + url = self.request_path + '?num=1&start=1&name_only=true' + with test_app.test_request_context(url): + actual = self.handler.do_get() + self.assertEqual(1, len(actual['features'])) + self.assertEqual(2, actual['total_count']) + self.assertEqual('feature one', actual['features'][0]['name']) + + # User wants only 1 result, starting at index 2, but there are no more. + url = self.request_path + '?num=1&start=2&name_only=true' + with test_app.test_request_context(url): + actual = self.handler.do_get() + self.assertEqual(0, len(actual['features'])) + self.assertEqual(2, actual['total_count']) + + # User wants only more results that we have + url = self.request_path + '?num=999&name_only=true' + with test_app.test_request_context(url): + actual = self.handler.do_get() + self.assertEqual(2, len(actual['features'])) + self.assertEqual(2, actual['total_count']) + self.assertEqual('feature two', actual['features'][0]['name']) + self.assertEqual('feature one', actual['features'][1]['name']) + + # User wants only the result count, zero actual results. + url = self.request_path + '?num=0&name_only=true' + with test_app.test_request_context(url): + actual = self.handler.do_get() + self.assertEqual(0, len(actual['features'])) + self.assertEqual(2, actual['total_count']) + def test_get__all_listed__bad_pagination(self): """Reject requests that have bad pagination params.""" # Malformed start parameter @@ -737,11 +792,11 @@ def test_patch__enterprise_first_notice_becomes_not_breaking_feature(self, mock_ """PATCH request successful with first_enterprise_notification_milestone deleted.""" stable_date = _datetime_to_str(datetime.now().replace(year=datetime.now().year + 1, day=1)) mock_call.return_value = { 100: { 'version': 100, 'stable_date': stable_date } } - + self.feature_1.enterprise_impact = core_enums.ENTERPRISE_IMPACT_MEDIUM self.feature_1.first_enterprise_notification_milestone = 100 self.feature_1.put() - + # Signed-in user with permissions. testing_config.sign_in('admin@example.com', 123567890) @@ -769,11 +824,11 @@ def test_patch__first_notice_becomes_not_breaking_feature_already_published(self """PATCH request successful with first_enterprise_notification_milestone not deleted.""" stable_date = _datetime_to_str(datetime.now().replace(year=datetime.now().year - 1, day=1)) mock_call.return_value = { 100: { 'version': 100, 'stable_date': stable_date } } - + self.feature_1.enterprise_impact = core_enums.ENTERPRISE_IMPACT_MEDIUM self.feature_1.first_enterprise_notification_milestone = 100 self.feature_1.put() - + # Signed-in user with permissions. testing_config.sign_in('admin@example.com', 123567890) @@ -835,7 +890,7 @@ def test_patch__enterprise_first_notice_already_published(self, mock_call): """PATCH request successful with no changes to first_enterprise_notification_milestone.""" now = datetime.now() mock_call.return_value = { - 100: { + 100: { 'version': 100, 'stable_date': _datetime_to_str(now.replace(year=now.year - 1, day=1)) }, diff --git a/api/review_latency_api.py b/api/review_latency_api.py index 516ee479fc2b..7ce4aea8f667 100644 --- a/api/review_latency_api.py +++ b/api/review_latency_api.py @@ -23,6 +23,7 @@ from chromestatus_openapi.models.review_latency import ReviewLatency from framework import basehandlers +from framework import permissions from internals import slo from internals.core_enums import * from internals.core_models import FeatureEntry @@ -39,6 +40,7 @@ class ReviewLatencyAPI(basehandlers.APIHandler): """Implements the OpenAPI /spec_mentors path.""" + @permissions.require_create_feature def do_get(self, **kwargs): """Get a list of matching spec mentors. diff --git a/api/review_latency_api_test.py b/api/review_latency_api_test.py index 48465aa17050..7836f56c634c 100644 --- a/api/review_latency_api_test.py +++ b/api/review_latency_api_test.py @@ -14,8 +14,9 @@ import testing_config # isort: split +import flask +import werkzeug.exceptions # Flask HTTP stuff. from datetime import datetime - from unittest import mock from google.cloud import ndb # type: ignore @@ -23,6 +24,10 @@ from internals.core_enums import * from internals.core_models import FeatureEntry from internals.review_models import Gate +from internals import user_models + + +test_app = flask.Flask(__name__) def make_feature_and_gates(name): @@ -41,6 +46,10 @@ def make_feature_and_gates(name): class ReviewLatencyAPITest(testing_config.CustomTestCase): def setUp(self): + self.app_admin = user_models.AppUser(email='admin@example.com') + self.app_admin.is_admin = True + self.app_admin.put() + self.handler = review_latency_api.ReviewLatencyAPI() self.request_path = '/api/v0/review-latency' @@ -53,6 +62,8 @@ def setUp(self): self.last_week = datetime(2024, 3, 15) def tearDown(self): + testing_config.sign_out() + self.app_admin.key.delete() kinds: list[ndb.Model] = [FeatureEntry, Gate] for kind in kinds: for entity in kind.query(): @@ -60,18 +71,36 @@ def tearDown(self): def test_do_get__nothing_requested(self): """When no reviews have been started, the result is empty.""" - actual = self.handler.do_get(today=self.today) + testing_config.sign_in('admin@example.com', 123567890) + with test_app.test_request_context(self.request_path): + actual = self.handler.do_get(today=self.today) self.assertEqual([], actual) + def test_do_get__anon(self): + """Only users who can create features can view reports.""" + testing_config.sign_out() + with test_app.test_request_context(self.request_path): + with self.assertRaises(werkzeug.exceptions.Forbidden): + self.handler.do_get(today=self.today) + + def test_do_get__rando(self): + """Only users who can create features can view reports.""" + testing_config.sign_in('someone@example.com', 1232345) + with test_app.test_request_context(self.request_path): + with self.assertRaises(werkzeug.exceptions.Forbidden): + self.handler.do_get(today=self.today) + def test_do_get__normal(self): """It produces a report of review latency.""" + testing_config.sign_in('admin@example.com', 123567890) self.g_1_1.requested_on = self.last_week self.g_1_1.responded_on = self.today self.g_1_1.put() self.g_1_2.requested_on = self.last_week self.g_1_2.put() - actual = self.handler.do_get(today=self.today) + with test_app.test_request_context(self.request_path): + actual = self.handler.do_get(today=self.today) expected = [ { 'feature': {'name': 'Feature one', 'id': self.fe_1_id}, @@ -88,6 +117,7 @@ def test_do_get__normal(self): def test_do_get__sorting(self): """Multiple results are sorted by earlest review request.""" + testing_config.sign_in('admin@example.com', 123567890) self.g_1_1.requested_on = self.yesterday self.g_1_1.responded_on = self.today self.g_1_1.put() @@ -99,7 +129,8 @@ def test_do_get__sorting(self): self.g_2_1.responded_on = self.yesterday self.g_2_1.put() - actual = self.handler.do_get(today=self.today) + with test_app.test_request_context(self.request_path): + actual = self.handler.do_get(today=self.today) expected = [ { 'feature': {'name': 'Feature two', 'id': self.fe_2_id}, diff --git a/api/spec_mentors_api_test.py b/api/spec_mentors_api_test.py index 911ca50aa06f..dda0253d0887 100644 --- a/api/spec_mentors_api_test.py +++ b/api/spec_mentors_api_test.py @@ -62,8 +62,8 @@ def tearDown(self): self.app_admin.key.delete() testing_config.sign_out() - rediscache.delete_keys_with_prefix('features|*') - rediscache.delete_keys_with_prefix('FeatureEntries|*') + rediscache.delete_keys_with_prefix('features') + rediscache.delete_keys_with_prefix('FeatureEntries') def createFeature(self, params) -> FeatureEntry: with test_app.test_request_context( diff --git a/client-src/elements/chromedash-all-features-page.ts b/client-src/elements/chromedash-all-features-page.ts index 15c9321d465d..4c4d494140e3 100644 --- a/client-src/elements/chromedash-all-features-page.ts +++ b/client-src/elements/chromedash-all-features-page.ts @@ -44,6 +44,10 @@ export class ChromedashAllFeaturesPage extends LitElement { num = 100; @state() starredFeatures: Set = new Set(); + @state() + isNewfeaturesPage = false; + @state() + nameOnly = false; @queryAll('chromedash-feature-table') chromedashFeatureTables; @@ -83,6 +87,9 @@ export class ChromedashAllFeaturesPage extends LitElement { ) { this.num = parseInt(this.rawQuery['num']); } + if (this.isNewfeaturesPage) { + this.nameOnly = true; + } } fetchData() { @@ -131,6 +138,7 @@ export class ChromedashAllFeaturesPage extends LitElement { .sortSpec=${this.sortSpec} .start=${this.start} .num=${this.num} + .nameOnly=${this.nameOnly} ?showQuery=${this.showQuery} ?signedIn=${Boolean(this.user)} ?canEdit=${this.user && this.user.can_edit_all} diff --git a/client-src/elements/chromedash-app.ts b/client-src/elements/chromedash-app.ts index 9347a236552f..85cfb47988b0 100644 --- a/client-src/elements/chromedash-app.ts +++ b/client-src/elements/chromedash-app.ts @@ -8,6 +8,7 @@ import {User} from '../js-src/cs-client.js'; import {ChromedashDrawer, DRAWER_WIDTH_PX} from './chromedash-drawer.js'; import {ChromedashGateColumn} from './chromedash-gate-column.js'; import { + clearURLParams, IS_MOBILE, isoDateString, parseRawQuery, @@ -358,10 +359,12 @@ export class ChromedashApp extends LitElement { this.pageComponent.showQuery = false; this.pageComponent.rawQuery = parseRawQuery(ctx.querystring); }); - page('/newfeatures', ctx => { + page('/newfeatures', () => page.redirect('/features')); + page('/features', ctx => { if (!this.setupNewPage(ctx, 'chromedash-all-features-page', true)) return; this.pageComponent.user = this.user; this.pageComponent.rawQuery = parseRawQuery(ctx.querystring); + this.pageComponent.isNewfeaturesPage = true; this.pageComponent.addEventListener( 'search', this.handleSearchQuery.bind(this) @@ -548,6 +551,7 @@ export class ChromedashApp extends LitElement { handleSearchQuery(e) { updateURLParams('q', e.detail.query); + clearURLParams('start'); } showSidebar() { @@ -622,11 +626,11 @@ export class ChromedashApp extends LitElement { } renderRolloutBanner(currentPage) { - if (currentPage.startsWith('/newfeatures')) { + if (currentPage.startsWith('/features')) { return html` -
- Back to the old features page -
+

+ Use the old features page +

`; } @@ -678,10 +682,10 @@ export class ChromedashApp extends LitElement { .timestamp=${this.bannerTime} > - ${this.renderRolloutBanner(this.currentPage)}
${this.renderContentAndSidebar()}
+ ${this.renderRolloutBanner(this.currentPage)} diff --git a/client-src/elements/chromedash-drawer.ts b/client-src/elements/chromedash-drawer.ts index 38f8289d255a..2506d1c60134 100644 --- a/client-src/elements/chromedash-drawer.ts +++ b/client-src/elements/chromedash-drawer.ts @@ -231,7 +231,7 @@ export class ChromedashDrawer extends LitElement { } isCurrentPage(href) { - return this.currentPage.startsWith(href); + return this.currentPage === href; } toggleDrawerActions() { @@ -261,6 +261,12 @@ export class ChromedashDrawer extends LitElement { const myFeaturesMenu = this.renderMyFeaturesMenu(); const adminMenu = this.renderAdminMenu(); + const year = new Date().getFullYear(); + const shippingThisYear = this.renderNavItem( + '/features?q=shipping_year=' + year, + 'Shipping ' + year + ); + return html` ${accountMenu} ${this.renderNavItem('/roadmap', 'Roadmap')} - ${this.renderNavItem('/features', 'All features')} ${myFeaturesMenu} + ${this.renderNavItem('/features', 'All features')} ${shippingThisYear} + ${myFeaturesMenu}
Stats
${this.renderNavItem('/metrics/css/popularity', 'CSS')} @@ -346,7 +353,6 @@ export class ChromedashDrawer extends LitElement { ${this.renderNavItem('/admin/feature_links', 'Feature links')} ${this.renderNavItem('/reports/feature-latency', 'Feature latency')} ${this.renderNavItem('/reports/review-latency', 'Review latency')} - ${this.renderNavItem('/admin/slo_report', 'SLO report JSON')} ${this.renderNavItem('/admin/blink', 'Subscriptions')} ${this.renderNavItem('/admin/find_stop_words', 'Find stop words JSON')} `; diff --git a/client-src/elements/chromedash-feature-pagination.ts b/client-src/elements/chromedash-feature-pagination.ts index b1d69d3d1ee2..ee2455fc1f57 100644 --- a/client-src/elements/chromedash-feature-pagination.ts +++ b/client-src/elements/chromedash-feature-pagination.ts @@ -23,7 +23,11 @@ import { nothing, } from 'lit'; import {customElement, property} from 'lit/decorators.js'; -import {formatURLParams} from './utils.js'; +import { + formatURLParams, + formatUrlForRelativeOffset, + formatUrlForOffset, +} from './utils.js'; import {ifDefined} from 'lit/directives/if-defined.js'; import {range} from 'lit/directives/range.js'; import {map} from 'lit/directives/map.js'; @@ -82,22 +86,6 @@ export class ChromedashFeaturePagination extends LitElement { ]; } - formatUrlForRelativeOffset(delta: number): string | undefined { - const offset = this.start + delta; - if ( - this.totalCount === undefined || - offset <= -this.pageSize || - offset >= this.totalCount - ) { - return undefined; - } - return this.formatUrlForOffset(Math.max(0, offset)); - } - - formatUrlForOffset(offset: number): string { - return formatURLParams('start', offset).toString(); - } - renderPageButtons(): TemplateResult { if (this.totalCount === undefined || this.totalCount === 0) { return html``; @@ -136,7 +124,7 @@ export class ChromedashFeaturePagination extends LitElement { variant="text" id="jump_1" class="page-button ${0 === currentPage ? 'active' : ''}" - href=${this.formatUrlForOffset(0)} + href=${formatUrlForOffset(0)} > ${1} @@ -148,7 +136,7 @@ export class ChromedashFeaturePagination extends LitElement { variant="text" id="jump_${i + 1}" class="page-button ${i === currentPage ? 'active' : ''}" - href=${this.formatUrlForOffset(i * this.pageSize)} + href=${formatUrlForOffset(i * this.pageSize)} > ${i + 1} @@ -160,7 +148,7 @@ export class ChromedashFeaturePagination extends LitElement { variant="text" id="jump_${numPages}" class="page-button ${numPages - 1 === currentPage ? 'active' : ''}" - href=${this.formatUrlForOffset((numPages - 1) * this.pageSize)} + href=${formatUrlForOffset((numPages - 1) * this.pageSize)} > ${numPages} ` @@ -202,8 +190,18 @@ export class ChromedashFeaturePagination extends LitElement { return html``; } - const prevUrl = this.formatUrlForRelativeOffset(-this.pageSize); - const nextUrl = this.formatUrlForRelativeOffset(this.pageSize); + const prevUrl = formatUrlForRelativeOffset( + this.start, + -this.pageSize, + this.pageSize, + this.totalCount + ); + const nextUrl = formatUrlForRelativeOffset( + this.start, + this.pageSize, + this.pageSize, + this.totalCount + ); return html`