Update PATCH request handler to make stage entity changes (#3013)
* update patch request to handle stage changes * changes suggested by @jrobbins
This commit is contained in:
Daniel Smith
GitHub
Родитель
a13a09271c
Коммит
fe9b97277b
|
|
@ -0,0 +1,116 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# Copyright 2023 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.
|
||||
|
||||
# Data type for lists defining field data type information.
|
||||
FIELD_INFO_DATA_TYPE = list[tuple[str, str]]
|
||||
|
||||
# List with fields that can be edited on feature create/update
|
||||
# and their data types.
|
||||
# Field name, data type
|
||||
FEATURE_FIELD_DATA_TYPES: FIELD_INFO_DATA_TYPE = [
|
||||
('activation_risks', 'str'),
|
||||
('adoption_expectation', 'str'),
|
||||
('adoption_plan', 'str'),
|
||||
('all_platforms', 'bool'),
|
||||
('all_platforms_descr', 'str'),
|
||||
('anticipated_spec_changes', 'str'),
|
||||
('api_spec', 'bool'),
|
||||
('availability_expectation', 'str'),
|
||||
('blink_components', 'list'),
|
||||
('breaking_change', 'bool'),
|
||||
('bug_url', 'str'),
|
||||
('category', 'int'),
|
||||
('cc_emails', 'list'),
|
||||
('comments', 'str'),
|
||||
('debuggability', 'str'),
|
||||
('devrel', 'list'),
|
||||
('devtrial_instructions', 'str'),
|
||||
('doc_links', 'list'),
|
||||
('editors_emails', 'list'),
|
||||
('enterprise_feature_categories', 'list'),
|
||||
('ergonomics_risks', 'str'),
|
||||
('explainer_links', 'list'),
|
||||
('feature_type', 'int'),
|
||||
('ff_views', 'int'),
|
||||
('ff_views_link', 'str'),
|
||||
('ff_views_notes', 'str'),
|
||||
('flag_name', 'str'),
|
||||
('impl_status_chrome', 'int'),
|
||||
('initial_public_proposal_url', 'str'),
|
||||
('intent_stage', 'int'),
|
||||
('interop_compat_risks', 'str'),
|
||||
('launch_bug_url', 'str'),
|
||||
('screenshot_links', 'list'),
|
||||
('measurement', 'str'),
|
||||
('motivation', 'str'),
|
||||
('name', 'str'),
|
||||
('non_oss_deps', 'str'),
|
||||
('ongoing_constraints', 'str'),
|
||||
('other_views_notes', 'str'),
|
||||
('owner_emails', 'list'),
|
||||
('prefixed', 'bool'),
|
||||
('privacy_review_status', 'int'),
|
||||
('requires_embedder_support', 'bool'),
|
||||
('safari_views', 'int'),
|
||||
('safari_views_link', 'str'),
|
||||
('safari_views_notes', 'str'),
|
||||
('sample_links', 'list'),
|
||||
('search_tags', 'list'),
|
||||
('security_review_status', 'int'),
|
||||
('security_risks', 'str'),
|
||||
('spec_link', 'str'),
|
||||
('spec_mentors', 'list'),
|
||||
('standard_maturity', 'int'),
|
||||
('summary', 'str'),
|
||||
('tag_review', 'str'),
|
||||
('tag_review_status', 'int'),
|
||||
('unlisted', 'bool'),
|
||||
('web_dev_views', 'int'),
|
||||
('web_dev_views_link', 'str'),
|
||||
('web_dev_views_notes', 'str'),
|
||||
('webview_risks', 'str'),
|
||||
('wpt', 'bool'),
|
||||
('wpt_descr', 'str'),
|
||||
]
|
||||
|
||||
# List with stage fields that can be edited on feature create/update
|
||||
# and their data types.
|
||||
# Field name, data type
|
||||
STAGE_FIELD_DATA_TYPES: FIELD_INFO_DATA_TYPE = [
|
||||
('display_name', 'str'),
|
||||
('browser', 'str'),
|
||||
('experiment_goals', 'str'),
|
||||
('experiment_risks', 'str'),
|
||||
('experiment_extension_reason', 'str'),
|
||||
('intent_thread_url', 'str'),
|
||||
('origin_trial_feedback_url', 'str'),
|
||||
('announcement_url', 'str'),
|
||||
('rollout_impact', 'int'),
|
||||
('rollout_milestone', 'int'),
|
||||
('rollout_platforms', 'list'),
|
||||
('rollout_details', 'str'),
|
||||
('enterprise_policies', 'str'),
|
||||
]
|
||||
|
||||
MILESTONESET_FIELD_DATA_TYPES: FIELD_INFO_DATA_TYPE = [
|
||||
('desktop_first', 'int'),
|
||||
('desktop_last', 'int'),
|
||||
('android_first', 'int'),
|
||||
('android_last', 'int'),
|
||||
('ios_first', 'int'),
|
||||
('ios_last', 'int'),
|
||||
('webview_first', 'int'),
|
||||
('webview_last', 'int'),
|
||||
]
|
||||
|
|
@ -17,91 +17,24 @@ from datetime import datetime
|
|||
from typing import Any
|
||||
from google.cloud import ndb
|
||||
|
||||
from api import api_specs
|
||||
from api import converters
|
||||
from framework import basehandlers
|
||||
from framework import permissions
|
||||
from framework import rediscache
|
||||
from framework import users
|
||||
from internals.core_enums import *
|
||||
from internals.core_models import FeatureEntry, Stage
|
||||
from internals.core_models import FeatureEntry, MilestoneSet, Stage
|
||||
from internals.review_models import Gate
|
||||
from internals.data_types import VerboseFeatureDict
|
||||
from internals import feature_helpers
|
||||
from internals import search
|
||||
import settings
|
||||
|
||||
|
||||
class FeaturesAPI(basehandlers.APIHandler):
|
||||
"""Features are the the main records that we track."""
|
||||
|
||||
# Dictionary with fields that can be edited on feature creation
|
||||
# and their data types.
|
||||
# Field name, data type
|
||||
FIELD_DATA_TYPES_CREATE: dict[str, str] = {
|
||||
'activation_risks': 'str',
|
||||
'adoption_expectation': 'str',
|
||||
'adoption_plan': 'str',
|
||||
'all_platforms': 'bool',
|
||||
'all_platforms_descr': 'str',
|
||||
'anticipated_spec_changes': 'str',
|
||||
'api_spec': 'bool',
|
||||
'availability_expectation': 'str',
|
||||
'blink_components': 'list',
|
||||
'breaking_change': 'bool',
|
||||
'bug_url': 'str',
|
||||
'category': 'int',
|
||||
'cc_emails': 'list',
|
||||
'comments': 'str',
|
||||
'debuggability': 'str',
|
||||
'devrel': 'list',
|
||||
'devtrial_instructions': 'str',
|
||||
'doc_links': 'list',
|
||||
'editors_emails': 'list',
|
||||
'enterprise_feature_categories': 'list',
|
||||
'ergonomics_risks': 'str',
|
||||
'explainer_links': 'list',
|
||||
'feature_type': 'int',
|
||||
'ff_views': 'int',
|
||||
'ff_views_link': 'str',
|
||||
'ff_views_notes': 'str',
|
||||
'flag_name': 'str',
|
||||
'impl_status_chrome': 'int',
|
||||
'initial_public_proposal_url': 'str',
|
||||
'intent_stage': 'int',
|
||||
'interop_compat_risks': 'str',
|
||||
'launch_bug_url': 'str',
|
||||
'screenshot_links': 'list',
|
||||
'measurement': 'str',
|
||||
'motivation': 'str',
|
||||
'name': 'str',
|
||||
'non_oss_deps': 'str',
|
||||
'ongoing_constraints': 'str',
|
||||
'other_views_notes': 'str',
|
||||
'owner_emails': 'list',
|
||||
'prefixed': 'bool',
|
||||
'privacy_review_status': 'int',
|
||||
'requires_embedder_support': 'bool',
|
||||
'safari_views': 'int',
|
||||
'safari_views_link': 'str',
|
||||
'safari_views_notes': 'str',
|
||||
'sample_links': 'list',
|
||||
'search_tags': 'list',
|
||||
'security_review_status': 'int',
|
||||
'security_risks': 'str',
|
||||
'spec_link': 'str',
|
||||
'spec_mentors': 'list',
|
||||
'standard_maturity': 'int',
|
||||
'summary': 'str',
|
||||
'tag_review': 'str',
|
||||
'tag_review_status': 'int',
|
||||
'unlisted': 'bool',
|
||||
'web_dev_views': 'int',
|
||||
'web_dev_views_link': 'str',
|
||||
'web_dev_views_notes': 'str',
|
||||
'webview_risks': 'str',
|
||||
'wpt': 'bool',
|
||||
'wpt_descr': 'str',
|
||||
}
|
||||
|
||||
def _abort_invalid_data_type(
|
||||
self, field: str, field_type: str, value: Any) -> None:
|
||||
"""Abort the process if an invalid data type is given."""
|
||||
|
|
@ -111,14 +44,10 @@ class FeaturesAPI(basehandlers.APIHandler):
|
|||
def _format_field_val(
|
||||
self,
|
||||
field: str,
|
||||
value: Any
|
||||
field_type: str,
|
||||
value: Any,
|
||||
) -> str | int | bool | list | None:
|
||||
"""Format the given feature value based on the field type."""
|
||||
# Only fields with defined data types are allowed.
|
||||
if field not in self.FIELD_DATA_TYPES_CREATE:
|
||||
self.abort(400, msg=f'Field "{field}" data format not found.')
|
||||
|
||||
field_type = self.FIELD_DATA_TYPES_CREATE[field]
|
||||
|
||||
# If the field is empty, no need to format.
|
||||
if value is None:
|
||||
|
|
@ -202,8 +131,12 @@ class FeaturesAPI(basehandlers.APIHandler):
|
|||
if field not in body:
|
||||
self.abort(400, msg=f'Required field "{field}" not provided.')
|
||||
|
||||
fields_dict = {field: self._format_field_val(field, value)
|
||||
for field, value in body.items()}
|
||||
fields_dict = {}
|
||||
for field, field_type in api_specs.FEATURE_FIELD_DATA_TYPES:
|
||||
if field in body:
|
||||
fields_dict[field] = self._format_field_val(
|
||||
field, field_type, body[field])
|
||||
|
||||
# Try to create the feature using the provided data.
|
||||
try:
|
||||
feature = FeatureEntry(**fields_dict,
|
||||
|
|
@ -240,16 +173,108 @@ class FeaturesAPI(basehandlers.APIHandler):
|
|||
if new_gates:
|
||||
ndb.put_multi(new_gates)
|
||||
|
||||
def _update_field_value(
|
||||
self,
|
||||
entity: FeatureEntry | MilestoneSet | Stage,
|
||||
field: str,
|
||||
field_type: str,
|
||||
value: Any
|
||||
) -> None:
|
||||
new_value = self._format_field_val(field, field_type, value)
|
||||
setattr(entity, field, new_value)
|
||||
|
||||
def _patch_update_stages(
|
||||
self,
|
||||
stage_changes_list: list[dict[str, Any]]
|
||||
) -> bool:
|
||||
"""Update stage fields with changes provided in the PATCH request."""
|
||||
stages: 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.
|
||||
if 'id' not in change_info:
|
||||
self.abort(400, msg='Missing stage ID in stage updates')
|
||||
id = change_info['id']
|
||||
stage = Stage.get_by_id(id)
|
||||
if not stage:
|
||||
self.abort(400, msg=f'Stage not found for ID {id}')
|
||||
|
||||
# Update stage fields.
|
||||
for field, field_type in api_specs.STAGE_FIELD_DATA_TYPES:
|
||||
if field not in change_info:
|
||||
continue
|
||||
self._update_field_value(stage, field, field_type, change_info[field])
|
||||
stage_was_updated = True
|
||||
|
||||
# Update milestone fields.
|
||||
milestones = stage.milestones
|
||||
for field, field_type in api_specs.MILESTONESET_FIELD_DATA_TYPES:
|
||||
if field not in change_info:
|
||||
continue
|
||||
if milestones is None:
|
||||
milestones = MilestoneSet()
|
||||
self._update_field_value(
|
||||
milestones, field, field_type, change_info[field])
|
||||
stage_was_updated = True
|
||||
stage.milestones = milestones
|
||||
|
||||
if stage_was_updated:
|
||||
stages.append(stage)
|
||||
|
||||
# Save all of the updates made.
|
||||
# Return a boolean representing if any changes were made to any stages.
|
||||
if stages:
|
||||
ndb.put_multi(stages)
|
||||
return True
|
||||
return False
|
||||
|
||||
def _patch_update_special_fields(
|
||||
self,
|
||||
feature: FeatureEntry,
|
||||
feature_changes: dict[str, Any],
|
||||
has_updated: bool
|
||||
) -> None:
|
||||
"""Handle any special FeatureEntry fields."""
|
||||
now = datetime.now()
|
||||
# Handle accuracy notifications 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
|
||||
|
||||
if has_updated:
|
||||
user_email = self.get_current_user().email()
|
||||
feature.updater_email = user_email
|
||||
feature.updated = now
|
||||
|
||||
def _patch_update_feature(
|
||||
self,
|
||||
feature: FeatureEntry,
|
||||
feature_changes: dict[str, Any],
|
||||
has_updated: bool
|
||||
) -> None:
|
||||
"""Update feature fields with changes provided in the PATCH request."""
|
||||
for field, field_type in api_specs.FEATURE_FIELD_DATA_TYPES:
|
||||
if field not in feature_changes:
|
||||
continue
|
||||
self._update_field_value(
|
||||
feature, field, field_type, feature_changes[field])
|
||||
has_updated = True
|
||||
|
||||
self._patch_update_special_fields(feature, feature_changes, has_updated)
|
||||
feature.put()
|
||||
|
||||
def do_patch(self, **kwargs):
|
||||
"""Handle PATCH requests to update fields in a single feature."""
|
||||
feature_id = kwargs['feature_id']
|
||||
body = self.get_json_param_dict()
|
||||
|
||||
# update_fields represents which fields will be updated by this request.
|
||||
fields_to_update = body.get('update_fields', [])
|
||||
# Check if valid ID is provided and fetch feature if it exists.
|
||||
if 'id' not in body['feature_changes']:
|
||||
self.abort(400, msg='Missing feature ID in feature updates')
|
||||
feature_id = body['feature_changes']['id']
|
||||
feature: FeatureEntry | None = FeatureEntry.get_by_id(feature_id)
|
||||
if feature is None:
|
||||
self.abort(404, msg=f'Feature {feature_id} not found')
|
||||
if not feature:
|
||||
self.abort(400, msg=f'Feature not found for ID {feature_id}')
|
||||
|
||||
# Validate the user has edit permissions and redirect if needed.
|
||||
redirect_resp = permissions.validate_feature_edit_permission(
|
||||
|
|
@ -257,18 +282,9 @@ class FeaturesAPI(basehandlers.APIHandler):
|
|||
if redirect_resp:
|
||||
return redirect_resp
|
||||
|
||||
# Update each field specified in the field mask.
|
||||
for field in fields_to_update:
|
||||
# Each field specified should be a valid field that exists on the entity.
|
||||
if not hasattr(feature, field):
|
||||
self.abort(400, msg=f'FeatureEntry has no attribute {field}.')
|
||||
if field in FeatureEntry.FIELDS_IMMUTABLE_BY_USER:
|
||||
self.abort(400, f'FeatureEntry field {field} is immutable.')
|
||||
setattr(feature, field, body.get(field, None))
|
||||
has_updated = self._patch_update_stages(body['stages'])
|
||||
self._patch_update_feature(feature, body['feature_changes'], has_updated)
|
||||
|
||||
feature.updater_email = self.get_current_user().email()
|
||||
feature.updated = datetime.now()
|
||||
feature.put()
|
||||
return {'message': f'Feature {feature_id} updated.'}
|
||||
|
||||
@permissions.require_admin_site
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@
|
|||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
from datetime import datetime
|
||||
import testing_config # Must be imported before the module under test.
|
||||
|
||||
import flask
|
||||
|
|
@ -112,6 +113,7 @@ class FeaturesAPITest(testing_config.CustomTestCase):
|
|||
self.ship_stage_1 = Stage(feature_id=self.feature_1_id,
|
||||
stage_type=160, milestones=MilestoneSet(desktop_first=1))
|
||||
self.ship_stage_1.put()
|
||||
self.ship_stage_1_id = self.ship_stage_1.key.integer_id()
|
||||
|
||||
self.feature_2 = FeatureEntry(
|
||||
name='feature two', summary='sum K', feature_type=1,
|
||||
|
|
@ -407,71 +409,203 @@ class FeaturesAPITest(testing_config.CustomTestCase):
|
|||
|
||||
def test_patch__valid(self):
|
||||
"""PATCH request successful with valid input from user with permissions."""
|
||||
# Signed-in user with permissions
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
new_summary = 'a different summary'
|
||||
new_owner_emails = ['test@example.com']
|
||||
valid_request_body = {
|
||||
'update_fields': ['summary', 'owner_emails'],
|
||||
'summary': new_summary,
|
||||
'owner_emails': new_owner_emails,
|
||||
'feature_changes': {
|
||||
'id': self.feature_1_id,
|
||||
'summary': new_summary,
|
||||
'owner_emails': new_owner_emails,
|
||||
},
|
||||
'stages': [],
|
||||
}
|
||||
request_path = f'{self.request_path}/{self.feature_1_id}'
|
||||
request_path = f'{self.request_path}/update'
|
||||
with test_app.test_request_context(request_path, json=valid_request_body):
|
||||
response = self.handler.do_patch(feature_id=self.feature_1_id)
|
||||
# Success response should be returned
|
||||
response = self.handler.do_patch()
|
||||
# Success response should be returned.
|
||||
self.assertEqual({'message': f'Feature {self.feature_1_id} updated.'}, response)
|
||||
# Assert that changes were made.
|
||||
self.assertEqual(self.feature_1.summary, new_summary)
|
||||
self.assertEqual(self.feature_1.owner_emails, new_owner_emails)
|
||||
# Updater email field should be changed
|
||||
# Updater email field should be changed.
|
||||
self.assertIsNotNone(self.feature_1.updated)
|
||||
self.assertEqual(self.feature_1.updater_email, 'admin@example.com')
|
||||
|
||||
def test_patch__stage_changes(self):
|
||||
"""Valid PATCH updates stage entities."""
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
new_intent_url = 'https://example.com/intent'
|
||||
valid_request_body = {
|
||||
'feature_changes': {
|
||||
'id': self.feature_1_id,
|
||||
},
|
||||
'stages': [
|
||||
{
|
||||
'id': self.ship_stage_1_id,
|
||||
'intent_thread_url': new_intent_url,
|
||||
},
|
||||
],
|
||||
}
|
||||
request_path = f'{self.request_path}/update'
|
||||
with test_app.test_request_context(request_path, json=valid_request_body):
|
||||
response = self.handler.do_patch()
|
||||
|
||||
# Success response should be returned.
|
||||
self.assertEqual(
|
||||
{'message': f'Feature {self.feature_1_id} updated.'}, response)
|
||||
# Assert that changes were made.
|
||||
self.assertEqual(self.ship_stage_1.intent_thread_url, new_intent_url)
|
||||
# Updater email field should be changed even with only stage changes.
|
||||
self.assertIsNotNone(self.feature_1.updated)
|
||||
self.assertEqual(self.feature_1.updater_email, 'admin@example.com')
|
||||
|
||||
def test_patch__milestone_changes(self):
|
||||
"""Valid PATCH updates milestone fields on stage entities."""
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
new_desktop_first = 100
|
||||
valid_request_body = {
|
||||
'feature_changes': {
|
||||
'id': self.feature_1_id,
|
||||
},
|
||||
'stages': [
|
||||
{
|
||||
'id': self.ship_stage_1_id,
|
||||
'desktop_first': new_desktop_first,
|
||||
},
|
||||
],
|
||||
}
|
||||
request_path = f'{self.request_path}/update'
|
||||
with test_app.test_request_context(request_path, json=valid_request_body):
|
||||
response = self.handler.do_patch()
|
||||
|
||||
# Success response should be returned.
|
||||
self.assertEqual(
|
||||
{'message': f'Feature {self.feature_1_id} updated.'}, response)
|
||||
# Assert that changes were made.
|
||||
self.assertIsNotNone(self.ship_stage_1.milestones)
|
||||
self.assertEqual(
|
||||
self.ship_stage_1.milestones.desktop_first, new_desktop_first)
|
||||
# Updater email field should be changed.
|
||||
self.assertIsNotNone(self.feature_1.updated)
|
||||
self.assertEqual(self.feature_1.updater_email, 'admin@example.com')
|
||||
|
||||
def test_patch__milestone_changes_null(self):
|
||||
"""Valid PATCH updates milestone fields when milestones object is null."""
|
||||
self.ship_stage_1.milestones = None
|
||||
self.ship_stage_1.put()
|
||||
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
new_desktop_first = 100
|
||||
valid_request_body = {
|
||||
'feature_changes': {
|
||||
'id': self.feature_1_id,
|
||||
},
|
||||
'stages': [
|
||||
{
|
||||
'id': self.ship_stage_1_id,
|
||||
'desktop_first': new_desktop_first,
|
||||
},
|
||||
],
|
||||
}
|
||||
request_path = f'{self.request_path}/update'
|
||||
with test_app.test_request_context(request_path, json=valid_request_body):
|
||||
response = self.handler.do_patch()
|
||||
|
||||
# Success response should be returned.
|
||||
self.assertEqual(
|
||||
{'message': f'Feature {self.feature_1_id} updated.'}, response)
|
||||
# Assert that changes were made.
|
||||
self.assertIsNotNone(self.ship_stage_1.milestones)
|
||||
self.assertEqual(
|
||||
self.ship_stage_1.milestones.desktop_first, new_desktop_first)
|
||||
# Updater email field should be changed even with only stage changes.
|
||||
self.assertIsNotNone(self.feature_1.updated)
|
||||
self.assertEqual(self.feature_1.updater_email, 'admin@example.com')
|
||||
|
||||
def test_patch__no_permissions(self):
|
||||
"""We give 403 if the user does not have feature edit access."""
|
||||
testing_config.sign_in('someuser@example.com', 123567890)
|
||||
|
||||
request_body = {
|
||||
'feature_changes': {
|
||||
'id': self.feature_1_id,
|
||||
},
|
||||
'stages': [],
|
||||
}
|
||||
request_path = f'{self.request_path}/{self.feature_1_id}'
|
||||
with test_app.test_request_context(request_path, json={}):
|
||||
with test_app.test_request_context(request_path, json=request_body):
|
||||
with self.assertRaises(werkzeug.exceptions.Forbidden):
|
||||
self.handler.do_patch(feature_id=self.feature_1_id)
|
||||
|
||||
def test_patch__invalid_fields(self):
|
||||
"""PATCH request fails with 400 when supplying invalid fields."""
|
||||
# Signed-in user with permissions
|
||||
"""PATCH request does not attempt to update invalid fields."""
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
bad_param = 'Not a real field'
|
||||
new_owner_emails = ['test@example.com']
|
||||
invalid_request_body = {
|
||||
'update_fields': ['bad_param', 'owner_emails'],
|
||||
'bad_param': bad_param,
|
||||
'owner_emails': new_owner_emails,
|
||||
'feature_changes': {
|
||||
'id': self.feature_1_id,
|
||||
'bad_param': bad_param,
|
||||
},
|
||||
'stages': [
|
||||
{
|
||||
'id': self.ship_stage_1_id,
|
||||
'bad_param': bad_param,
|
||||
},
|
||||
],
|
||||
}
|
||||
request_path = f'{self.request_path}/{self.feature_1_id}'
|
||||
request_path = f'{self.request_path}/update'
|
||||
with test_app.test_request_context(request_path, json=invalid_request_body):
|
||||
with self.assertRaises(werkzeug.exceptions.BadRequest):
|
||||
self.handler.do_patch(feature_id=self.feature_1_id)
|
||||
self.handler.do_patch(feature_id=self.feature_1_id)
|
||||
# Updater email field should NOT be changed. No changes were made.
|
||||
self.assertIsNone(self.feature_1.updater_email)
|
||||
|
||||
def test_patch__immutable_fields(self):
|
||||
"""PATCH request fails with 400 when immutable field change is attempted."""
|
||||
# Signed-in user with permissions
|
||||
def test_patch__accurate_as_of(self):
|
||||
"""Updates accurate_as_of for accuracy verification request."""
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
new_creator = 'differentuser@example.com'
|
||||
invalid_request_body = {
|
||||
'update_fields': ['creator_email'],
|
||||
'creator_email': new_creator,
|
||||
# Add some outstanding notifications beforehand.
|
||||
self.feature_1.outstanding_notifications = 2
|
||||
old_accuracy_date = datetime(2020, 1, 1)
|
||||
self.feature_1.accurate_as_of = old_accuracy_date
|
||||
self.feature_1.put()
|
||||
|
||||
valid_request_body = {
|
||||
'feature_changes': {
|
||||
'id': self.feature_1_id,
|
||||
'accurate_as_of': True,
|
||||
},
|
||||
'stages': [
|
||||
{
|
||||
'id': self.ship_stage_1_id,
|
||||
'desktop_first': 115
|
||||
},
|
||||
],
|
||||
}
|
||||
request_path = f'{self.request_path}/{self.feature_1_id}'
|
||||
with test_app.test_request_context(request_path, json=invalid_request_body):
|
||||
with self.assertRaises(werkzeug.exceptions.BadRequest):
|
||||
self.handler.do_patch(feature_id=self.feature_1_id)
|
||||
request_path = f'{self.request_path}/update'
|
||||
with test_app.test_request_context(request_path, json=valid_request_body):
|
||||
self.handler.do_patch(feature_id=self.feature_1_id)
|
||||
# Assert that changes were made.
|
||||
self.assertIsNotNone(self.feature_1.accurate_as_of)
|
||||
self.assertTrue(self.feature_1.accurate_as_of > old_accuracy_date)
|
||||
self.assertEqual(self.feature_1.outstanding_notifications, 0)
|
||||
# Updater email field should be changed.
|
||||
self.assertIsNotNone(self.feature_1.updated)
|
||||
self.assertEqual(self.feature_1.updater_email, 'admin@example.com')
|
||||
|
||||
def test_post__valid(self):
|
||||
"""POST request successful with valid input from user with permissions."""
|
||||
# Signed-in user with permissions
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
valid_request_body = {
|
||||
|
|
@ -507,7 +641,7 @@ class FeaturesAPITest(testing_config.CustomTestCase):
|
|||
|
||||
def test_post__valid_stage_and_gate_creation(self):
|
||||
"""POST request successful with valid input from user with permissions."""
|
||||
# Signed-in user with permissions
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
valid_request_body = {
|
||||
|
|
@ -552,10 +686,10 @@ class FeaturesAPITest(testing_config.CustomTestCase):
|
|||
|
||||
def test_post__invalid_fields(self):
|
||||
"""POST request fails with 400 when supplying invalid fields."""
|
||||
# Signed-in user with permissions
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
invalid_request_body = {
|
||||
request_body = {
|
||||
'bad_param': 'Not a real field', # Bad field.
|
||||
|
||||
'name': 'A name',
|
||||
|
|
@ -570,16 +704,29 @@ class FeaturesAPITest(testing_config.CustomTestCase):
|
|||
'web_dev_views': 1,
|
||||
}
|
||||
request_path = f'{self.request_path}/create'
|
||||
with test_app.test_request_context(request_path, json=invalid_request_body):
|
||||
with self.assertRaises(werkzeug.exceptions.BadRequest):
|
||||
self.handler.do_post()
|
||||
with test_app.test_request_context(request_path, json=request_body):
|
||||
response = self.handler.do_post()
|
||||
# A new feature ID should be returned.
|
||||
self.assertIsNotNone(response['feature_id'])
|
||||
self.assertTrue(type(response['feature_id']) == int)
|
||||
# New feature should exist.
|
||||
new_feature: FeatureEntry | None = (
|
||||
FeatureEntry.get_by_id(response['feature_id']))
|
||||
self.assertIsNotNone(new_feature)
|
||||
|
||||
# New feature's values should match fields in JSON body (except bad param).
|
||||
for field, value in request_body.items():
|
||||
# Invalid fields are ignored and not updated.
|
||||
if field == 'bad_param':
|
||||
continue
|
||||
self.assertEqual(getattr(new_feature, field), value)
|
||||
|
||||
def test_post__immutable_fields(self):
|
||||
"""POST request fails with 400 when immutable field is provided."""
|
||||
# Signed-in user with permissions
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
invalid_request_body = {
|
||||
request_body = {
|
||||
'creator_email': 'differentuser@example.com', # Immutable.
|
||||
|
||||
'name': 'A name',
|
||||
|
|
@ -594,13 +741,29 @@ class FeaturesAPITest(testing_config.CustomTestCase):
|
|||
'web_dev_views': 1,
|
||||
}
|
||||
request_path = f'{self.request_path}/create'
|
||||
with test_app.test_request_context(request_path, json=invalid_request_body):
|
||||
with self.assertRaises(werkzeug.exceptions.BadRequest):
|
||||
self.handler.do_post()
|
||||
with test_app.test_request_context(request_path, json=request_body):
|
||||
response = self.handler.do_post()
|
||||
# A new feature ID should be returned.
|
||||
self.assertIsNotNone(response['feature_id'])
|
||||
self.assertTrue(type(response['feature_id']) == int)
|
||||
# New feature should exist.
|
||||
new_feature: FeatureEntry | None = (
|
||||
FeatureEntry.get_by_id(response['feature_id']))
|
||||
self.assertIsNotNone(new_feature)
|
||||
|
||||
# New feature's values should match fields in JSON body
|
||||
# (except immutable field).
|
||||
for field, value in request_body.items():
|
||||
if field == 'creator_email':
|
||||
# User's email should match creator_email field.
|
||||
# The given creator_email should be ignored.
|
||||
self.assertEqual(new_feature.creator_email, 'admin@example.com')
|
||||
else:
|
||||
self.assertEqual(getattr(new_feature, field), value)
|
||||
|
||||
def test_post__bad_data_type_int(self):
|
||||
"""POST request fails with 400 when a bad int data type is provided."""
|
||||
# Signed-in user with permissions
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
invalid_request_body = {
|
||||
|
|
@ -622,7 +785,7 @@ class FeaturesAPITest(testing_config.CustomTestCase):
|
|||
|
||||
def test_post__bad_data_type_list(self):
|
||||
"""POST request fails with 400 when a bad list data type is provided."""
|
||||
# Signed-in user with permissions
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
invalid_request_body = {
|
||||
|
|
@ -644,7 +807,7 @@ class FeaturesAPITest(testing_config.CustomTestCase):
|
|||
|
||||
def test_post__missing_required_field(self):
|
||||
"""POST request fails with 400 when missing required fields."""
|
||||
# Signed-in user with permissions
|
||||
# Signed-in user with permissions.
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
invalid_request_body = {
|
||||
|
|
|
|||
|
|
@ -278,10 +278,10 @@ export function clearURLParams(key) {
|
|||
|
||||
/**
|
||||
* @typedef {Object} UpdateSubmitBody
|
||||
* @property {Object.<string, *>} featureChanges An object with feature changes.
|
||||
* @property {Object.<string, *>} feature_changes An object with feature changes.
|
||||
* key=field name, value=new field value.
|
||||
* @property {Array.<Object>} stages The list of changes to specific stages.
|
||||
* @property {boolean} hasChanges Whether any valid changes are present for submission.
|
||||
* @property {boolean} has_changes Whether any valid changes are present for submission.
|
||||
*/
|
||||
|
||||
/**
|
||||
|
|
@ -326,8 +326,8 @@ export function formatFeatureChanges(fieldValues, featureId) {
|
|||
}
|
||||
|
||||
return {
|
||||
featureChanges,
|
||||
feature_changes: featureChanges,
|
||||
stages: Object.values(stages),
|
||||
hasChanges,
|
||||
has_changes: hasChanges,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -116,9 +116,9 @@ go/this-is-a-test
|
|||
},
|
||||
];
|
||||
const expected = {
|
||||
featureChanges: {id: 1},
|
||||
feature_changes: {id: 1},
|
||||
stages: [],
|
||||
hasChanges: false,
|
||||
has_changes: false,
|
||||
};
|
||||
assert.deepEqual(formatFeatureChanges(testFieldValues, featureId), expected);
|
||||
});
|
||||
|
|
@ -133,12 +133,12 @@ go/this-is-a-test
|
|||
},
|
||||
];
|
||||
const expected = {
|
||||
featureChanges: {
|
||||
feature_changes: {
|
||||
id: 1,
|
||||
example_field: '123',
|
||||
},
|
||||
stages: [],
|
||||
hasChanges: true,
|
||||
has_changes: true,
|
||||
};
|
||||
assert.deepEqual(formatFeatureChanges(testFieldValues, featureId), expected);
|
||||
});
|
||||
|
|
@ -153,11 +153,11 @@ go/this-is-a-test
|
|||
},
|
||||
];
|
||||
const expected = {
|
||||
featureChanges: {id: 1},
|
||||
feature_changes: {id: 1},
|
||||
stages: [
|
||||
{id: 1, example_field: '123'},
|
||||
],
|
||||
hasChanges: true,
|
||||
has_changes: true,
|
||||
};
|
||||
assert.deepEqual(formatFeatureChanges(testFieldValues, featureId), expected);
|
||||
});
|
||||
|
|
@ -172,9 +172,9 @@ go/this-is-a-test
|
|||
},
|
||||
];
|
||||
const expected = {
|
||||
featureChanges: {id: 1, implicit_value_field: 123},
|
||||
feature_changes: {id: 1, implicit_value_field: 123},
|
||||
stages: [],
|
||||
hasChanges: true,
|
||||
has_changes: true,
|
||||
};
|
||||
assert.deepEqual(formatFeatureChanges(testFieldValues, featureId), expected);
|
||||
});
|
||||
|
|
@ -189,9 +189,9 @@ go/this-is-a-test
|
|||
},
|
||||
];
|
||||
const expected = {
|
||||
featureChanges: {id: 1},
|
||||
feature_changes: {id: 1},
|
||||
stages: [],
|
||||
hasChanges: false,
|
||||
has_changes: false,
|
||||
};
|
||||
assert.deepEqual(formatFeatureChanges(testFieldValues, featureId), expected);
|
||||
});
|
||||
|
|
@ -227,12 +227,12 @@ go/this-is-a-test
|
|||
},
|
||||
];
|
||||
const expected = {
|
||||
featureChanges: {id: 1, example_field1: '123'},
|
||||
feature_changes: {id: 1, example_field1: '123'},
|
||||
stages: [
|
||||
{id: 1, example_field2: '456'},
|
||||
{id: 2, example_field3: '789'},
|
||||
],
|
||||
hasChanges: true,
|
||||
has_changes: true,
|
||||
};
|
||||
assert.deepEqual(formatFeatureChanges(testFieldValues, featureId), expected);
|
||||
});
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче