Use FeatureEntry for most client-side use cases (#2433)
* Use FeatureEntry entity for most client-side uses * leave notify and amendments to Feature for now * handle redis keys
This commit is contained in:
Daniel Smith
GitHub
Родитель
5f4c89657e
Коммит
e2ce1d405e
|
|
@ -32,16 +32,16 @@ NOW = datetime.datetime.now()
|
|||
class ApprovalsAPITest(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.feature_1 = core_models.Feature(
|
||||
self.feature_1 = core_models.FeatureEntry(
|
||||
name='feature one', summary='sum', category=1)
|
||||
self.feature_1.put()
|
||||
self.feature_id = self.feature_1.key.integer_id()
|
||||
|
||||
self.gate_1 = Gate(id=1, feature_id=self.feature_id, stage_id=1,
|
||||
gate_type=1, state=Approval.NA)
|
||||
gate_type=1, state=Vote.NA)
|
||||
self.gate_1.put()
|
||||
self.gate_2 = Gate(id=2, feature_id=self.feature_id, stage_id=2,
|
||||
gate_type=2, state=Approval.NA)
|
||||
gate_type=2, state=Vote.NA)
|
||||
self.gate_2.put()
|
||||
|
||||
self.handler = approvals_api.ApprovalsAPI()
|
||||
|
|
@ -244,7 +244,7 @@ class ApprovalsAPITest(testing_config.CustomTestCase):
|
|||
class ApprovalConfigsAPITest(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.feature_1 = core_models.Feature(
|
||||
self.feature_1 = core_models.FeatureEntry(
|
||||
name='feature one', summary='sum', category=1)
|
||||
self.feature_1.put()
|
||||
self.feature_1_id = self.feature_1.key.integer_id()
|
||||
|
|
@ -253,7 +253,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
|
|||
owners=['one_a@example.com', 'one_b@example.com'])
|
||||
self.config_1.put()
|
||||
|
||||
self.feature_2 = core_models.Feature(
|
||||
self.feature_2 = core_models.FeatureEntry(
|
||||
name='feature two', summary='sum', category=1)
|
||||
self.feature_2.put()
|
||||
self.feature_2_id = self.feature_2.key.integer_id()
|
||||
|
|
@ -262,7 +262,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
|
|||
owners=['two_a@example.com', 'two_b@example.com'])
|
||||
self.config_2.put()
|
||||
|
||||
self.feature_3 = core_models.Feature(
|
||||
self.feature_3 = core_models.FeatureEntry(
|
||||
name='feature three', summary='sum', category=1)
|
||||
self.feature_3.put()
|
||||
self.feature_3_id = self.feature_3.key.integer_id()
|
||||
|
|
|
|||
|
|
@ -77,7 +77,7 @@ class CommentsConvertersTest(testing_config.CustomTestCase):
|
|||
class CommentsAPITest(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.feature_1 = core_models.Feature(
|
||||
self.feature_1 = core_models.FeatureEntry(
|
||||
name='feature one', summary='sum', category=1)
|
||||
self.feature_1.put()
|
||||
self.feature_id = self.feature_1.key.integer_id()
|
||||
|
|
|
|||
|
|
@ -290,6 +290,8 @@ def feature_entry_to_json_verbose(fe: FeatureEntry) -> dict[str, Any]:
|
|||
stages['dev_trial'], 'ios_first', True)
|
||||
d['dt_milestone_webview_start'] = _stage_attr(
|
||||
stages['dev_trial'], 'webview_first', True)
|
||||
d['ready_for_trial_url'] = _stage_attr(
|
||||
stages['dev_trial'], 'announcement_url')
|
||||
|
||||
# Origin trial stage fields.
|
||||
d['ot_milestone_desktop_start'] = _stage_attr(
|
||||
|
|
@ -320,6 +322,7 @@ def feature_entry_to_json_verbose(fe: FeatureEntry) -> dict[str, Any]:
|
|||
|
||||
# Ship stage fields.
|
||||
d['intent_to_ship_url'] = _stage_attr(stages['ship'], 'intent_thread_url')
|
||||
d['finch_url'] = _stage_attr(stages['ship'], 'finch_url')
|
||||
|
||||
impl_status_chrome = d.pop('impl_status_chrome', None)
|
||||
standard_maturity = d.pop('standard_maturity', None)
|
||||
|
|
@ -427,3 +430,62 @@ def feature_entry_to_json_verbose(fe: FeatureEntry) -> dict[str, Any]:
|
|||
|
||||
del_none(d) # Further prune response by removing null/[] values.
|
||||
return d
|
||||
|
||||
def feature_entry_to_json_basic(fe: FeatureEntry) -> dict[str, Any]:
|
||||
"""Returns a dictionary with basic info about a feature."""
|
||||
# Return an empty dictionary if the entity has not been saved to datastore.
|
||||
if not fe.key:
|
||||
return {}
|
||||
|
||||
return {
|
||||
'id': fe.key.integer_id(),
|
||||
'name': fe.name,
|
||||
'summary': fe.summary,
|
||||
'unlisted': fe.unlisted,
|
||||
'blink_components': fe.blink_components or [],
|
||||
'browsers': {
|
||||
'chrome': {
|
||||
'bug': fe.bug_url,
|
||||
'blink_components': fe.blink_components or [],
|
||||
'devrel': fe.devrel_emails or [],
|
||||
'owners': fe.owner_emails or [],
|
||||
'origintrial': fe.impl_status_chrome == ORIGIN_TRIAL,
|
||||
'intervention': fe.impl_status_chrome == INTERVENTION,
|
||||
'prefixed': fe.prefixed,
|
||||
'flag': fe.impl_status_chrome == BEHIND_A_FLAG,
|
||||
'status': {
|
||||
'text': IMPLEMENTATION_STATUS[fe.impl_status_chrome],
|
||||
'val': fe.impl_status_chrome
|
||||
}
|
||||
},
|
||||
'ff': {
|
||||
'view': {
|
||||
'text': VENDOR_VIEWS[fe.ff_views],
|
||||
'val': fe.ff_views,
|
||||
'url': fe.ff_views_link,
|
||||
'notes': fe.ff_views_notes,
|
||||
}
|
||||
},
|
||||
'safari': {
|
||||
'view': {
|
||||
'text': VENDOR_VIEWS[fe.safari_views],
|
||||
'val': fe.safari_views,
|
||||
'url': fe.safari_views_link,
|
||||
'notes': fe.safari_views_notes,
|
||||
}
|
||||
},
|
||||
'webdev': {
|
||||
'view': {
|
||||
'text': WEB_DEV_VIEWS[fe.web_dev_views],
|
||||
'val': fe.web_dev_views,
|
||||
'url': fe.web_dev_views_link,
|
||||
'notes': fe.web_dev_views_notes,
|
||||
}
|
||||
},
|
||||
'other': {
|
||||
'view': {
|
||||
'notes': fe.other_views_notes,
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -85,7 +85,7 @@ class FeaturesAPI(basehandlers.APIHandler):
|
|||
return {'message': 'ID does not match any feature.'}
|
||||
feature.deleted = True
|
||||
feature.put()
|
||||
rediscache.delete_keys_with_prefix(Feature.feature_cache_prefix())
|
||||
rediscache.delete_keys_with_prefix(FeatureEntry.feature_cache_prefix())
|
||||
|
||||
# Write for new FeatureEntry entity.
|
||||
feature_entry: Optional[FeatureEntry] = (
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ import werkzeug.exceptions # Flask HTTP stuff.
|
|||
|
||||
from api import features_api
|
||||
from internals import core_enums
|
||||
from internals import core_models
|
||||
from internals.core_models import Feature, FeatureEntry, MilestoneSet, Stage
|
||||
from internals import user_models
|
||||
from framework import rediscache
|
||||
|
||||
|
|
@ -30,7 +30,7 @@ test_app = flask.Flask(__name__)
|
|||
class FeaturesAPITestDelete(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.feature_1 = core_models.Feature(
|
||||
self.feature_1 = FeatureEntry(
|
||||
name='feature one', summary='sum', category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT)
|
||||
self.feature_1.put()
|
||||
|
|
@ -45,7 +45,7 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
|
|||
|
||||
def tearDown(self):
|
||||
cache_key = '%s|%s' % (
|
||||
core_models.Feature.DEFAULT_CACHE_KEY, self.feature_1.key.integer_id())
|
||||
FeatureEntry.DEFAULT_CACHE_KEY, self.feature_1.key.integer_id())
|
||||
self.feature_1.key.delete()
|
||||
self.app_admin.key.delete()
|
||||
testing_config.sign_out()
|
||||
|
|
@ -59,7 +59,7 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
|
|||
actual_json = self.handler.do_delete(feature_id=self.feature_id)
|
||||
self.assertEqual({'message': 'Done'}, actual_json)
|
||||
|
||||
revised_feature = core_models.Feature.get_by_id(self.feature_id)
|
||||
revised_feature = FeatureEntry.get_by_id(self.feature_id)
|
||||
self.assertTrue(revised_feature.deleted)
|
||||
|
||||
def test_delete__forbidden(self):
|
||||
|
|
@ -70,7 +70,7 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
|
|||
with self.assertRaises(werkzeug.exceptions.Forbidden):
|
||||
self.handler.do_delete(feature_id=self.feature_id)
|
||||
|
||||
revised_feature = core_models.Feature.get_by_id(self.feature_id)
|
||||
revised_feature = FeatureEntry.get_by_id(self.feature_id)
|
||||
self.assertFalse(revised_feature.deleted)
|
||||
|
||||
def test_delete__invalid(self):
|
||||
|
|
@ -81,7 +81,7 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
|
|||
with self.assertRaises(werkzeug.exceptions.BadRequest):
|
||||
self.handler.do_delete()
|
||||
|
||||
revised_feature = core_models.Feature.get_by_id(self.feature_id)
|
||||
revised_feature = FeatureEntry.get_by_id(self.feature_id)
|
||||
self.assertFalse(revised_feature.deleted)
|
||||
|
||||
def test_delete__not_found(self):
|
||||
|
|
@ -92,53 +92,57 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
|
|||
with self.assertRaises(werkzeug.exceptions.NotFound):
|
||||
self.handler.do_delete(feature_id=self.feature_id + 1)
|
||||
|
||||
revised_feature = core_models.Feature.get_by_id(self.feature_id)
|
||||
revised_feature = FeatureEntry.get_by_id(self.feature_id)
|
||||
self.assertFalse(revised_feature.deleted)
|
||||
|
||||
|
||||
class FeaturesAPITestGet_NewSchema(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.feature_1 = core_models.Feature(
|
||||
name='feature one', summary='sum Z',
|
||||
owner=['feature_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT)
|
||||
self.feature_1.put()
|
||||
self.feature_1_id = self.feature_1.key.integer_id()
|
||||
self.fe_1 = core_models.FeatureEntry(
|
||||
id=self.feature_1_id,
|
||||
self.feature_1 = FeatureEntry(
|
||||
name='feature one', summary='sum Z',
|
||||
owner_emails=['feature_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT)
|
||||
self.fe_1.put()
|
||||
self.feature_1.put()
|
||||
self.feature_1_id = self.feature_1.key.integer_id()
|
||||
|
||||
self.feature_2 = core_models.Feature(
|
||||
name='feature two', summary='sum K',
|
||||
owner=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT)
|
||||
self.feature_2.put()
|
||||
self.feature_2_id = self.feature_2.key.integer_id()
|
||||
self.fe_2 = core_models.FeatureEntry(
|
||||
id=self.feature_2_id,
|
||||
self.feature_2 = FeatureEntry(
|
||||
name='feature two', summary='sum K',
|
||||
owner_emails=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT)
|
||||
self.fe_2.put()
|
||||
self.feature_2.put()
|
||||
self.feature_2_id = self.feature_2.key.integer_id()
|
||||
|
||||
self.feature_3 = core_models.Feature(
|
||||
self.feature_3 = FeatureEntry(
|
||||
name='feature three', summary='sum A',
|
||||
owner=['other_owner@example.com'], category=1,
|
||||
owner_emails=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT,
|
||||
unlisted=True)
|
||||
self.feature_3.put()
|
||||
self.feature_3_id = self.feature_3.key.integer_id()
|
||||
self.fe_3 = core_models.FeatureEntry(
|
||||
id=self.feature_3_id,
|
||||
|
||||
# Feature entities for testing legacy functions.
|
||||
self.legacy_feature_1 = Feature(
|
||||
name='feature one', summary='sum Z',
|
||||
owner=['feature_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT)
|
||||
self.legacy_feature_1.put()
|
||||
self.legacy_feature_1_id = self.feature_1.key.integer_id()
|
||||
|
||||
self.legacy_feature_2 = Feature(
|
||||
name='feature two', summary='sum K',
|
||||
owner=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT)
|
||||
self.legacy_feature_2.put()
|
||||
self.legacy_feature_2_id = self.feature_2.key.integer_id()
|
||||
|
||||
self.legacy_feature_3 = Feature(
|
||||
name='feature three', summary='sum A',
|
||||
owner_emails=['other_owner@example.com'], category=1,
|
||||
owner=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT,
|
||||
unlisted=True)
|
||||
self.fe_3.put()
|
||||
self.legacy_feature_3.put()
|
||||
self.legacy_feature_3_id = self.feature_3.key.integer_id()
|
||||
|
||||
self.request_path = '/api/v0/features'
|
||||
self.handler = features_api.FeaturesAPI()
|
||||
|
|
@ -148,15 +152,13 @@ class FeaturesAPITestGet_NewSchema(testing_config.CustomTestCase):
|
|||
self.app_admin.put()
|
||||
|
||||
def tearDown(self):
|
||||
self.feature_1.key.delete()
|
||||
self.feature_2.key.delete()
|
||||
self.feature_3.key.delete()
|
||||
self.fe_1.key.delete()
|
||||
self.fe_2.key.delete()
|
||||
self.fe_3.key.delete()
|
||||
self.app_admin.key.delete()
|
||||
for kind in [Feature, FeatureEntry, user_models.AppUser]:
|
||||
for entity in kind.query():
|
||||
entity.key.delete()
|
||||
|
||||
testing_config.sign_out()
|
||||
rediscache.delete_keys_with_prefix('features|*')
|
||||
rediscache.delete_keys_with_prefix('FeatureEntries|*')
|
||||
|
||||
def test_get__all_listed(self):
|
||||
"""Get all features that are listed."""
|
||||
|
|
@ -239,8 +241,8 @@ class FeaturesAPITestGet_NewSchema(testing_config.CustomTestCase):
|
|||
|
||||
def test_get__all_unlisted_no_perms(self):
|
||||
"""JSON feed does not include unlisted features for users who can't edit."""
|
||||
self.fe_1.unlisted = True
|
||||
self.fe_1.put()
|
||||
self.feature_1.unlisted = True
|
||||
self.feature_1.put()
|
||||
|
||||
# No signed-in user
|
||||
with test_app.test_request_context(self.request_path):
|
||||
|
|
@ -259,8 +261,8 @@ class FeaturesAPITestGet_NewSchema(testing_config.CustomTestCase):
|
|||
|
||||
def test_get__all_unlisted_can_edit(self):
|
||||
"""JSON feed includes unlisted features for users who may edit."""
|
||||
self.fe_1.unlisted = True
|
||||
self.fe_1.put()
|
||||
self.feature_1.unlisted = True
|
||||
self.feature_1.put()
|
||||
|
||||
# Signed-in user with permissions
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
|
@ -318,27 +320,58 @@ class FeaturesAPITestGet_NewSchema(testing_config.CustomTestCase):
|
|||
class FeaturesAPITestGet_OldSchema(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.feature_1 = core_models.Feature(
|
||||
self.feature_1 = FeatureEntry(
|
||||
name='feature one', summary='sum Z',
|
||||
owner=['feature_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT, shipped_milestone=1)
|
||||
owner_emails=['feature_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT, feature_type=0)
|
||||
self.feature_1.put()
|
||||
self.feature_1_id = self.feature_1.key.integer_id()
|
||||
|
||||
self.feature_2 = core_models.Feature(
|
||||
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.feature_2 = FeatureEntry(
|
||||
name='feature two', summary='sum K',
|
||||
owner=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT, shipped_milestone=2)
|
||||
owner_emails=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT, feature_type=1)
|
||||
self.feature_2.put()
|
||||
self.feature_2_id = self.feature_2.key.integer_id()
|
||||
self.ship_stage_2 = Stage(feature_id=self.feature_1_id,
|
||||
stage_type=260, milestones=MilestoneSet(desktop_first=2))
|
||||
|
||||
self.feature_3 = core_models.Feature(
|
||||
self.feature_3 = FeatureEntry(
|
||||
name='feature three', summary='sum A',
|
||||
owner=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT, shipped_milestone=2,
|
||||
owner_emails=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT, feature_type=2,
|
||||
unlisted=True)
|
||||
self.feature_3.put()
|
||||
self.feature_3_id = self.feature_3.key.integer_id()
|
||||
self.ship_stage_3 = Stage(feature_id=self.feature_1_id,
|
||||
stage_type=360, milestones=MilestoneSet(desktop_first=2))
|
||||
|
||||
# Feature entities for testing legacy functions.
|
||||
self.legacy_feature_1 = Feature(
|
||||
name='feature one', summary='sum Z',
|
||||
owner=['feature_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT,
|
||||
shipped_milestone=1)
|
||||
self.legacy_feature_1.put()
|
||||
self.legacy_feature_1_id = self.feature_1.key.integer_id()
|
||||
|
||||
self.legacy_feature_2 = Feature(
|
||||
name='feature two', summary='sum K',
|
||||
owner=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT,
|
||||
shipped_milestone=2)
|
||||
self.legacy_feature_2.put()
|
||||
self.legacy_feature_2_id = self.feature_2.key.integer_id()
|
||||
|
||||
self.legacy_feature_3 = Feature(
|
||||
name='feature three', summary='sum A',
|
||||
owner=['other_owner@example.com'], category=1,
|
||||
intent_stage=core_enums.INTENT_IMPLEMENT,
|
||||
shipped_milestone=2, unlisted=True)
|
||||
self.legacy_feature_3.put()
|
||||
self.legacy_feature_3_id = self.feature_3.key.integer_id()
|
||||
|
||||
self.request_path = '/api/v0/features'
|
||||
self.handler = features_api.FeaturesAPI()
|
||||
|
|
@ -348,12 +381,12 @@ class FeaturesAPITestGet_OldSchema(testing_config.CustomTestCase):
|
|||
self.app_admin.put()
|
||||
|
||||
def tearDown(self):
|
||||
self.feature_1.key.delete()
|
||||
self.feature_2.key.delete()
|
||||
self.feature_3.key.delete()
|
||||
self.app_admin.key.delete()
|
||||
for kind in [Feature, FeatureEntry, Stage, user_models.AppUser]:
|
||||
for entity in kind.query():
|
||||
entity.key.delete()
|
||||
testing_config.sign_out()
|
||||
rediscache.delete_keys_with_prefix('features|*')
|
||||
rediscache.delete_keys_with_prefix('FeatureEntries|*')
|
||||
|
||||
def test_get__in_milestone_listed(self):
|
||||
"""Get all features in a specific milestone that are listed."""
|
||||
|
|
@ -373,8 +406,8 @@ class FeaturesAPITestGet_OldSchema(testing_config.CustomTestCase):
|
|||
|
||||
def test_get__in_milestone_unlisted_no_perms(self):
|
||||
"""JSON feed does not include unlisted features for users who can't edit."""
|
||||
self.feature_1.unlisted = True
|
||||
self.feature_1.put()
|
||||
self.legacy_feature_1.unlisted = True
|
||||
self.legacy_feature_1.put()
|
||||
|
||||
# No signed-in user
|
||||
with test_app.test_request_context(self.request_path+'?milestone=1'):
|
||||
|
|
|
|||
|
|
@ -28,7 +28,6 @@ export function formatFeatureForEdit(feature) {
|
|||
|
||||
// from feature.standards
|
||||
spec_link: feature.standards.spec,
|
||||
standardization: feature.standards.status.val,
|
||||
standard_maturity: feature.standards.maturity.val,
|
||||
|
||||
tag_review_status: feature.tag_review_status_int,
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ from framework import users
|
|||
from framework import utils
|
||||
from framework import xsrf
|
||||
from internals import approval_defs
|
||||
from internals.core_models import Feature
|
||||
from internals.core_models import FeatureEntry
|
||||
from internals import user_models
|
||||
|
||||
from google.auth.transport import requests
|
||||
|
|
@ -117,12 +117,13 @@ class BaseHandler(flask.views.MethodView):
|
|||
self.abort(400, msg='Parameter %r was not a bool' % name)
|
||||
return val
|
||||
|
||||
def get_specified_feature(self, feature_id: Optional[int]=None) -> Feature:
|
||||
def get_specified_feature(
|
||||
self, feature_id: Optional[int]=None) -> FeatureEntry:
|
||||
"""Get the feature specified in the featureId parameter."""
|
||||
feature_id = (feature_id or
|
||||
self.get_int_param('featureId', required=True))
|
||||
# Load feature directly from NDB so as to never get a stale cached copy.
|
||||
feature = Feature.get_by_id(feature_id)
|
||||
feature = FeatureEntry.get_by_id(feature_id)
|
||||
if not feature:
|
||||
self.abort(404, msg='Feature not found')
|
||||
user = self.get_current_user()
|
||||
|
|
|
|||
|
|
@ -15,12 +15,13 @@
|
|||
|
||||
|
||||
import logging
|
||||
from typing import Optional
|
||||
import flask
|
||||
|
||||
import settings
|
||||
from framework.users import User
|
||||
from internals import feature_helpers
|
||||
from internals.core_models import Feature
|
||||
from internals.core_models import FeatureEntry
|
||||
from internals.user_models import AppUser
|
||||
|
||||
|
||||
|
|
@ -73,7 +74,7 @@ def can_edit_any_feature(user: User) -> bool:
|
|||
return app_user.is_admin or app_user.is_site_editor
|
||||
|
||||
|
||||
def feature_edit_list(user: User) -> list[Feature]:
|
||||
def feature_edit_list(user: User) -> list[int]:
|
||||
"""Return a list of features the current user can edit"""
|
||||
if not user:
|
||||
return []
|
||||
|
|
@ -100,18 +101,19 @@ def can_edit_feature(user: User, feature_id: int) -> bool:
|
|||
return False
|
||||
|
||||
# Load feature directly from NDB so as to never get a stale cached copy.
|
||||
feature = Feature.get_by_id(feature_id)
|
||||
feature: Optional[FeatureEntry] = FeatureEntry.get_by_id(feature_id)
|
||||
if not feature:
|
||||
return False
|
||||
|
||||
email = user.email()
|
||||
# Check if the user is an owner, editor, or creator for this feature.
|
||||
# If yes, the feature can be edited.
|
||||
return (email in feature.owner or
|
||||
email in feature.editors or email == feature.creator)
|
||||
return (email in feature.owner_emails or
|
||||
email in feature.editor_emails or
|
||||
email == feature.creator_email)
|
||||
|
||||
|
||||
def can_approve_feature(user: User, feature: Feature, approvers) -> bool:
|
||||
def can_approve_feature(user: User, feature: FeatureEntry, approvers) -> bool:
|
||||
"""Return True if the user is allowed to approve the given feature."""
|
||||
# TODO(jrobbins): make this per-feature
|
||||
if not can_view_feature(user, feature):
|
||||
|
|
@ -192,7 +194,7 @@ def validate_feature_edit_permission(handler_obj, feature_id: int):
|
|||
|
||||
# Redirect to 404 if feature is not found.
|
||||
# Load feature directly from NDB so as to never get a stale cached copy.
|
||||
if Feature.get_by_id(int(feature_id)) is None:
|
||||
if FeatureEntry.get_by_id(int(feature_id)) is None:
|
||||
handler_obj.abort(404, msg='Feature not found')
|
||||
|
||||
# Redirect to 403 if user does not have edit permission for feature.
|
||||
|
|
|
|||
|
|
@ -77,11 +77,11 @@ class PermissionFunctionTests(testing_config.CustomTestCase):
|
|||
self.users.append(self.feature_editor)
|
||||
|
||||
# Feature for checking permissions against
|
||||
self.feature_1 = core_models.Feature(
|
||||
self.feature_1 = core_models.FeatureEntry(
|
||||
name='feature one', summary='sum',
|
||||
creator="feature_creator@example.com",
|
||||
owner=['feature_owner@example.com'],
|
||||
editors=['feature_editor@example.com'], category=1)
|
||||
creator_email="feature_creator@example.com",
|
||||
owner_emails=['feature_owner@example.com'],
|
||||
editor_emails=['feature_editor@example.com'], category=1)
|
||||
self.feature_1.put()
|
||||
self.feature_id = self.feature_1.key.integer_id()
|
||||
|
||||
|
|
|
|||
|
|
@ -150,33 +150,6 @@ class Feature(DictModel):
|
|||
except Exception as e:
|
||||
logging.error(e)
|
||||
|
||||
def crbug_number(self) -> Optional[str]:
|
||||
if not self.bug_url:
|
||||
return None
|
||||
m = re.search(r'[\/|?id=]([0-9]+)$', self.bug_url)
|
||||
if m:
|
||||
return m.group(1)
|
||||
return None
|
||||
|
||||
def new_crbug_url(self) -> str:
|
||||
url = 'https://bugs.chromium.org/p/chromium/issues/entry'
|
||||
if len(self.blink_components) > 0:
|
||||
params = ['components=' + self.blink_components[0]]
|
||||
else:
|
||||
params = ['components=' + settings.DEFAULT_COMPONENT]
|
||||
crbug_number = self.crbug_number()
|
||||
if crbug_number and self.impl_status_chrome in (
|
||||
NO_ACTIVE_DEV,
|
||||
PROPOSED,
|
||||
IN_DEVELOPMENT,
|
||||
BEHIND_A_FLAG,
|
||||
ORIGIN_TRIAL,
|
||||
INTERVENTION):
|
||||
params.append('blocking=' + crbug_number)
|
||||
if self.owner:
|
||||
params.append('cc=' + ','.join(self.owner))
|
||||
return url + '?' + '&'.join(params)
|
||||
|
||||
def stash_values(self) -> None:
|
||||
|
||||
# Stash existing values when entity is created so we can diff property
|
||||
|
|
@ -490,74 +463,30 @@ class FeatureEntry(ndb.Model): # Copy from Feature
|
|||
def feature_cache_prefix(cls):
|
||||
return '%s|*' % (cls.DEFAULT_CACHE_KEY)
|
||||
|
||||
@classmethod
|
||||
def get_feature_entry(self, feature_id: int, update_cache: bool=False
|
||||
) -> Optional[FeatureEntry]:
|
||||
KEY = self.feature_cache_key(
|
||||
FeatureEntry.DEFAULT_CACHE_KEY, feature_id)
|
||||
feature = rediscache.get(KEY)
|
||||
def stash_values(self) -> None:
|
||||
# Stash existing values when entity is created so we can diff property
|
||||
# values later in put() to know what's changed.
|
||||
# https://stackoverflow.com/a/41344898
|
||||
|
||||
if feature is None or update_cache:
|
||||
entry = FeatureEntry.get_by_id(feature_id)
|
||||
if entry:
|
||||
if entry.deleted:
|
||||
return None
|
||||
rediscache.set(KEY, entry)
|
||||
for prop_name in self._properties.keys():
|
||||
old_val = getattr(self, prop_name, None)
|
||||
setattr(self, '_old_' + prop_name, old_val)
|
||||
setattr(self, '_values_stashed', True)
|
||||
|
||||
return entry
|
||||
def put(self, notify: bool=False, **kwargs) -> Any:
|
||||
key = super(FeatureEntry, self).put(**kwargs)
|
||||
# TODO(danielrsmith): Notifying subscribers will not fully function
|
||||
# until stage fields are also stashed and marked as changed.
|
||||
# Notifying and saving amendments is handled by Feature until then.
|
||||
#
|
||||
# notifier_helpers.notify_subscribers_and_save_amendments(self, notify)
|
||||
|
||||
@classmethod
|
||||
def filter_unlisted(self, entry_list: list[FeatureEntry]
|
||||
) -> list[FeatureEntry]:
|
||||
"""Filters feature entries to display only features the user should see."""
|
||||
user = users.get_current_user()
|
||||
email = None
|
||||
if user:
|
||||
email = user.email()
|
||||
allowed_entries = []
|
||||
for fe in entry_list:
|
||||
# Owners and editors of a feature can see their unlisted features.
|
||||
if (not fe.unlisted or
|
||||
email in fe.owners or
|
||||
email in fe.editors or
|
||||
(email is not None and fe.creator == email)):
|
||||
allowed_entries.append(fe)
|
||||
# Invalidate rediscache for the individual feature view.
|
||||
cache_key = FeatureEntry.feature_cache_key(
|
||||
FeatureEntry.DEFAULT_CACHE_KEY, self.key.integer_id())
|
||||
rediscache.delete(cache_key)
|
||||
|
||||
return allowed_entries
|
||||
|
||||
@classmethod
|
||||
def get_by_ids(self, entry_ids: list[int], update_cache: bool=False
|
||||
) -> list[int]:
|
||||
"""Return a list of FeatureEntry instances for the specified features.
|
||||
|
||||
Because the cache may rarely have stale data, this should only be
|
||||
used for displaying data read-only, not for populating forms or
|
||||
procesing a POST to edit data. For editing use case, load the
|
||||
data from NDB directly.
|
||||
"""
|
||||
result_dict: dict[int, int] = {}
|
||||
futures = []
|
||||
|
||||
for fe_id in entry_ids:
|
||||
lookup_key = self.feature_cache_key(
|
||||
FeatureEntry.DEFAULT_CACHE_KEY, fe_id)
|
||||
entry = rediscache.get(lookup_key)
|
||||
if entry is None or update_cache:
|
||||
futures.append(FeatureEntry.get_by_id_async(fe_id))
|
||||
else:
|
||||
result_dict[fe_id] = entry
|
||||
|
||||
for future in futures:
|
||||
entry = future.get_result()
|
||||
if entry and not entry.deleted:
|
||||
store_key = self.feature_cache_key(
|
||||
FeatureEntry.DEFAULT_CACHE_KEY, entry.key.integer_id())
|
||||
rediscache.set(store_key, entry)
|
||||
result_dict[entry.key.integer_id()] = entry
|
||||
|
||||
result_list = [result_dict[fe_id] for fe_id in entry_ids
|
||||
if fe_id in result_dict]
|
||||
return result_list
|
||||
return key
|
||||
|
||||
# Note: get_in_milestone will be in a new file legacy_queries.py.
|
||||
|
||||
|
|
|
|||
|
|
@ -47,31 +47,52 @@ class ModelsFunctionsTest(testing_config.CustomTestCase):
|
|||
class FeatureTest(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.feature_2 = core_models.Feature(
|
||||
name='feature b', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1)
|
||||
self.feature_2 = core_models.FeatureEntry(
|
||||
name='feature b', summary='sum',
|
||||
owner_emails=['feature_owner@example.com'], category=1)
|
||||
self.feature_2.put()
|
||||
|
||||
self.feature_1 = core_models.Feature(
|
||||
name='feature a', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, impl_status_chrome=3)
|
||||
self.feature_1 = core_models.FeatureEntry(
|
||||
name='feature a', summary='sum', impl_status_chrome=3,
|
||||
owner_emails=['feature_owner@example.com'], category=1)
|
||||
self.feature_1.put()
|
||||
|
||||
self.feature_4 = core_models.Feature(
|
||||
name='feature d', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, impl_status_chrome=2)
|
||||
self.feature_4 = core_models.FeatureEntry(
|
||||
name='feature d', summary='sum', category=1, impl_status_chrome=2,
|
||||
owner_emails=['feature_owner@example.com'])
|
||||
self.feature_4.put()
|
||||
|
||||
self.feature_3 = core_models.Feature(
|
||||
name='feature c', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, impl_status_chrome=2)
|
||||
self.feature_3 = core_models.FeatureEntry(
|
||||
name='feature c', summary='sum', category=1, impl_status_chrome=2,
|
||||
owner_emails=['feature_owner@example.com'])
|
||||
self.feature_3.put()
|
||||
|
||||
# Legacy entities for testing legacy functions.
|
||||
self.legacy_feature_2 = core_models.Feature(
|
||||
name='feature b', summary='sum',
|
||||
owner=['feature_owner@example.com'], category=1)
|
||||
self.legacy_feature_2.put()
|
||||
|
||||
self.legacy_feature_1 = core_models.Feature(
|
||||
name='feature a', summary='sum', impl_status_chrome=3,
|
||||
owner=['feature_owner@example.com'], category=1)
|
||||
self.legacy_feature_1.put()
|
||||
|
||||
self.legacy_feature_4 = core_models.Feature(
|
||||
name='feature d', summary='sum', category=1, impl_status_chrome=2,
|
||||
owner=['feature_owner@example.com'])
|
||||
self.legacy_feature_4.put()
|
||||
|
||||
self.legacy_feature_3 = core_models.Feature(
|
||||
name='feature c', summary='sum', category=1, impl_status_chrome=2,
|
||||
owner=['feature_owner@example.com'])
|
||||
self.legacy_feature_3.put()
|
||||
|
||||
def tearDown(self):
|
||||
self.feature_1.key.delete()
|
||||
self.feature_2.key.delete()
|
||||
self.feature_3.key.delete()
|
||||
self.feature_4.key.delete()
|
||||
for kind in [core_models.Feature, core_models.FeatureEntry]:
|
||||
for entity in kind.query():
|
||||
entity.key.delete()
|
||||
|
||||
rediscache.flushall()
|
||||
|
||||
def test_get_all__normal(self):
|
||||
|
|
@ -79,7 +100,7 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
actual = feature_helpers.get_all(update_cache=True)
|
||||
names = [f['name'] for f in actual]
|
||||
self.assertEqual(
|
||||
['feature c', 'feature d', 'feature a', 'feature b'],
|
||||
['feature b', 'feature a', 'feature d', 'feature c'],
|
||||
names)
|
||||
|
||||
self.feature_1.summary = 'revised summary'
|
||||
|
|
@ -87,7 +108,7 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
actual = feature_helpers.get_all(update_cache=True)
|
||||
names = [f['name'] for f in actual]
|
||||
self.assertEqual(
|
||||
['feature a', 'feature c', 'feature d', 'feature b'],
|
||||
['feature b', 'feature a', 'feature d', 'feature c'],
|
||||
names)
|
||||
|
||||
def test_get_all__category(self):
|
||||
|
|
@ -111,16 +132,16 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
def test_get_all__owner(self):
|
||||
"""We can retrieve a list of all features with a given owner."""
|
||||
actual = feature_helpers.get_all(
|
||||
filterby=('owner', 'owner@example.com'), update_cache=True)
|
||||
filterby=('owner_emails', 'owner@example.com'), update_cache=True)
|
||||
names = [f['name'] for f in actual]
|
||||
self.assertEqual(
|
||||
[],
|
||||
names)
|
||||
|
||||
self.feature_1.owner = ['owner@example.com']
|
||||
self.feature_1.owner_emails = ['owner@example.com']
|
||||
self.feature_1.put() # Changes updated field.
|
||||
actual = feature_helpers.get_all(
|
||||
filterby=('owner', 'owner@example.com'), update_cache=True)
|
||||
filterby=('owner_emails', 'owner@example.com'), update_cache=True)
|
||||
names = [f['name'] for f in actual]
|
||||
self.assertEqual(
|
||||
['feature a'],
|
||||
|
|
@ -129,26 +150,26 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
def test_get_all__owner_unlisted(self):
|
||||
"""Unlisted features should still be visible to their owners."""
|
||||
self.feature_2.unlisted = True
|
||||
self.feature_2.owner = ['feature_owner@example.com']
|
||||
self.feature_2.owner_emails = ['feature_owner@example.com']
|
||||
self.feature_2.put()
|
||||
testing_config.sign_in('feature_owner@example.com', 1234567890)
|
||||
actual = feature_helpers.get_all(update_cache=True)
|
||||
names = [f['name'] for f in actual]
|
||||
testing_config.sign_out()
|
||||
self.assertEqual(
|
||||
['feature b', 'feature c', 'feature d', 'feature a'], names)
|
||||
['feature b', 'feature a', 'feature d', 'feature c'], names)
|
||||
|
||||
def test_get_all__editor_unlisted(self):
|
||||
"""Unlisted features should still be visible to feature editors."""
|
||||
self.feature_2.unlisted = True
|
||||
self.feature_2.editors = ['feature_editor@example.com']
|
||||
self.feature_2.editor_emails = ['feature_editor@example.com']
|
||||
self.feature_2.put()
|
||||
testing_config.sign_in("feature_editor@example.com", 1234567890)
|
||||
actual = feature_helpers.get_all(update_cache=True)
|
||||
names = [f['name'] for f in actual]
|
||||
testing_config.sign_out()
|
||||
self.assertEqual(
|
||||
['feature b', 'feature c', 'feature d', 'feature a'], names)
|
||||
['feature b', 'feature a', 'feature d', 'feature c'], names)
|
||||
|
||||
def test_get_by_ids__empty(self):
|
||||
"""A request to load zero features returns zero results."""
|
||||
|
|
@ -165,9 +186,9 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
self.assertEqual('feature a', actual[0]['name'])
|
||||
self.assertEqual('feature b', actual[1]['name'])
|
||||
|
||||
lookup_key_1 = '%s|%s' % (core_models.Feature.DEFAULT_CACHE_KEY,
|
||||
lookup_key_1 = '%s|%s' % (core_models.FeatureEntry.DEFAULT_CACHE_KEY,
|
||||
self.feature_1.key.integer_id())
|
||||
lookup_key_2 = '%s|%s' % (core_models.Feature.DEFAULT_CACHE_KEY,
|
||||
lookup_key_2 = '%s|%s' % (core_models.FeatureEntry.DEFAULT_CACHE_KEY,
|
||||
self.feature_2.key.integer_id())
|
||||
self.assertEqual('feature a', rediscache.get(lookup_key_1)['name'])
|
||||
self.assertEqual('feature b', rediscache.get(lookup_key_2)['name'])
|
||||
|
|
@ -175,7 +196,7 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
def test_get_by_ids__cache_hit(self):
|
||||
"""We can load features from rediscache."""
|
||||
cache_key = '%s|%s' % (
|
||||
core_models.Feature.DEFAULT_CACHE_KEY, self.feature_1.key.integer_id())
|
||||
core_models.FeatureEntry.DEFAULT_CACHE_KEY, self.feature_1.key.integer_id())
|
||||
cached_feature = {
|
||||
'name': 'fake cached_feature',
|
||||
'id': self.feature_1.key.integer_id(),
|
||||
|
|
@ -206,9 +227,7 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
def test_get_by_ids__cached_correctly(self):
|
||||
"""We should no longer be able to trigger bug #1647."""
|
||||
# Cache one to try to trigger the bug.
|
||||
feature_helpers.get_by_ids([
|
||||
self.feature_2.key.integer_id(),
|
||||
])
|
||||
feature_helpers.get_by_ids([self.feature_2.key.integer_id()])
|
||||
|
||||
# Now do the lookup, but it would cache feature_2 at the key for feature_3.
|
||||
feature_helpers.get_by_ids([
|
||||
|
|
@ -246,8 +265,8 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
|
||||
def test_get_chronological__unlisted(self):
|
||||
"""Unlisted features are not included in the list."""
|
||||
self.feature_2.unlisted = True
|
||||
self.feature_2.put()
|
||||
self.legacy_feature_2.unlisted = True
|
||||
self.legacy_feature_2.put()
|
||||
actual = feature_helpers.get_chronological()
|
||||
names = [f['name'] for f in actual]
|
||||
self.assertEqual(
|
||||
|
|
@ -256,8 +275,8 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
|
||||
def test_get_chronological__unlisted_shown(self):
|
||||
"""Unlisted features are included for users with edit access."""
|
||||
self.feature_2.unlisted = True
|
||||
self.feature_2.put()
|
||||
self.legacy_feature_2.unlisted = True
|
||||
self.legacy_feature_2.put()
|
||||
actual = feature_helpers.get_chronological(show_unlisted=True)
|
||||
names = [f['name'] for f in actual]
|
||||
self.assertEqual(
|
||||
|
|
@ -266,21 +285,21 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
|
||||
def test_get_in_milestone__normal(self):
|
||||
"""We can retrieve a list of features."""
|
||||
self.feature_2.impl_status_chrome = 7
|
||||
self.feature_2.shipped_milestone = 1
|
||||
self.feature_2.put()
|
||||
self.legacy_feature_2.impl_status_chrome = 7
|
||||
self.legacy_feature_2.shipped_milestone = 1
|
||||
self.legacy_feature_2.put()
|
||||
|
||||
self.feature_1.impl_status_chrome = 5
|
||||
self.feature_1.shipped_milestone = 1
|
||||
self.feature_1.put()
|
||||
self.legacy_feature_1.impl_status_chrome = 5
|
||||
self.legacy_feature_1.shipped_milestone = 1
|
||||
self.legacy_feature_1.put()
|
||||
|
||||
self.feature_3.impl_status_chrome = 5
|
||||
self.feature_3.shipped_milestone = 1
|
||||
self.feature_3.put()
|
||||
self.legacy_feature_3.impl_status_chrome = 5
|
||||
self.legacy_feature_3.shipped_milestone = 1
|
||||
self.legacy_feature_3.put()
|
||||
|
||||
self.feature_4.impl_status_chrome = 7
|
||||
self.feature_4.shipped_milestone = 2
|
||||
self.feature_4.put()
|
||||
self.legacy_feature_4.impl_status_chrome = 7
|
||||
self.legacy_feature_4.shipped_milestone = 2
|
||||
self.legacy_feature_4.put()
|
||||
|
||||
actual = feature_helpers.get_in_milestone(milestone=1)
|
||||
removed = [f['name'] for f in actual['Removed']]
|
||||
|
|
@ -301,22 +320,22 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
|
||||
def test_get_in_milestone__unlisted(self):
|
||||
"""Unlisted features should not be listed for users who can't edit."""
|
||||
self.feature_2.unlisted = True
|
||||
self.feature_2.impl_status_chrome = 7
|
||||
self.feature_2.shipped_milestone = 1
|
||||
self.feature_2.put()
|
||||
self.legacy_feature_2.unlisted = True
|
||||
self.legacy_feature_2.impl_status_chrome = 7
|
||||
self.legacy_feature_2.shipped_milestone = 1
|
||||
self.legacy_feature_2.put()
|
||||
|
||||
self.feature_1.impl_status_chrome = 5
|
||||
self.feature_1.shipped_milestone = 1
|
||||
self.feature_1.put()
|
||||
self.legacy_feature_1.impl_status_chrome = 5
|
||||
self.legacy_feature_1.shipped_milestone = 1
|
||||
self.legacy_feature_1.put()
|
||||
|
||||
self.feature_3.impl_status_chrome = 5
|
||||
self.feature_3.shipped_milestone = 1
|
||||
self.feature_3.put()
|
||||
self.legacy_feature_3.impl_status_chrome = 5
|
||||
self.legacy_feature_3.shipped_milestone = 1
|
||||
self.legacy_feature_3.put()
|
||||
|
||||
self.feature_4.impl_status_chrome = 7
|
||||
self.feature_4.shipped_milestone = 2
|
||||
self.feature_4.put()
|
||||
self.legacy_feature_4.impl_status_chrome = 7
|
||||
self.legacy_feature_4.shipped_milestone = 2
|
||||
self.legacy_feature_4.put()
|
||||
|
||||
actual = feature_helpers.get_in_milestone(milestone=1)
|
||||
self.assertEqual(
|
||||
|
|
@ -325,22 +344,22 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
|
||||
def test_get_in_milestone__unlisted_shown(self):
|
||||
"""Unlisted features should be listed for users who can edit."""
|
||||
self.feature_2.unlisted = True
|
||||
self.feature_2.impl_status_chrome = 7
|
||||
self.feature_2.shipped_milestone = 1
|
||||
self.feature_2.put()
|
||||
self.legacy_feature_2.unlisted = True
|
||||
self.legacy_feature_2.impl_status_chrome = 7
|
||||
self.legacy_feature_2.shipped_milestone = 1
|
||||
self.legacy_feature_2.put()
|
||||
|
||||
self.feature_1.impl_status_chrome = 5
|
||||
self.feature_1.shipped_milestone = 1
|
||||
self.feature_1.put()
|
||||
self.legacy_feature_1.impl_status_chrome = 5
|
||||
self.legacy_feature_1.shipped_milestone = 1
|
||||
self.legacy_feature_1.put()
|
||||
|
||||
self.feature_3.impl_status_chrome = 5
|
||||
self.feature_3.shipped_milestone = 1
|
||||
self.feature_3.put()
|
||||
self.legacy_feature_3.impl_status_chrome = 5
|
||||
self.legacy_feature_3.shipped_milestone = 1
|
||||
self.legacy_feature_3.put()
|
||||
|
||||
self.feature_4.impl_status_chrome = 7
|
||||
self.feature_4.shipped_milestone = 2
|
||||
self.feature_4.put()
|
||||
self.legacy_feature_4.impl_status_chrome = 7
|
||||
self.legacy_feature_4.shipped_milestone = 2
|
||||
self.legacy_feature_4.put()
|
||||
|
||||
actual = feature_helpers.get_in_milestone(
|
||||
milestone=1, show_unlisted=True)
|
||||
|
|
|
|||
|
|
@ -14,16 +14,48 @@
|
|||
# limitations under the License.
|
||||
|
||||
import logging
|
||||
import re
|
||||
from typing import Any, Optional
|
||||
from google.cloud import ndb # type: ignore
|
||||
|
||||
from api.converters import feature_to_legacy_json
|
||||
from api import converters
|
||||
from framework import rediscache
|
||||
from framework import users
|
||||
from internals.core_enums import *
|
||||
from internals.core_models import Feature
|
||||
from internals.core_models import Feature, FeatureEntry
|
||||
import settings
|
||||
|
||||
def get_by_ids(feature_ids: list[int],
|
||||
|
||||
def _crbug_number(bug_url: Optional[str]) -> Optional[str]:
|
||||
if bug_url is None:
|
||||
return None
|
||||
m = re.search(r'[\/|?id=]([0-9]+)$', bug_url)
|
||||
if m:
|
||||
return m.group(1)
|
||||
return None
|
||||
|
||||
def _new_crbug_url(blink_components: Optional[list[str]],
|
||||
bug_url: Optional[str], impl_status_chrome: int,
|
||||
owner_emails: list[str]=list()) -> str:
|
||||
url = 'https://bugs.chromium.org/p/chromium/issues/entry'
|
||||
if blink_components and len(blink_components) > 0:
|
||||
params = ['components=' + blink_components[0]]
|
||||
else:
|
||||
params = ['components=' + settings.DEFAULT_COMPONENT]
|
||||
crbug_number = _crbug_number(bug_url)
|
||||
if crbug_number and impl_status_chrome in (
|
||||
NO_ACTIVE_DEV,
|
||||
PROPOSED,
|
||||
IN_DEVELOPMENT,
|
||||
BEHIND_A_FLAG,
|
||||
ORIGIN_TRIAL,
|
||||
INTERVENTION):
|
||||
params.append('blocking=' + crbug_number)
|
||||
if owner_emails:
|
||||
params.append('cc=' + ','.join(owner_emails))
|
||||
return url + '?' + '&'.join(params)
|
||||
|
||||
def get_by_ids_legacy(feature_ids: list[int],
|
||||
update_cache: bool=False) -> list[dict[str, Any]]:
|
||||
"""Return a list of JSON dicts for the specified features.
|
||||
|
||||
|
|
@ -47,10 +79,12 @@ def get_by_ids(feature_ids: list[int],
|
|||
for future in futures:
|
||||
unformatted_feature: Optional[Feature] = future.get_result()
|
||||
if unformatted_feature and not unformatted_feature.deleted:
|
||||
feature = feature_to_legacy_json(unformatted_feature)
|
||||
feature = converters.feature_to_legacy_json(unformatted_feature)
|
||||
feature['updated_display'] = (
|
||||
unformatted_feature.updated.strftime("%Y-%m-%d"))
|
||||
feature['new_crbug_url'] = unformatted_feature.new_crbug_url()
|
||||
feature['new_crbug_url'] = _new_crbug_url(
|
||||
unformatted_feature.blink_components, unformatted_feature.bug_url,
|
||||
unformatted_feature.impl_status_chrome, unformatted_feature.owner)
|
||||
store_key = Feature.feature_cache_key(
|
||||
Feature.DEFAULT_CACHE_KEY, unformatted_feature.key.integer_id())
|
||||
rediscache.set(store_key, feature)
|
||||
|
|
@ -61,7 +95,7 @@ def get_by_ids(feature_ids: list[int],
|
|||
if feature_id in result_dict]
|
||||
return result_list
|
||||
|
||||
def get_all(limit=None, order='-updated', filterby=None,
|
||||
def get_all_legacy(limit=None, order='-updated', filterby=None,
|
||||
update_cache=False, keys_only=False):
|
||||
"""Return JSON dicts for entities that fit the filterby criteria.
|
||||
|
||||
|
|
@ -99,13 +133,13 @@ def get_all(limit=None, order='-updated', filterby=None,
|
|||
feature_list = query.fetch(limit, keys_only=keys_only)
|
||||
if not keys_only:
|
||||
feature_list = [
|
||||
feature_to_legacy_json(f) for f in feature_list]
|
||||
converters.feature_to_legacy_json(f) for f in feature_list]
|
||||
|
||||
rediscache.set(KEY, feature_list)
|
||||
|
||||
return feature_list
|
||||
|
||||
def get_feature(feature_id: int, update_cache: bool=False) -> Optional[Feature]:
|
||||
def get_feature_legacy(feature_id: int, update_cache: bool=False) -> Optional[Feature]:
|
||||
"""Return a JSON dict for a feature.
|
||||
|
||||
Because the cache may rarely have stale data, this should only be
|
||||
|
|
@ -117,14 +151,16 @@ def get_feature(feature_id: int, update_cache: bool=False) -> Optional[Feature]:
|
|||
feature = rediscache.get(KEY)
|
||||
|
||||
if feature is None or update_cache:
|
||||
unformatted_feature = Feature.get_by_id(feature_id)
|
||||
unformatted_feature: Optional[Feature] = Feature.get_by_id(feature_id)
|
||||
if unformatted_feature:
|
||||
if unformatted_feature.deleted:
|
||||
return None
|
||||
feature = feature_to_legacy_json(unformatted_feature)
|
||||
feature = converters.feature_to_legacy_json(unformatted_feature)
|
||||
feature['updated_display'] = (
|
||||
unformatted_feature.updated.strftime("%Y-%m-%d"))
|
||||
feature['new_crbug_url'] = unformatted_feature.new_crbug_url()
|
||||
feature['new_crbug_url'] = _new_crbug_url(
|
||||
unformatted_feature.blink_components, unformatted_feature.bug_url,
|
||||
unformatted_feature.impl_status_chrome, unformatted_feature.owner)
|
||||
rediscache.set(KEY, feature)
|
||||
|
||||
return feature
|
||||
|
|
@ -238,7 +274,7 @@ def get_chronological(limit=None, update_cache: bool=False,
|
|||
all_features.extend(no_longer_pursuing_features)
|
||||
all_features = [f for f in all_features if not f.deleted]
|
||||
|
||||
feature_list = [feature_to_legacy_json(f) for f in all_features]
|
||||
feature_list = [converters.feature_to_legacy_json(f) for f in all_features]
|
||||
|
||||
Feature._annotate_first_of_milestones(feature_list)
|
||||
|
||||
|
|
@ -250,7 +286,7 @@ def get_chronological(limit=None, update_cache: bool=False,
|
|||
|
||||
def get_in_milestone(milestone: int,
|
||||
show_unlisted: bool=False) -> dict[str, list[dict[str, Any]]]:
|
||||
"""Return {reason: [feaure_dict]} with all the reasons a feature can
|
||||
"""Return {reason: [feature_dict]} with all the reasons a feature can
|
||||
be part of a milestone.
|
||||
|
||||
Because the cache may rarely have stale data, this should only be
|
||||
|
|
@ -386,14 +422,15 @@ def get_in_milestone(milestone: int,
|
|||
all_features[shipping_type].sort(key=lambda f: f.name)
|
||||
all_features[shipping_type] = [
|
||||
f for f in all_features[shipping_type] if not f.deleted]
|
||||
features_by_type[shipping_type] = [
|
||||
feature_to_legacy_json(f) for f in all_features[shipping_type]]
|
||||
features_by_type[shipping_type] = [converters.feature_to_legacy_json(f)
|
||||
for f in all_features[shipping_type]]
|
||||
|
||||
rediscache.set(cache_key, features_by_type)
|
||||
|
||||
for shipping_type in features_by_type:
|
||||
if not show_unlisted:
|
||||
features_by_type[shipping_type] = filter_unlisted(features_by_type[shipping_type])
|
||||
features_by_type[shipping_type] = filter_unlisted(
|
||||
features_by_type[shipping_type])
|
||||
|
||||
return features_by_type
|
||||
|
||||
|
|
@ -423,3 +460,123 @@ def get_all_with_statuses(statuses, update_cache=False):
|
|||
rediscache.set(KEY, feature_list)
|
||||
|
||||
return feature_list
|
||||
|
||||
def get_all(limit: Optional[int]=None,
|
||||
order: str='-updated', filterby: Optional[tuple[str, Any]]=None,
|
||||
update_cache: bool=False, keys_only: bool=False) -> list[dict]:
|
||||
"""Return JSON dicts for entities that fit the filterby criteria.
|
||||
|
||||
Because the cache may rarely have stale data, this should only be
|
||||
used for displaying data read-only, not for populating forms or
|
||||
procesing a POST to edit data. For editing use case, load the
|
||||
data from NDB directly.
|
||||
"""
|
||||
KEY = '%s|%s|%s|%s' % (
|
||||
FeatureEntry.DEFAULT_CACHE_KEY, order, limit, keys_only)
|
||||
|
||||
# TODO(ericbidelman): Support more than one filter.
|
||||
if filterby is not None:
|
||||
s = ('%s%s' % (filterby[0], filterby[1])).replace(' ', '')
|
||||
KEY += '|%s' % s
|
||||
|
||||
feature_list = rediscache.get(KEY)
|
||||
|
||||
if feature_list is None or update_cache:
|
||||
query = FeatureEntry.query().order(-FeatureEntry.updated) #.order('name')
|
||||
query = query.filter(FeatureEntry.deleted == False)
|
||||
|
||||
# TODO(ericbidelman): Support more than one filter.
|
||||
if filterby:
|
||||
filter_type, comparator = filterby
|
||||
if filter_type == 'can_edit':
|
||||
# can_edit will check if the user has any access to edit the feature.
|
||||
# This includes being an owner, editor, or the original creator
|
||||
# of the feature.
|
||||
query = query.filter(
|
||||
ndb.OR(FeatureEntry.owner_emails == comparator,
|
||||
FeatureEntry.editor_emails == comparator,
|
||||
FeatureEntry.creator_email == comparator))
|
||||
else:
|
||||
query = query.filter(getattr(FeatureEntry, filter_type) == comparator)
|
||||
|
||||
feature_list = query.fetch(limit, keys_only=keys_only)
|
||||
if not keys_only:
|
||||
feature_list = [
|
||||
converters.feature_entry_to_json_basic(f) for f in feature_list]
|
||||
|
||||
rediscache.set(KEY, feature_list)
|
||||
|
||||
return feature_list
|
||||
|
||||
def get_feature(
|
||||
feature_id: int, update_cache: bool=False) -> Optional[FeatureEntry]:
|
||||
"""Return a JSON dict for a feature.
|
||||
|
||||
Because the cache may rarely have stale data, this should only be
|
||||
used for displaying data read-only, not for populating forms or
|
||||
procesing a POST to edit data. For editing use case, load the
|
||||
data from NDB directly.
|
||||
"""
|
||||
KEY = Feature.feature_cache_key(FeatureEntry.DEFAULT_CACHE_KEY, feature_id)
|
||||
feature = rediscache.get(KEY)
|
||||
|
||||
if feature is None or update_cache:
|
||||
unformatted_feature: Optional[FeatureEntry] = (
|
||||
FeatureEntry.get_by_id(feature_id))
|
||||
if unformatted_feature:
|
||||
if unformatted_feature.deleted:
|
||||
return None
|
||||
feature = converters.feature_entry_to_json_verbose(unformatted_feature)
|
||||
feature['updated_display'] = (
|
||||
unformatted_feature.updated.strftime("%Y-%m-%d"))
|
||||
feature['new_crbug_url'] = _new_crbug_url(
|
||||
unformatted_feature.blink_components, unformatted_feature.bug_url,
|
||||
unformatted_feature.impl_status_chrome,
|
||||
unformatted_feature.owner_emails)
|
||||
rediscache.set(KEY, feature)
|
||||
|
||||
return feature
|
||||
|
||||
def get_by_ids(feature_ids: list[int],
|
||||
update_cache: bool=False) -> list[dict[str, Any]]:
|
||||
"""Return a list of JSON dicts for the specified features.
|
||||
|
||||
Because the cache may rarely have stale data, this should only be
|
||||
used for displaying data read-only, not for populating forms or
|
||||
procesing a POST to edit data. For editing use case, load the
|
||||
data from NDB directly.
|
||||
"""
|
||||
result_dict = {}
|
||||
futures = []
|
||||
|
||||
for feature_id in feature_ids:
|
||||
lookup_key = FeatureEntry.feature_cache_key(
|
||||
FeatureEntry.DEFAULT_CACHE_KEY, feature_id)
|
||||
feature = rediscache.get(lookup_key)
|
||||
if feature is None or update_cache:
|
||||
futures.append(FeatureEntry.get_by_id_async(feature_id))
|
||||
else:
|
||||
result_dict[feature_id] = feature
|
||||
|
||||
for future in futures:
|
||||
unformatted_feature: Optional[FeatureEntry] = future.get_result()
|
||||
if unformatted_feature and not unformatted_feature.deleted:
|
||||
feature = converters.feature_entry_to_json_verbose(unformatted_feature)
|
||||
if unformatted_feature.updated is not None:
|
||||
feature['updated_display'] = (
|
||||
unformatted_feature.updated.strftime("%Y-%m-%d"))
|
||||
else:
|
||||
feature['updated_display'] = ''
|
||||
feature['new_crbug_url'] = _new_crbug_url(
|
||||
unformatted_feature.blink_components, unformatted_feature.bug_url,
|
||||
unformatted_feature.impl_status_chrome,
|
||||
unformatted_feature.owner_emails)
|
||||
store_key = FeatureEntry.feature_cache_key(
|
||||
FeatureEntry.DEFAULT_CACHE_KEY, unformatted_feature.key.integer_id())
|
||||
rediscache.set(store_key, feature)
|
||||
result_dict[unformatted_feature.key.integer_id()] = feature
|
||||
|
||||
result_list = [
|
||||
result_dict[feature_id] for feature_id in feature_ids
|
||||
if feature_id in result_dict]
|
||||
return result_list
|
||||
|
|
|
|||
|
|
@ -242,7 +242,7 @@ class FeatureStar(ndb.Model):
|
|||
if feature_entry.star_count < 0:
|
||||
logging.error('count would be < 0: %r', (email, feature_id, starred))
|
||||
return
|
||||
feature_entry.put() # And, do not call notifiy.
|
||||
feature_entry.put() # And, do not call notify.
|
||||
|
||||
@classmethod
|
||||
def get_user_stars(self, email):
|
||||
|
|
|
|||
|
|
@ -13,11 +13,30 @@
|
|||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
from framework import cloud_tasks_helpers
|
||||
from typing import Any, Optional
|
||||
from api import converters
|
||||
from framework import cloud_tasks_helpers, rediscache, users
|
||||
from internals import core_enums
|
||||
from internals import feature_helpers
|
||||
from internals import review_models
|
||||
|
||||
def _get_changes_as_amendments(f) -> list[review_models.Amendment]:
|
||||
"""Get all feature changes as Amendment entities."""
|
||||
# Diff values to see what properties have changed.
|
||||
amendments = []
|
||||
for prop_name in f._properties.keys():
|
||||
if prop_name in (
|
||||
'created_by', 'updated_by', 'updated', 'created'):
|
||||
continue
|
||||
new_val = getattr(f, prop_name, None)
|
||||
old_val = getattr(f, '_old_' + prop_name, None)
|
||||
if new_val != old_val:
|
||||
if (new_val == '' or new_val == False) and old_val is None:
|
||||
continue
|
||||
amendments.append(
|
||||
review_models.Amendment(field_name=prop_name,
|
||||
old_value=str(old_val), new_value=str(new_val)))
|
||||
return amendments
|
||||
|
||||
def notify_feature_subscribers_of_changes(feature,
|
||||
amendments: list[review_models.Amendment], is_update: bool) -> None:
|
||||
"""Async notifies subscribers of new features and property changes to
|
||||
|
|
@ -34,8 +53,33 @@ def notify_feature_subscribers_of_changes(feature,
|
|||
params = {
|
||||
'changes': changed_props,
|
||||
'is_update': is_update,
|
||||
'feature': feature_helpers.feature_to_legacy_json(feature)
|
||||
# TODO(danielrsmith): Change converter function
|
||||
# after switch to using FeatureEntry here.
|
||||
'feature': converters.feature_to_legacy_json(feature)
|
||||
}
|
||||
|
||||
# Create task to email subscribers.
|
||||
cloud_tasks_helpers.enqueue_task('/tasks/email-subscribers', params)
|
||||
|
||||
def notify_subscribers_and_save_amendments(fe, notify: bool=True) -> None:
|
||||
"""Notify subscribers of changes to FeatureEntry and save amendments."""
|
||||
is_update = (fe.key is not None)
|
||||
amendments = _get_changes_as_amendments(fe)
|
||||
|
||||
# Document changes as new Activity entity with amendments only if all true:
|
||||
# 1. This is an update to an existing feature.
|
||||
# 2. We used stash_values() to document what fields changed.
|
||||
# 3. One or more fields were changed.
|
||||
should_write_activity = (is_update and hasattr(fe, '_values_stashed')
|
||||
and len(amendments) > 0)
|
||||
|
||||
if should_write_activity:
|
||||
user = users.get_current_user()
|
||||
email = user.email() if user else None
|
||||
activity = review_models.Activity(feature_id=fe.key.integer_id(),
|
||||
author=email, content='')
|
||||
activity.amendments = amendments
|
||||
activity.put()
|
||||
|
||||
if notify:
|
||||
notify_feature_subscribers_of_changes(fe, amendments, is_update)
|
||||
|
|
|
|||
|
|
@ -281,8 +281,8 @@ class ActivityTest(testing_config.CustomTestCase):
|
|||
|
||||
def setUp(self):
|
||||
self.feature_1 = core_models.Feature(
|
||||
name='feature a', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1)
|
||||
name='feature a', summary='sum', category=1,
|
||||
owner=['feature_owner@example.com'])
|
||||
self.feature_1.put()
|
||||
testing_config.sign_in('one@example.com', 123567890)
|
||||
|
||||
|
|
|
|||
|
|
@ -91,6 +91,7 @@ class FeatureCreateHandler(basehandlers.FlaskHandler):
|
|||
|
||||
# Remove all feature-related cache.
|
||||
rediscache.delete_keys_with_prefix(Feature.feature_cache_prefix())
|
||||
rediscache.delete_keys_with_prefix(FeatureEntry.feature_cache_prefix())
|
||||
|
||||
redirect_url = '/guide/edit/' + str(key.integer_id())
|
||||
return self.redirect(redirect_url)
|
||||
|
|
@ -159,7 +160,7 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
|
|||
|
||||
def process_post_data(self, **kwargs):
|
||||
feature_id = kwargs.get('feature_id', None)
|
||||
stage_id = kwargs.get('stage_id', 0)
|
||||
intent_stage_id = kwargs.get('stage_id', 0)
|
||||
# Validate the user has edit permissions and redirect if needed.
|
||||
redirect_resp = permissions.validate_feature_edit_permission(
|
||||
self, feature_id)
|
||||
|
|
@ -168,9 +169,9 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
|
|||
|
||||
if feature_id:
|
||||
# Load feature directly from NDB so as to never get a stale cached copy.
|
||||
feature = Feature.get_by_id(feature_id)
|
||||
feature_entry = FeatureEntry.get_by_id(feature_id)
|
||||
if feature is None:
|
||||
feature: Feature = Feature.get_by_id(feature_id)
|
||||
feature_entry: FeatureEntry = FeatureEntry.get_by_id(feature_id)
|
||||
if feature_entry is None:
|
||||
self.abort(404, msg='Feature not found')
|
||||
else:
|
||||
feature.stash_values()
|
||||
|
|
@ -420,8 +421,8 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
|
|||
feature.intent_stage = int(self.form.get('intent_stage'))
|
||||
update_items.append(('intent_stage', int(self.form.get('intent_stage'))))
|
||||
elif self.form.get('set_stage') == 'on':
|
||||
feature.intent_stage = stage_id
|
||||
update_items.append(('intent_stage', stage_id))
|
||||
feature.intent_stage = intent_stage_id
|
||||
update_items.append(('intent_stage', intent_stage_id))
|
||||
|
||||
if self.touched('category'):
|
||||
feature.category = int(self.form.get('category'))
|
||||
|
|
@ -582,6 +583,7 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
|
|||
|
||||
# Remove all feature-related cache.
|
||||
rediscache.delete_keys_with_prefix(Feature.feature_cache_prefix())
|
||||
rediscache.delete_keys_with_prefix(FeatureEntry.feature_cache_prefix())
|
||||
|
||||
# Update full-text index.
|
||||
if feature_entry:
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ from google.cloud import ndb # type: ignore
|
|||
|
||||
from framework import rediscache
|
||||
from internals import core_enums
|
||||
from internals.core_models import Feature, FeatureEntry, Stage
|
||||
from internals.core_models import Feature, FeatureEntry, MilestoneSet, Stage
|
||||
from internals.review_models import Gate
|
||||
from pages import guide
|
||||
|
||||
|
|
@ -129,7 +129,8 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase):
|
|||
core_enums.STAGE_BLINK_ORIGIN_TRIAL,
|
||||
core_enums.STAGE_BLINK_EXTEND_ORIGIN_TRIAL]
|
||||
for stage_type in stage_types:
|
||||
stage = Stage(feature_id=feature_id, stage_type=stage_type)
|
||||
stage = Stage(feature_id=feature_id, stage_type=stage_type,
|
||||
milestones=MilestoneSet())
|
||||
stage.put()
|
||||
|
||||
self.request_path = ('/guide/stage/%d/%d' % (
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@
|
|||
# limitations under the License.
|
||||
|
||||
# from google.appengine.api import users
|
||||
from api.converters import feature_to_legacy_json
|
||||
from api.converters import feature_entry_to_json_verbose
|
||||
|
||||
from internals import core_enums
|
||||
from framework import basehandlers
|
||||
|
|
@ -51,7 +51,7 @@ class IntentEmailPreviewHandler(basehandlers.FlaskHandler):
|
|||
"""Return a dictionary of data used to render the page."""
|
||||
page_data = {
|
||||
'subject_prefix': self.compute_subject_prefix(f, intent_stage),
|
||||
'feature': feature_to_legacy_json(f),
|
||||
'feature': feature_entry_to_json_verbose(f),
|
||||
'sections_to_show': processes.INTENT_EMAIL_SECTIONS.get(
|
||||
intent_stage, []),
|
||||
'intent_stage': intent_stage,
|
||||
|
|
|
|||
|
|
@ -36,8 +36,8 @@ TESTDATA = testing_config.Testdata(__file__)
|
|||
class IntentEmailPreviewHandlerTest(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.feature_1 = core_models.Feature(
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
self.feature_1 = core_models.FeatureEntry(
|
||||
name='feature one', summary='sum', owner_emails=['user1@google.com'],
|
||||
category=1, intent_stage=core_enums.INTENT_IMPLEMENT)
|
||||
self.feature_1.put()
|
||||
|
||||
|
|
@ -191,11 +191,11 @@ class IntentEmailPreviewTemplateTest(testing_config.CustomTestCase):
|
|||
|
||||
def setUp(self):
|
||||
super(IntentEmailPreviewTemplateTest, self).setUp()
|
||||
self.feature_1 = core_models.Feature(
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
self.feature_1 = core_models.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('Feature', 234)
|
||||
self.feature_1.key = ndb.Key('FeatureEntry', 234)
|
||||
self.feature_1.put()
|
||||
|
||||
self.request_path = '/admin/features/launch/%d/%d?intent' % (
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче