New permissions system implementation (#2011)
* Check edit permissions by individual feature * update logic to check permissions * editors field added * editors can see edit access in UI * myfeatures shows editable features * Make editor field plural * 1 more plural update * change var name for site editor * Editors can delete a feature * Site editors can now be designated in admin ui * fix existing tests to work with new changes * feature and permissions tests * accounts and models test * new creator field * update tests for creator field * code cleanup * add creator field to new feature * unlisted features are displayed in "My Features" If the user is the creator, an owner, or an editor. * slight update in unlisted features logic * My features displays all editable features in a single dropdown. * fix web tests * add test to ensure creator can edit feature * remove unused _reject_or_proceed() change * remove mangle and log statement * Set original permissions All changes to permissions.py removed. This commit can be reverted to enable to new permissions system. * changes suggested by @jrobbins * update from merge * remove creator field for later implementation * changes suggested by @jrobbins
This commit is contained in:
Daniel Smith
GitHub
Родитель
8a66dbc810
Коммит
83e77b9d5b
|
|
@ -35,11 +35,12 @@ class AccountsAPI(basehandlers.APIHandler):
|
|||
"""Process a request to create an account."""
|
||||
email = self.get_param('email', required=True)
|
||||
is_admin = self.get_bool_param('isAdmin')
|
||||
user = self.create_account(email, is_admin)
|
||||
is_site_editor = self.get_bool_param('isSiteEditor')
|
||||
user = self.create_account(email, is_admin, is_site_editor)
|
||||
response_json = user.format_for_template()
|
||||
return response_json
|
||||
|
||||
def create_account(self, email, is_admin):
|
||||
def create_account(self, email, is_admin, is_site_editor):
|
||||
"""Create and store a new account entity."""
|
||||
# Don't add a duplicate email address.
|
||||
user = models.AppUser.query(
|
||||
|
|
@ -47,6 +48,7 @@ class AccountsAPI(basehandlers.APIHandler):
|
|||
if not user:
|
||||
user = models.AppUser(email=str(email))
|
||||
user.is_admin = is_admin
|
||||
user.is_site_editor = is_site_editor
|
||||
user.put()
|
||||
return user
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -31,6 +31,11 @@ class AccountsAPITest(testing_config.CustomTestCase):
|
|||
self.app_admin = models.AppUser(email='admin@example.com')
|
||||
self.app_admin.is_admin = True
|
||||
self.app_admin.put()
|
||||
|
||||
self.app_editor = models.AppUser(email='editor@example.com')
|
||||
self.app_editor.is_site_editor = True
|
||||
self.app_editor.put()
|
||||
|
||||
self.appuser_1 = models.AppUser(email='user@example.com')
|
||||
self.appuser_1.put()
|
||||
self.appuser_id = self.appuser_1.key.integer_id()
|
||||
|
|
@ -41,30 +46,56 @@ class AccountsAPITest(testing_config.CustomTestCase):
|
|||
def tearDown(self):
|
||||
self.appuser_1.key.delete()
|
||||
self.app_admin.key.delete()
|
||||
self.app_editor.key.delete()
|
||||
|
||||
def test_create__normal_valid(self):
|
||||
"""Admin wants to register a normal account."""
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
json_data = {'email': 'new@example.com', 'isAdmin': False}
|
||||
json_data = {
|
||||
'email': 'new_user@example.com',
|
||||
'isAdmin': False, 'isSiteEditor': False}
|
||||
with test_app.test_request_context(self.request_path, json=json_data):
|
||||
actual_json = self.handler.do_post()
|
||||
self.assertEqual('new@example.com', actual_json['email'])
|
||||
self.assertEqual('new_user@example.com', actual_json['email'])
|
||||
self.assertFalse(actual_json['is_site_editor'])
|
||||
self.assertFalse(actual_json['is_admin'])
|
||||
|
||||
new_appuser = (models.AppUser.query(
|
||||
models.AppUser.email == 'new@example.com').get())
|
||||
self.assertEqual('new@example.com', new_appuser.email)
|
||||
models.AppUser.email == 'new_user@example.com').get())
|
||||
self.assertEqual('new_user@example.com', new_appuser.email)
|
||||
self.assertFalse(new_appuser.is_admin)
|
||||
|
||||
def test_create__site_editor_valid(self):
|
||||
"""Admin wants to register a new site editor account."""
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
json_data = {
|
||||
'email': 'new_site_editor@example.com',
|
||||
'isAdmin': False, 'isSiteEditor': True}
|
||||
with test_app.test_request_context(self.request_path, json=json_data):
|
||||
actual_json = self.handler.do_post()
|
||||
self.assertEqual('new_site_editor@example.com', actual_json['email'])
|
||||
self.assertFalse(actual_json['is_admin'])
|
||||
self.assertTrue(actual_json['is_site_editor'])
|
||||
|
||||
new_appuser = models.AppUser.query(
|
||||
models.AppUser.email == 'new_site_editor@example.com').get()
|
||||
self.assertEqual('new_site_editor@example.com', new_appuser.email)
|
||||
self.assertTrue(new_appuser.is_site_editor)
|
||||
self.assertFalse(new_appuser.is_admin)
|
||||
|
||||
def test_create__admin_valid(self):
|
||||
"""Admin wants to register a new admin account."""
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
||||
json_data = {'email': 'new_admin@example.com', 'isAdmin': True}
|
||||
json_data = {
|
||||
'email': 'new_admin@example.com',
|
||||
'isAdmin': True, 'isSiteEditor': True}
|
||||
with test_app.test_request_context(self.request_path, json=json_data):
|
||||
actual_json = self.handler.do_post()
|
||||
self.assertEqual('new_admin@example.com', actual_json['email'])
|
||||
self.assertTrue(actual_json['is_site_editor'])
|
||||
self.assertTrue(actual_json['is_admin'])
|
||||
|
||||
new_appuser = models.AppUser.query(
|
||||
|
|
@ -84,6 +115,18 @@ class AccountsAPITest(testing_config.CustomTestCase):
|
|||
models.AppUser.email == 'new@example.com').get()
|
||||
self.assertIsNone(new_appuser)
|
||||
|
||||
def test_create__site_editor_forbidden(self):
|
||||
"""Site editors cannot create an account."""
|
||||
testing_config.sign_in('editor@example.com', 123567890)
|
||||
|
||||
with test_app.test_request_context(self.request_path):
|
||||
with self.assertRaises(werkzeug.exceptions.Forbidden):
|
||||
self.handler.do_post()
|
||||
|
||||
new_appuser = models.AppUser.query(
|
||||
models.AppUser.email == 'new@example.com').get()
|
||||
self.assertIsNone(new_appuser)
|
||||
|
||||
def test_create__invalid(self):
|
||||
"""We cannot create an account without an email address."""
|
||||
testing_config.sign_in('admin@example.com', 123567890)
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ class FeaturesAPI(basehandlers.APIHandler):
|
|||
|
||||
def do_search(self):
|
||||
user = users.get_current_user()
|
||||
# Show unlisted features to site editors or admins.
|
||||
show_unlisted_features = permissions.can_edit_feature(user, None)
|
||||
features_on_page = None
|
||||
|
||||
|
|
|
|||
|
|
@ -98,9 +98,10 @@ class FeaturesAPITestGet(testing_config.CustomTestCase):
|
|||
|
||||
def setUp(self):
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=5,
|
||||
intent_stage=models.INTENT_IMPLEMENT, shipped_milestone=1)
|
||||
name='feature one', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=5, intent_stage=models.INTENT_IMPLEMENT,
|
||||
shipped_milestone=1)
|
||||
self.feature_1.put()
|
||||
self.feature_id = self.feature_1.key.integer_id()
|
||||
|
||||
|
|
|
|||
|
|
@ -38,9 +38,10 @@ class PermissionsAPI(basehandlers.APIHandler):
|
|||
'can_create_feature': permissions.can_create_feature(user),
|
||||
'can_approve': permissions.can_approve_feature(
|
||||
user, None, approvers),
|
||||
'can_edit': permissions.can_edit_any_feature(user),
|
||||
'can_edit_all': permissions.can_edit_feature(user, None),
|
||||
'is_admin': permissions.can_admin_site(user),
|
||||
'email': user.email(),
|
||||
'editable_features': []
|
||||
}
|
||||
|
||||
return {'user': user_data}
|
||||
|
|
|
|||
|
|
@ -50,14 +50,15 @@ class PermissionsAPITest(testing_config.CustomTestCase):
|
|||
'user': {
|
||||
'can_create_feature': False,
|
||||
'can_approve': False,
|
||||
'can_edit': False,
|
||||
'can_edit_all': False,
|
||||
'is_admin': False,
|
||||
'email': 'one@example.com'
|
||||
'email': 'one@example.com',
|
||||
'editable_features': []
|
||||
}}
|
||||
self.assertEqual(expected, actual)
|
||||
|
||||
def test_get__googler(self):
|
||||
"""Googlers have default permissions to create feature and edit."""
|
||||
"""Googlers have default permissions to create and edit features."""
|
||||
testing_config.sign_in('one@google.com', 67890)
|
||||
with test_app.test_request_context(self.request_path):
|
||||
actual = self.handler.do_get()
|
||||
|
|
@ -65,8 +66,9 @@ class PermissionsAPITest(testing_config.CustomTestCase):
|
|||
'user': {
|
||||
'can_create_feature': True,
|
||||
'can_approve': False,
|
||||
'can_edit': True,
|
||||
'can_edit_all': True,
|
||||
'is_admin': False,
|
||||
'email': 'one@google.com'
|
||||
'email': 'one@google.com',
|
||||
'editable_features': []
|
||||
}}
|
||||
self.assertEqual(expected, actual)
|
||||
|
|
|
|||
|
|
@ -318,8 +318,9 @@ class FlaskHandler(BaseHandler):
|
|||
'can_create_feature': permissions.can_create_feature(user),
|
||||
'can_approve': permissions.can_approve_feature(
|
||||
user, None, approvers),
|
||||
'can_edit': permissions.can_edit_any_feature(user),
|
||||
'can_edit_all': permissions.can_edit_any_feature(user),
|
||||
'is_admin': permissions.can_admin_site(user),
|
||||
'editable_features': [],
|
||||
'email': user.email(),
|
||||
'dismissed_cues': json.dumps(user_pref.dismissed_cues),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -51,22 +51,48 @@ test_app = basehandlers.FlaskApplication(
|
|||
class PermissionFunctionTests(testing_config.CustomTestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.users = []
|
||||
self.app_user = models.AppUser(email='registered@example.com')
|
||||
self.app_user.put()
|
||||
self.users.append(self.app_user)
|
||||
|
||||
self.app_admin = models.AppUser(email='admin@example.com')
|
||||
self.app_admin.is_admin = True
|
||||
self.app_admin.put()
|
||||
self.users.append(self.app_admin)
|
||||
|
||||
self.app_editor = models.AppUser(email='editor@example.com')
|
||||
self.app_editor.is_site_editor = True
|
||||
self.app_editor.put()
|
||||
self.users.append(self.app_editor)
|
||||
|
||||
self.feature_owner = models.AppUser(email='feature_owner@example.com')
|
||||
self.feature_owner.put()
|
||||
self.users.append(self.feature_owner)
|
||||
|
||||
self.feature_editor = models.AppUser(email='feature_editor@example.com')
|
||||
self.feature_editor.put()
|
||||
self.users.append(self.feature_editor)
|
||||
|
||||
# Feature for checking permissions against
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum',
|
||||
owner=['feature_owner@example.com'],
|
||||
editors=['feature_editor@example.com'], category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=1)
|
||||
self.feature_1.put()
|
||||
self.feature_id = self.feature_1.key.integer_id()
|
||||
|
||||
def tearDown(self):
|
||||
self.app_user.delete()
|
||||
self.app_admin.delete()
|
||||
for user in self.users:
|
||||
user.delete()
|
||||
self.feature_1.key.delete()
|
||||
|
||||
def check_function_results(
|
||||
self, func, additional_args,
|
||||
unregistered='missing', registered='missing',
|
||||
special='missing', admin='missing', anon='missing'):
|
||||
"""Test func under four conditions and check expected results."""
|
||||
special='missing', site_editor='missing', admin='missing', anon='missing'):
|
||||
"""Test func under six conditions and check expected results."""
|
||||
# Test unregistered users
|
||||
testing_config.sign_in('unregistered@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
|
|
@ -86,6 +112,11 @@ class PermissionFunctionTests(testing_config.CustomTestCase):
|
|||
user = users.get_current_user()
|
||||
self.assertEqual(special, func(user, *additional_args))
|
||||
|
||||
# Test site editor user
|
||||
testing_config.sign_in('editor@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
self.assertEqual(site_editor, func(user, *additional_args))
|
||||
|
||||
# Test admin users
|
||||
testing_config.sign_in('admin@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
|
|
@ -96,48 +127,98 @@ class PermissionFunctionTests(testing_config.CustomTestCase):
|
|||
user = users.get_current_user()
|
||||
self.assertEqual(anon, func(user, *additional_args))
|
||||
|
||||
def check_function_results_with_feature(
|
||||
self, func, additional_args, unregistered='missing',
|
||||
registered='missing', feature_owner='missing', feature_editor='missing',
|
||||
site_editor='missing', admin='missing'):
|
||||
"""Test func in the context of a specific feature id."""
|
||||
# Test unregistered users
|
||||
testing_config.sign_in('unregistered@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
self.assertEqual(unregistered, func(user, *additional_args))
|
||||
|
||||
# Test registered users
|
||||
testing_config.sign_in('registered@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
self.assertEqual(registered, func(user, *additional_args))
|
||||
|
||||
# Test feature owners
|
||||
testing_config.sign_in('feature_owner@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
self.assertEqual(feature_owner, func(user, *additional_args))
|
||||
|
||||
# Test feature editors
|
||||
testing_config.sign_in('feature_editor@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
self.assertEqual(feature_editor, func(user, *additional_args))
|
||||
|
||||
# Test site editor user
|
||||
testing_config.sign_in('editor@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
self.assertEqual(site_editor, func(user, *additional_args))
|
||||
|
||||
# Test admin users
|
||||
testing_config.sign_in('admin@example.com', 123)
|
||||
user = users.get_current_user()
|
||||
self.assertEqual(admin, func(user, *additional_args))
|
||||
|
||||
def test_can_admin_site(self):
|
||||
self.check_function_results(
|
||||
permissions.can_admin_site, tuple(),
|
||||
unregistered=False, registered=False,
|
||||
special=False, admin=True, anon=False)
|
||||
special=False, site_editor=False, admin=True, anon=False)
|
||||
|
||||
def test_can_view_feature(self):
|
||||
self.check_function_results(
|
||||
permissions.can_view_feature, (None,),
|
||||
unregistered=True, registered=True,
|
||||
special=True, admin=True, anon=True)
|
||||
special=True, site_editor=True, admin=True, anon=True)
|
||||
|
||||
self.check_function_results_with_feature(
|
||||
permissions.can_view_feature, (self.feature_id,),
|
||||
unregistered=True, registered=True,
|
||||
feature_owner=True, feature_editor=True,
|
||||
site_editor=True, admin=True
|
||||
)
|
||||
|
||||
def test_can_create_feature(self):
|
||||
self.check_function_results(
|
||||
permissions.can_create_feature, tuple(),
|
||||
unregistered=False, registered=True,
|
||||
special=True, admin=True, anon=False)
|
||||
special=True, site_editor=True, admin=True, anon=False)
|
||||
|
||||
def test_can_edit_any_feature(self):
|
||||
self.check_function_results(
|
||||
permissions.can_edit_any_feature, tuple(),
|
||||
unregistered=False, registered=True,
|
||||
special=True, admin=True, anon=False)
|
||||
special=True, site_editor=True, admin=True, anon=False)
|
||||
|
||||
def test_can_edit_feature(self):
|
||||
self.check_function_results(
|
||||
permissions.can_edit_feature, (None,),
|
||||
unregistered=False, registered=True,
|
||||
special=True, admin=True, anon=False)
|
||||
special=True, site_editor=True, admin=True, anon=False)
|
||||
|
||||
# Check in context of specific feature.
|
||||
self.check_function_results_with_feature(
|
||||
permissions.can_edit_feature, (self.feature_id,),
|
||||
unregistered=False, registered=True,
|
||||
feature_owner=True, feature_editor=True,
|
||||
site_editor=True, admin=True
|
||||
)
|
||||
|
||||
def test_can_approve_feature(self):
|
||||
approvers = []
|
||||
self.check_function_results(
|
||||
permissions.can_approve_feature, (None, approvers),
|
||||
unregistered=False, registered=False,
|
||||
special=False, admin=True, anon=False)
|
||||
special=False, site_editor=False, admin=True, anon=False)
|
||||
|
||||
approvers = ['registered@example.com']
|
||||
self.check_function_results(
|
||||
permissions.can_approve_feature, (None, approvers),
|
||||
unregistered=False, registered=True,
|
||||
special=False, admin=True, anon=False)
|
||||
special=False, site_editor=False, admin=True, anon=False)
|
||||
|
||||
|
||||
class RequireAdminSiteTests(testing_config.CustomTestCase):
|
||||
|
|
@ -146,12 +227,17 @@ class RequireAdminSiteTests(testing_config.CustomTestCase):
|
|||
self.app_user = models.AppUser(email='registered@example.com')
|
||||
self.app_user.put()
|
||||
|
||||
self.app_editor = models.AppUser(email='editor@example.com')
|
||||
self.app_editor.is_site_editor = True
|
||||
self.app_editor.put()
|
||||
|
||||
self.app_admin = models.AppUser(email='admin@example.com')
|
||||
self.app_admin.is_admin = True
|
||||
self.app_admin.put()
|
||||
|
||||
def tearDown(self):
|
||||
self.app_user.delete()
|
||||
self.app_editor.delete()
|
||||
self.app_admin.delete()
|
||||
|
||||
def test_require_admin_site__unregistered_user(self):
|
||||
|
|
@ -181,6 +267,15 @@ class RequireAdminSiteTests(testing_config.CustomTestCase):
|
|||
handler.do_post()
|
||||
self.assertEqual(handler.called_with, None)
|
||||
|
||||
def test_require_admin_site__editor(self):
|
||||
"""Wrapped method rejects call from a site editor."""
|
||||
handler = MockHandler()
|
||||
testing_config.sign_in('editor@example.com', 123)
|
||||
with test_app.test_request_context('/path', method='POST'):
|
||||
with self.assertRaises(werkzeug.exceptions.Forbidden):
|
||||
handler.do_post()
|
||||
self.assertEqual(handler.called_with, None)
|
||||
|
||||
def test_require_admin_site__admin(self):
|
||||
"""Wrapped method accepts call from an admin user."""
|
||||
handler = MockHandler()
|
||||
|
|
|
|||
|
|
@ -202,6 +202,10 @@ indexes:
|
|||
- name: "updated"
|
||||
direction: desc
|
||||
- name: "name"
|
||||
- kind: Feature
|
||||
properties:
|
||||
- name: owner
|
||||
- name: editors
|
||||
- kind: "FeatureObserver"
|
||||
properties:
|
||||
- name: "bucket_id"
|
||||
|
|
|
|||
|
|
@ -633,6 +633,7 @@ class Feature(DictModel):
|
|||
'docs': d.pop('doc_links', []),
|
||||
}
|
||||
d['tags'] = d.pop('search_tags', [])
|
||||
d['editors'] = d.pop('editors', [])
|
||||
d['browsers'] = {
|
||||
'chrome': {
|
||||
'bug': d.pop('bug_url', None),
|
||||
|
|
@ -748,6 +749,7 @@ class Feature(DictModel):
|
|||
d = self.to_dict()
|
||||
#d['id'] = self.key().id
|
||||
d['owner'] = ', '.join(self.owner)
|
||||
d['editors'] = ', '.join(self.editors)
|
||||
d['explainer_links'] = '\r\n'.join(self.explainer_links)
|
||||
d['spec_mentors'] = ', '.join(self.spec_mentors)
|
||||
d['standard_maturity'] = self.standard_maturity or UNKNOWN_STD
|
||||
|
|
@ -778,10 +780,15 @@ class Feature(DictModel):
|
|||
|
||||
# TODO(ericbidelman): Support more than one filter.
|
||||
if filterby:
|
||||
if filterby[0] == 'category':
|
||||
query = query.filter(Feature.category == filterby[1])
|
||||
elif filterby[0] == 'owner':
|
||||
query = query.filter(Feature.owner == filterby[1])
|
||||
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(Feature.owner == comparator, Feature.editors == comparator))
|
||||
else:
|
||||
query = query.filter(getattr(Feature, filter_type) == comparator)
|
||||
|
||||
features = query.fetch(limit)
|
||||
|
||||
|
|
@ -879,6 +886,23 @@ class Feature(DictModel):
|
|||
ramcache.set(KEY, feature)
|
||||
|
||||
return feature
|
||||
|
||||
@classmethod
|
||||
def filter_unlisted(self, feature_list):
|
||||
"""Filters a feature list to display only features the user should see."""
|
||||
user = users.get_current_user()
|
||||
email = None
|
||||
if user:
|
||||
email = user.email()
|
||||
listed_features = []
|
||||
for f in feature_list:
|
||||
# Owners and editors of a feature should still be able to see their features.
|
||||
if ((not f.get('unlisted', False)) or
|
||||
('browsers' in f and email in f['browsers']['chrome']['owners']) or
|
||||
(email in f.get('editors', []))):
|
||||
listed_features.append(f)
|
||||
|
||||
return listed_features
|
||||
|
||||
@classmethod
|
||||
def get_by_ids(self, feature_ids, update_cache=False):
|
||||
|
|
@ -1000,11 +1024,9 @@ class Feature(DictModel):
|
|||
|
||||
ramcache.set(cache_key, feature_list)
|
||||
|
||||
allowed_feature_list = [
|
||||
f for f in feature_list
|
||||
if show_unlisted or not f['unlisted']]
|
||||
|
||||
return allowed_feature_list
|
||||
if not show_unlisted:
|
||||
feature_list = self.filter_unlisted(feature_list)
|
||||
return feature_list
|
||||
|
||||
@classmethod
|
||||
def get_in_milestone(
|
||||
|
|
@ -1012,145 +1034,147 @@ class Feature(DictModel):
|
|||
if milestone == None:
|
||||
return None
|
||||
|
||||
cache_key = '%s|%s|%s|%s' % (
|
||||
Feature.DEFAULT_CACHE_KEY, 'milestone', show_unlisted, milestone)
|
||||
cached_allowed_features_by_type = ramcache.get(cache_key)
|
||||
if cached_allowed_features_by_type:
|
||||
return cached_allowed_features_by_type
|
||||
|
||||
all_features = {}
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[REMOVED]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[INTERVENTION]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[ORIGIN_TRIAL]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[BEHIND_A_FLAG]] = []
|
||||
|
||||
logging.info('Getting chronological feature list in milestone %d',
|
||||
milestone)
|
||||
# Start each query asynchronously in parallel.
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.shipped_milestone == milestone)
|
||||
desktop_shipping_features_future = q.fetch_async(None)
|
||||
|
||||
# Features with an android shipping milestone but no desktop milestone.
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.shipped_android_milestone == milestone)
|
||||
q = q.filter(Feature.shipped_milestone == None)
|
||||
android_only_shipping_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in origin trial (Desktop) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.ot_milestone_desktop_start == milestone)
|
||||
desktop_origin_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in origin trial (Android) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.ot_milestone_android_start == milestone)
|
||||
q = q.filter(Feature.ot_milestone_desktop_start == None)
|
||||
android_origin_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in origin trial (Webview) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.ot_milestone_webview_start == milestone)
|
||||
q = q.filter(Feature.ot_milestone_desktop_start == None)
|
||||
webview_origin_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in dev trial (Desktop) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.dt_milestone_desktop_start == milestone)
|
||||
desktop_dev_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in dev trial (Android) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.dt_milestone_android_start == milestone)
|
||||
q = q.filter(Feature.dt_milestone_desktop_start == None)
|
||||
android_dev_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Wait for all futures to complete.
|
||||
desktop_shipping_features = desktop_shipping_features_future.result()
|
||||
android_only_shipping_features = (
|
||||
android_only_shipping_features_future.result())
|
||||
desktop_origin_trial_features = (
|
||||
desktop_origin_trial_features_future.result())
|
||||
android_origin_trial_features = (
|
||||
android_origin_trial_features_future.result())
|
||||
webview_origin_trial_features = (
|
||||
webview_origin_trial_features_future.result())
|
||||
desktop_dev_trial_features = desktop_dev_trial_features_future.result()
|
||||
android_dev_trial_features = android_dev_trial_features_future.result()
|
||||
|
||||
# Push feature to list corresponding to the implementation status of
|
||||
# feature in queried milestone
|
||||
for feature in desktop_shipping_features:
|
||||
if feature.impl_status_chrome == ENABLED_BY_DEFAULT:
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]].append(feature)
|
||||
elif feature.impl_status_chrome == DEPRECATED:
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]].append(feature)
|
||||
elif feature.impl_status_chrome == REMOVED:
|
||||
all_features[IMPLEMENTATION_STATUS[REMOVED]].append(feature)
|
||||
elif feature.impl_status_chrome == INTERVENTION:
|
||||
all_features[IMPLEMENTATION_STATUS[INTERVENTION]].append(feature)
|
||||
elif (feature.feature_type == FEATURE_TYPE_DEPRECATION_ID and
|
||||
Feature.dt_milestone_desktop_start != None):
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]].append(feature)
|
||||
elif feature.feature_type == FEATURE_TYPE_INCUBATE_ID:
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]].append(feature)
|
||||
|
||||
# Push feature to list corresponding to the implementation status
|
||||
# of feature in queried milestone
|
||||
for feature in android_only_shipping_features:
|
||||
if feature.impl_status_chrome == ENABLED_BY_DEFAULT:
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]].append(feature)
|
||||
elif feature.impl_status_chrome == DEPRECATED:
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]].append(feature)
|
||||
elif feature.impl_status_chrome == REMOVED:
|
||||
all_features[IMPLEMENTATION_STATUS[REMOVED]].append(feature)
|
||||
elif (feature.feature_type == FEATURE_TYPE_DEPRECATION_ID and
|
||||
Feature.dt_milestone_android_start != None):
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]].append(feature)
|
||||
elif feature.feature_type == FEATURE_TYPE_INCUBATE_ID:
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]].append(feature)
|
||||
|
||||
for feature in desktop_origin_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[ORIGIN_TRIAL]].append(feature)
|
||||
|
||||
for feature in android_origin_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[ORIGIN_TRIAL]].append(feature)
|
||||
|
||||
for feature in webview_origin_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[ORIGIN_TRIAL]].append(feature)
|
||||
|
||||
for feature in desktop_dev_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[BEHIND_A_FLAG]].append(feature)
|
||||
|
||||
for feature in android_dev_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[BEHIND_A_FLAG]].append(feature)
|
||||
|
||||
features_by_type = {}
|
||||
allowed_features_by_type = {}
|
||||
cache_key = '%s|%s|%s' % (
|
||||
Feature.DEFAULT_CACHE_KEY, 'milestone', milestone)
|
||||
cached_features_by_type = ramcache.get(cache_key)
|
||||
if cached_features_by_type:
|
||||
features_by_type = cached_features_by_type
|
||||
else:
|
||||
all_features = {}
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[REMOVED]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[INTERVENTION]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[ORIGIN_TRIAL]] = []
|
||||
all_features[IMPLEMENTATION_STATUS[BEHIND_A_FLAG]] = []
|
||||
|
||||
# Construct results as: {type: [json_feature, ...], ...}.
|
||||
for shippingType in all_features:
|
||||
all_features[shippingType].sort(key=lambda f: f.name)
|
||||
all_features[shippingType] = [
|
||||
f for f in all_features[shippingType] if not f.deleted]
|
||||
features_by_type[shippingType] = [
|
||||
f.format_for_template() for f in all_features[shippingType]]
|
||||
allowed_features_by_type[shippingType] = [
|
||||
f for f in features_by_type[shippingType]
|
||||
if show_unlisted or not f['unlisted']]
|
||||
logging.info('Getting chronological feature list in milestone %d',
|
||||
milestone)
|
||||
# Start each query asynchronously in parallel.
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.shipped_milestone == milestone)
|
||||
desktop_shipping_features_future = q.fetch_async(None)
|
||||
|
||||
ramcache.set(cache_key, allowed_features_by_type)
|
||||
# Features with an android shipping milestone but no desktop milestone.
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.shipped_android_milestone == milestone)
|
||||
q = q.filter(Feature.shipped_milestone == None)
|
||||
android_only_shipping_features_future = q.fetch_async(None)
|
||||
|
||||
return allowed_features_by_type
|
||||
# Features that are in origin trial (Desktop) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.ot_milestone_desktop_start == milestone)
|
||||
desktop_origin_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in origin trial (Android) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.ot_milestone_android_start == milestone)
|
||||
q = q.filter(Feature.ot_milestone_desktop_start == None)
|
||||
android_origin_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in origin trial (Webview) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.ot_milestone_webview_start == milestone)
|
||||
q = q.filter(Feature.ot_milestone_desktop_start == None)
|
||||
webview_origin_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in dev trial (Desktop) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.dt_milestone_desktop_start == milestone)
|
||||
desktop_dev_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Features that are in dev trial (Android) in this milestone
|
||||
q = Feature.query()
|
||||
q = q.order(Feature.name)
|
||||
q = q.filter(Feature.dt_milestone_android_start == milestone)
|
||||
q = q.filter(Feature.dt_milestone_desktop_start == None)
|
||||
android_dev_trial_features_future = q.fetch_async(None)
|
||||
|
||||
# Wait for all futures to complete.
|
||||
desktop_shipping_features = desktop_shipping_features_future.result()
|
||||
android_only_shipping_features = (
|
||||
android_only_shipping_features_future.result())
|
||||
desktop_origin_trial_features = (
|
||||
desktop_origin_trial_features_future.result())
|
||||
android_origin_trial_features = (
|
||||
android_origin_trial_features_future.result())
|
||||
webview_origin_trial_features = (
|
||||
webview_origin_trial_features_future.result())
|
||||
desktop_dev_trial_features = desktop_dev_trial_features_future.result()
|
||||
android_dev_trial_features = android_dev_trial_features_future.result()
|
||||
|
||||
# Push feature to list corresponding to the implementation status of
|
||||
# feature in queried milestone
|
||||
for feature in desktop_shipping_features:
|
||||
if feature.impl_status_chrome == ENABLED_BY_DEFAULT:
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]].append(feature)
|
||||
elif feature.impl_status_chrome == DEPRECATED:
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]].append(feature)
|
||||
elif feature.impl_status_chrome == REMOVED:
|
||||
all_features[IMPLEMENTATION_STATUS[REMOVED]].append(feature)
|
||||
elif feature.impl_status_chrome == INTERVENTION:
|
||||
all_features[IMPLEMENTATION_STATUS[INTERVENTION]].append(feature)
|
||||
elif (feature.feature_type == FEATURE_TYPE_DEPRECATION_ID and
|
||||
Feature.dt_milestone_desktop_start != None):
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]].append(feature)
|
||||
elif feature.feature_type == FEATURE_TYPE_INCUBATE_ID:
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]].append(feature)
|
||||
|
||||
# Push feature to list corresponding to the implementation status
|
||||
# of feature in queried milestone
|
||||
for feature in android_only_shipping_features:
|
||||
if feature.impl_status_chrome == ENABLED_BY_DEFAULT:
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]].append(feature)
|
||||
elif feature.impl_status_chrome == DEPRECATED:
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]].append(feature)
|
||||
elif feature.impl_status_chrome == REMOVED:
|
||||
all_features[IMPLEMENTATION_STATUS[REMOVED]].append(feature)
|
||||
elif (feature.feature_type == FEATURE_TYPE_DEPRECATION_ID and
|
||||
Feature.dt_milestone_android_start != None):
|
||||
all_features[IMPLEMENTATION_STATUS[DEPRECATED]].append(feature)
|
||||
elif feature.feature_type == FEATURE_TYPE_INCUBATE_ID:
|
||||
all_features[IMPLEMENTATION_STATUS[ENABLED_BY_DEFAULT]].append(feature)
|
||||
|
||||
for feature in desktop_origin_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[ORIGIN_TRIAL]].append(feature)
|
||||
|
||||
for feature in android_origin_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[ORIGIN_TRIAL]].append(feature)
|
||||
|
||||
for feature in webview_origin_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[ORIGIN_TRIAL]].append(feature)
|
||||
|
||||
for feature in desktop_dev_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[BEHIND_A_FLAG]].append(feature)
|
||||
|
||||
for feature in android_dev_trial_features:
|
||||
all_features[IMPLEMENTATION_STATUS[BEHIND_A_FLAG]].append(feature)
|
||||
|
||||
# Construct results as: {type: [json_feature, ...], ...}.
|
||||
for shipping_type in all_features:
|
||||
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] = [
|
||||
f.format_for_template() for f in all_features[shipping_type]]
|
||||
|
||||
ramcache.set(cache_key, features_by_type)
|
||||
|
||||
for shipping_type in features_by_type:
|
||||
if not show_unlisted:
|
||||
features_by_type[shipping_type] = self.filter_unlisted(features_by_type[shipping_type])
|
||||
else:
|
||||
features_by_type[shipping_type] = all_features[shipping_type].copy()
|
||||
|
||||
|
||||
return features_by_type
|
||||
|
||||
def crbug_number(self):
|
||||
if not self.bug_url:
|
||||
|
|
@ -1249,6 +1273,7 @@ class Feature(DictModel):
|
|||
search_tags = ndb.StringProperty(repeated=True)
|
||||
comments = ndb.StringProperty()
|
||||
owner = ndb.StringProperty(repeated=True)
|
||||
editors = ndb.StringProperty(repeated=True)
|
||||
footprint = ndb.IntegerProperty() # Deprecated
|
||||
|
||||
# Tracability to intent discussion threads
|
||||
|
|
@ -1380,6 +1405,7 @@ QUERIABLE_FIELDS = {
|
|||
'tags': Feature.search_tags,
|
||||
'owner': Feature.owner,
|
||||
'browsers.chrome.owners': Feature.owner,
|
||||
'editors': Feature.editors,
|
||||
'intent_to_implement_url': Feature.intent_to_implement_url,
|
||||
'intent_to_ship_url': Feature.intent_to_ship_url,
|
||||
'ready_for_trial_url': Feature.ready_for_trial_url,
|
||||
|
|
@ -1656,6 +1682,7 @@ class AppUser(DictModel):
|
|||
|
||||
email = ndb.StringProperty(required=True)
|
||||
is_admin = ndb.BooleanProperty(default=False)
|
||||
is_site_editor = ndb.BooleanProperty(default=False)
|
||||
created = ndb.DateTimeProperty(auto_now_add=True)
|
||||
updated = ndb.DateTimeProperty(auto_now=True)
|
||||
|
||||
|
|
|
|||
|
|
@ -83,23 +83,27 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
def setUp(self):
|
||||
ramcache.SharedInvalidate.check_for_distributed_invalidation()
|
||||
self.feature_2 = models.Feature(
|
||||
name='feature b', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=3)
|
||||
name='feature b', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=3)
|
||||
self.feature_2.put()
|
||||
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature a', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=3)
|
||||
name='feature a', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=3)
|
||||
self.feature_1.put()
|
||||
|
||||
self.feature_4 = models.Feature(
|
||||
name='feature d', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=2)
|
||||
name='feature d', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=2)
|
||||
self.feature_4.put()
|
||||
|
||||
self.feature_3 = models.Feature(
|
||||
name='feature c', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=2)
|
||||
name='feature c', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=2)
|
||||
self.feature_3.put()
|
||||
|
||||
def tearDown(self):
|
||||
|
|
@ -160,6 +164,30 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
self.assertEqual(
|
||||
['feature a'],
|
||||
names)
|
||||
|
||||
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.put()
|
||||
testing_config.sign_in('feature_owner@example.com', 1234567890)
|
||||
actual = models.Feature.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)
|
||||
|
||||
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.put()
|
||||
testing_config.sign_in("feature_editor@example.com", 1234567890)
|
||||
actual = models.Feature.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)
|
||||
|
||||
def test_get_by_ids__empty(self):
|
||||
"""A request to load zero features returns zero results."""
|
||||
|
|
@ -190,12 +218,17 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
ramcache.global_cache.clear()
|
||||
cache_key = '%s|%s' % (
|
||||
models.Feature.DEFAULT_CACHE_KEY, self.feature_1.key.integer_id())
|
||||
ramcache.set(cache_key, 'fake cached feature')
|
||||
cached_feature = {
|
||||
'name': 'fake cached_feature',
|
||||
'id': self.feature_1.key.integer_id(),
|
||||
'unlisted': False
|
||||
}
|
||||
ramcache.set(cache_key, cached_feature)
|
||||
|
||||
actual = models.Feature.get_by_ids([self.feature_1.key.integer_id()])
|
||||
|
||||
self.assertEqual(1, len(actual))
|
||||
self.assertEqual('fake cached feature', actual[0])
|
||||
self.assertEqual(cached_feature, actual[0])
|
||||
|
||||
def test_get_by_ids__batch_order(self):
|
||||
"""Features are returned in the order of the given IDs."""
|
||||
|
|
@ -303,8 +336,8 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
enabled_by_default)
|
||||
self.assertEqual(6, len(actual))
|
||||
|
||||
cache_key = '%s|%s|%s|%s' % (
|
||||
models.Feature.DEFAULT_CACHE_KEY, 'milestone', False, 1)
|
||||
cache_key = '%s|%s|%s' % (
|
||||
models.Feature.DEFAULT_CACHE_KEY, 'milestone', 1)
|
||||
cached_result = ramcache.get(cache_key)
|
||||
self.assertEqual(cached_result, actual)
|
||||
|
||||
|
|
@ -359,13 +392,14 @@ class FeatureTest(testing_config.CustomTestCase):
|
|||
|
||||
def test_get_in_milestone__cached(self):
|
||||
"""If there is something in the cache, we use it."""
|
||||
cache_key = '%s|%s|%s|%s' % (
|
||||
models.Feature.DEFAULT_CACHE_KEY, 'milestone', False, 1)
|
||||
ramcache.set(cache_key, 'fake feature dict')
|
||||
cache_key = '%s|%s|%s' % (
|
||||
models.Feature.DEFAULT_CACHE_KEY, 'milestone', 1)
|
||||
cached_test_feature = {'test': [{'name': 'test_feature', 'unlisted': False}]}
|
||||
ramcache.set(cache_key, cached_test_feature)
|
||||
|
||||
actual = models.Feature.get_in_milestone(milestone=1)
|
||||
self.assertEqual(
|
||||
'fake feature dict',
|
||||
cached_test_feature,
|
||||
actual)
|
||||
|
||||
|
||||
|
|
@ -466,8 +500,9 @@ class CommentTest(testing_config.CustomTestCase):
|
|||
|
||||
def setUp(self):
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature a', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=3)
|
||||
name='feature a', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=3)
|
||||
self.feature_1.put()
|
||||
self.feature_1_id = self.feature_1.key.integer_id()
|
||||
self.comment_1_1 = models.Comment(
|
||||
|
|
@ -482,8 +517,9 @@ class CommentTest(testing_config.CustomTestCase):
|
|||
self.comment_1_2.put()
|
||||
|
||||
self.feature_2 = models.Feature(
|
||||
name='feature b', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=3)
|
||||
name='feature b', summary='sum', owner=['feature_owner@example.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=3)
|
||||
self.feature_2.put()
|
||||
self.feature_2_id = self.feature_2.key.integer_id()
|
||||
|
||||
|
|
|
|||
|
|
@ -22,7 +22,6 @@ from internals import approval_defs
|
|||
from internals import models
|
||||
from internals import notifier
|
||||
|
||||
|
||||
PENDING_STATES = [
|
||||
models.Approval.REVIEW_REQUESTED, models.Approval.REVIEW_STARTED,
|
||||
models.Approval.NEED_INFO]
|
||||
|
|
@ -69,12 +68,13 @@ def process_starred_me_query():
|
|||
return feature_ids
|
||||
|
||||
|
||||
def process_owner_me_query():
|
||||
"""Return features that the current user owns."""
|
||||
def process_access_me_query(field):
|
||||
"""Return features that the current user owns or can edit."""
|
||||
user = users.get_current_user()
|
||||
if not user:
|
||||
return []
|
||||
features = models.Feature.get_all(filterby=('owner', user.email()))
|
||||
# Checks if the user's email exists in the given field.
|
||||
features = models.Feature.get_all(filterby=(field, user.email()))
|
||||
feature_ids = [f['id'] for f in features]
|
||||
return feature_ids
|
||||
|
||||
|
|
@ -150,11 +150,18 @@ def process_query_term(field_name, op_str, val_str):
|
|||
return process_pending_approval_me_query()
|
||||
if query_term == 'starred-by:me':
|
||||
return process_starred_me_query()
|
||||
if query_term == 'owner:me':
|
||||
return process_owner_me_query()
|
||||
if query_term == 'is:recently-reviewed':
|
||||
return process_recent_reviews_query()
|
||||
|
||||
# These queries can display unlisted features if the users
|
||||
# has edit access to them. Also return a flag to signal this.
|
||||
if query_term == 'owner:me':
|
||||
return process_access_me_query('owner')
|
||||
if query_term == 'editor:me':
|
||||
return process_access_me_query('editors')
|
||||
if query_term == 'can_edit:me':
|
||||
return process_access_me_query('can_edit')
|
||||
|
||||
if val_str.startswith('"') and val_str.endswith('"'):
|
||||
val_str = val_str[1:-1]
|
||||
return process_queriable_field(field_name, op_str, val_str)
|
||||
|
|
@ -184,6 +191,7 @@ def process_query(
|
|||
terms = TERM_RE.findall(user_query + ' ')[:MAX_TERMS] or []
|
||||
if not show_deleted:
|
||||
terms.append(('deleted', '=', 'false', None))
|
||||
# TODO(jrobbins): include unlisted features that the user is allowed to view.
|
||||
if not show_unlisted:
|
||||
terms.append(('unlisted', '=', 'false', None))
|
||||
# 1b. Parse the sort directive.
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ class SearchFunctionsTest(testing_config.CustomTestCase):
|
|||
name='feature 1', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=3)
|
||||
self.feature_1.owner = ['owner@example.com']
|
||||
self.feature_1.editors = ['editor@example.com']
|
||||
self.feature_1.put()
|
||||
self.feature_2 = models.Feature(
|
||||
name='feature 2', summary='sum', category=2, visibility=1,
|
||||
|
|
@ -121,20 +122,33 @@ class SearchFunctionsTest(testing_config.CustomTestCase):
|
|||
self.assertEqual(len(actual), 1)
|
||||
self.assertEqual(actual[0], self.feature_1.key.integer_id())
|
||||
|
||||
def test_process_owner_me_query__none(self):
|
||||
def test_process_access_me_query__owner_none(self):
|
||||
"""We can return a list of features owned by the user."""
|
||||
testing_config.sign_in('visitor@example.com', 111)
|
||||
actual = search.process_owner_me_query()
|
||||
actual = search.process_access_me_query('owner')
|
||||
self.assertEqual(actual, [])
|
||||
|
||||
def test_process_owner_me_query__some(self):
|
||||
def test_process_access_me_query__owner_some(self):
|
||||
"""We can return a list of features owned by the user."""
|
||||
testing_config.sign_in('owner@example.com', 111)
|
||||
actual = search.process_owner_me_query()
|
||||
actual = search.process_access_me_query('owner')
|
||||
self.assertEqual(len(actual), 2)
|
||||
self.assertEqual(actual[0], self.feature_1.key.integer_id())
|
||||
self.assertEqual(actual[1], self.feature_2.key.integer_id())
|
||||
|
||||
def test_process_access_me_query__editors_none(self):
|
||||
"""We can return a list of features the user can edit."""
|
||||
testing_config.sign_in('visitor@example.com', 111)
|
||||
actual = search.process_access_me_query('editors')
|
||||
self.assertEqual(actual, [])
|
||||
|
||||
def test_process_access_me_query__editors_some(self):
|
||||
"""We can return a list of features the user can edit."""
|
||||
testing_config.sign_in('editor@example.com', 111)
|
||||
actual = search.process_access_me_query('editors')
|
||||
self.assertEqual(len(actual), 1)
|
||||
self.assertEqual(actual[0], self.feature_1.key.integer_id())
|
||||
|
||||
@mock.patch('internals.models.Approval.get_approvals')
|
||||
@mock.patch('internals.approval_defs.fields_approvable_by')
|
||||
def test_process_recent_reviews_query__none(
|
||||
|
|
@ -172,7 +186,7 @@ class SearchFunctionsTest(testing_config.CustomTestCase):
|
|||
|
||||
@mock.patch('internals.search.process_pending_approval_me_query')
|
||||
@mock.patch('internals.search.process_starred_me_query')
|
||||
@mock.patch('internals.search.process_owner_me_query')
|
||||
@mock.patch('internals.search.process_access_me_query')
|
||||
@mock.patch('internals.search.process_recent_reviews_query')
|
||||
def test_process_query__predefined(
|
||||
self, mock_recent, mock_own_me, mock_star_me, mock_pend_me):
|
||||
|
|
|
|||
|
|
@ -35,10 +35,17 @@ class TestWithFeature(testing_config.CustomTestCase):
|
|||
HANDLER_CLASS = 'subclasses fill this in'
|
||||
|
||||
def setUp(self):
|
||||
self.app_user = models.AppUser(email='registered@example.com')
|
||||
self.app_user.put()
|
||||
|
||||
self.app_admin = models.AppUser(email='admin@example.com')
|
||||
self.app_admin.is_admin = True
|
||||
self.app_admin.put()
|
||||
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='detailed sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=1,
|
||||
intent_stage=models.INTENT_IMPLEMENT)
|
||||
name='feature one', summary='detailed sum', owner=['owner@example.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=1, intent_stage=models.INTENT_IMPLEMENT)
|
||||
self.feature_1.put()
|
||||
self.feature_id = self.feature_1.key.integer_id()
|
||||
|
||||
|
|
@ -49,6 +56,9 @@ class TestWithFeature(testing_config.CustomTestCase):
|
|||
|
||||
def tearDown(self):
|
||||
self.feature_1.key.delete()
|
||||
self.app_user.delete()
|
||||
self.app_admin.delete()
|
||||
|
||||
ramcache.flush_all()
|
||||
ramcache.check_for_distributed_invalidation()
|
||||
|
||||
|
|
@ -83,11 +93,11 @@ class FeaturesJsonHandlerTest(TestWithFeature):
|
|||
self.assertEqual(0, len(json_data))
|
||||
|
||||
def test_get_template_data__unlisted_can_edit(self):
|
||||
"""JSON feed includes unlisted features for users who may edit."""
|
||||
"""JSON feed includes unlisted features for site editors and admins."""
|
||||
self.feature_1.unlisted = True
|
||||
self.feature_1.put()
|
||||
|
||||
testing_config.sign_in('user@google.com', 111)
|
||||
testing_config.sign_in('admin@example.com', 111)
|
||||
with test_app.test_request_context(self.request_path):
|
||||
json_data = self.handler.get_template_data()
|
||||
self.assertEqual(1, len(json_data))
|
||||
|
|
|
|||
|
|
@ -107,7 +107,7 @@ class FeatureNew(basehandlers.FlaskHandler):
|
|||
|
||||
TEMPLATE_PATH = 'guide/new.html'
|
||||
|
||||
@permissions.require_edit_feature
|
||||
@permissions.require_create_feature
|
||||
def get_template_data(self):
|
||||
user = self.get_current_user()
|
||||
|
||||
|
|
@ -118,9 +118,10 @@ class FeatureNew(basehandlers.FlaskHandler):
|
|||
}
|
||||
return template_data
|
||||
|
||||
@permissions.require_edit_feature
|
||||
@permissions.require_create_feature
|
||||
def process_post_data(self):
|
||||
owners = self.split_emails('owner')
|
||||
editors = self.split_emails('editors')
|
||||
|
||||
blink_components = (
|
||||
self.split_input('blink_components', delim=',') or
|
||||
|
|
@ -139,6 +140,7 @@ class FeatureNew(basehandlers.FlaskHandler):
|
|||
intent_stage=models.INTENT_NONE,
|
||||
summary=self.form.get('summary'),
|
||||
owner=owners,
|
||||
editors=editors,
|
||||
impl_status_chrome=models.NO_ACTIVE_DEV,
|
||||
standardization=models.EDITORS_DRAFT,
|
||||
unlisted=self.form.get('unlisted') == 'on',
|
||||
|
|
@ -170,6 +172,7 @@ class ProcessOverview(basehandlers.FlaskHandler):
|
|||
|
||||
@permissions.require_edit_feature
|
||||
def get_template_data(self, feature_id):
|
||||
|
||||
f = models.Feature.get_by_id(int(feature_id))
|
||||
if f is None:
|
||||
self.abort(404, msg='Feature not found')
|
||||
|
|
@ -246,6 +249,7 @@ class FeatureEditStage(basehandlers.FlaskHandler):
|
|||
|
||||
@permissions.require_edit_feature
|
||||
def get_template_data(self, feature_id, stage_id):
|
||||
|
||||
f, feature_process = self.get_feature_and_process(feature_id)
|
||||
|
||||
stage_name = ''
|
||||
|
|
@ -289,6 +293,7 @@ class FeatureEditStage(basehandlers.FlaskHandler):
|
|||
|
||||
@permissions.require_edit_feature
|
||||
def process_post_data(self, feature_id, stage_id=0):
|
||||
|
||||
if feature_id:
|
||||
feature = models.Feature.get_by_id(feature_id)
|
||||
if feature is None:
|
||||
|
|
@ -438,6 +443,9 @@ class FeatureEditStage(basehandlers.FlaskHandler):
|
|||
|
||||
if self.touched('owner'):
|
||||
feature.owner = self.split_emails('owner')
|
||||
|
||||
if self.touched('editors'):
|
||||
feature.editors = self.split_emails('editors')
|
||||
|
||||
if self.touched('doc_links'):
|
||||
feature.doc_links = self.parse_links('doc_links')
|
||||
|
|
@ -579,6 +587,7 @@ class FeatureEditAllFields(FeatureEditStage):
|
|||
|
||||
@permissions.require_edit_feature
|
||||
def get_template_data(self, feature_id):
|
||||
|
||||
f, feature_process = self.get_feature_and_process(feature_id)
|
||||
|
||||
feature_edit_dict = f.format_for_edit()
|
||||
|
|
|
|||
|
|
@ -137,9 +137,9 @@ class ProcessOverviewTest(testing_config.CustomTestCase):
|
|||
|
||||
def setUp(self):
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=models.DEV_NO_SIGNALS,
|
||||
impl_status_chrome=1)
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
category=1, visibility=1, standardization=1,
|
||||
web_dev_views=models.DEV_NO_SIGNALS, impl_status_chrome=1)
|
||||
self.feature_1.put()
|
||||
|
||||
self.request_path = '/guide/edit/%d' % self.feature_1.key.integer_id()
|
||||
|
|
@ -209,9 +209,9 @@ class ProcessOverviewTemplateTest(TestWithFeature):
|
|||
super(ProcessOverviewTemplateTest, self).setUp()
|
||||
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=models.DEV_NO_SIGNALS,
|
||||
impl_status_chrome=1)
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
category=1, visibility=1, standardization=1,
|
||||
web_dev_views=models.DEV_NO_SIGNALS, impl_status_chrome=1)
|
||||
self.feature_1.put()
|
||||
self.request_path = '/guide/edit/%d' % self.feature_1.key.integer_id()
|
||||
|
||||
|
|
@ -243,8 +243,9 @@ class FeatureEditStageTest(testing_config.CustomTestCase):
|
|||
|
||||
def setUp(self):
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=1)
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=1)
|
||||
self.feature_1.put()
|
||||
self.stage = models.INTENT_INCUBATE # Shows first form
|
||||
|
||||
|
|
@ -394,9 +395,9 @@ class FeatureEditStageTemplateTest(TestWithFeature):
|
|||
def setUp(self):
|
||||
super(FeatureEditStageTemplateTest, self).setUp()
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=models.DEV_NO_SIGNALS,
|
||||
impl_status_chrome=1)
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
category=1, visibility=1, standardization=1,
|
||||
web_dev_views=models.DEV_NO_SIGNALS, impl_status_chrome=1)
|
||||
self.feature_1.put()
|
||||
self.stage = models.INTENT_INCUBATE # Shows first form
|
||||
testing_config.sign_in('user1@google.com', 1234567890)
|
||||
|
|
@ -426,9 +427,9 @@ class FeatureEditAllFieldsTemplateTest(TestWithFeature):
|
|||
def setUp(self):
|
||||
super(FeatureEditAllFieldsTemplateTest, self).setUp()
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=models.DEV_NO_SIGNALS,
|
||||
impl_status_chrome=1)
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
category=1, visibility=1, standardization=1,
|
||||
web_dev_views=models.DEV_NO_SIGNALS, impl_status_chrome=1)
|
||||
self.feature_1.put()
|
||||
self.stage = models.INTENT_INCUBATE # Shows first form
|
||||
testing_config.sign_in('user1@google.com', 1234567890)
|
||||
|
|
|
|||
|
|
@ -144,6 +144,10 @@ ALL_FIELDS = {
|
|||
required=True, label='Feature owners',
|
||||
widget=forms.EmailInput(attrs=MULTI_EMAIL_FIELD_ATTRS)),
|
||||
|
||||
'editors': MultiEmailField(
|
||||
required=False, label='Feature editors',
|
||||
widget=forms.EmailInput(attrs=MULTI_EMAIL_FIELD_ATTRS)),
|
||||
|
||||
'category': forms.ChoiceField(
|
||||
required=False,
|
||||
initial=models.MISC,
|
||||
|
|
@ -502,7 +506,7 @@ ALL_FIELDS = {
|
|||
# These are shown in a top card for all processes.
|
||||
METADATA_FIELDS = [
|
||||
'name', 'summary', 'unlisted', 'owner',
|
||||
'category',
|
||||
'editors', 'category',
|
||||
'feature_type', 'intent_stage',
|
||||
'search_tags',
|
||||
# Implemention
|
||||
|
|
@ -576,7 +580,7 @@ def define_form_class_using_shared_fields(class_name, field_spec_list):
|
|||
NewFeatureForm = define_form_class_using_shared_fields(
|
||||
'NewFeatureForm',
|
||||
('name', 'summary',
|
||||
'unlisted', 'owner',
|
||||
'unlisted', 'owner', 'editors',
|
||||
'blink_components', 'category'))
|
||||
# Note: feature_type is done with custom HTML
|
||||
|
||||
|
|
@ -681,7 +685,7 @@ Any_Ship = define_form_class_using_shared_fields(
|
|||
|
||||
Existing_Prototype = define_form_class_using_shared_fields(
|
||||
'Existing_Prototype',
|
||||
('owner', 'blink_components', 'motivation', 'explainer_links',
|
||||
('owner', 'editors', 'blink_components', 'motivation', 'explainer_links',
|
||||
'spec_link', 'standard_maturity', 'api_spec', 'bug_url', 'launch_bug_url',
|
||||
'intent_to_implement_url', 'comments'))
|
||||
|
||||
|
|
@ -753,7 +757,7 @@ Flat_Metadata = define_form_class_using_shared_fields(
|
|||
'Flat_Metadata',
|
||||
(# Standardizaton
|
||||
'name', 'summary', 'unlisted', 'owner',
|
||||
'category',
|
||||
'editors', 'category',
|
||||
'feature_type', 'intent_stage',
|
||||
'search_tags',
|
||||
# Implementtion
|
||||
|
|
@ -880,7 +884,7 @@ DEPRECATED_FIELDS = ['standardization']
|
|||
DISPLAY_IN_FEATURE_HIGHLIGHTS = [
|
||||
'name', 'summary',
|
||||
'motivation', 'deprecation_motivation',
|
||||
'unlisted', 'owner',
|
||||
'unlisted', 'owner', 'editors',
|
||||
'search_tags',
|
||||
# Implementtion
|
||||
'impl_status_chrome',
|
||||
|
|
|
|||
|
|
@ -31,9 +31,9 @@ class IntentEmailPreviewHandlerTest(testing_config.CustomTestCase):
|
|||
|
||||
def setUp(self):
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=1,
|
||||
intent_stage=models.INTENT_IMPLEMENT)
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=1, intent_stage=models.INTENT_IMPLEMENT)
|
||||
self.feature_1.put()
|
||||
|
||||
self.request_path = '/admin/features/launch/%d/%d?intent' % (
|
||||
|
|
@ -187,9 +187,9 @@ class IntentEmailPreviewTemplateTest(testing_config.CustomTestCase):
|
|||
def setUp(self):
|
||||
super(IntentEmailPreviewTemplateTest, self).setUp()
|
||||
self.feature_1 = models.Feature(
|
||||
name='feature one', summary='sum', category=1, visibility=1,
|
||||
standardization=1, web_dev_views=1, impl_status_chrome=1,
|
||||
intent_stage=models.INTENT_IMPLEMENT)
|
||||
name='feature one', summary='sum', owner=['user1@google.com'],
|
||||
category=1, visibility=1, standardization=1, web_dev_views=1,
|
||||
impl_status_chrome=1, intent_stage=models.INTENT_IMPLEMENT)
|
||||
self.feature_1.put()
|
||||
|
||||
self.request_path = '/admin/features/launch/%d/%d?intent' % (
|
||||
|
|
|
|||
|
|
@ -69,6 +69,7 @@ export class ChromedashFeaturePage extends LitElement {
|
|||
user: {type: Object},
|
||||
featureId: {type: Number},
|
||||
feature: {type: Object},
|
||||
editableFeatures: {type: Object},
|
||||
process: {type: Object},
|
||||
fieldDefs: {type: Object},
|
||||
dismissedCues: {type: Array},
|
||||
|
|
@ -82,6 +83,7 @@ export class ChromedashFeaturePage extends LitElement {
|
|||
super();
|
||||
this.user = {};
|
||||
this.featureId = 0;
|
||||
this.editableFeatures = [];
|
||||
this.feature = {};
|
||||
this.process = {};
|
||||
this.fieldDefs = {};
|
||||
|
|
@ -193,6 +195,9 @@ export class ChromedashFeaturePage extends LitElement {
|
|||
}
|
||||
|
||||
renderSubHeader() {
|
||||
const canEdit = (this.user &&
|
||||
(this.user.can_edit_all || this.user.editableFeatures.includes(this.featureId)));
|
||||
|
||||
return html`
|
||||
<div id="subheader" style="display:block">
|
||||
<div class="tooltips" style="float:right">
|
||||
|
|
@ -225,7 +230,7 @@ export class ChromedashFeaturePage extends LitElement {
|
|||
</a>
|
||||
</span>
|
||||
`: nothing}
|
||||
${this.user && this.user.can_edit ? html`
|
||||
${canEdit ? html`
|
||||
<span class="tooltip" title="Edit this feature">
|
||||
<a href="/guide/edit/${this.featureId}" class="editfeature" data-tooltip>
|
||||
<iron-icon icon="chromestatus:create"></iron-icon>
|
||||
|
|
@ -252,8 +257,8 @@ export class ChromedashFeaturePage extends LitElement {
|
|||
return html`
|
||||
${this.feature.unlisted ? html`
|
||||
<section id="access">
|
||||
<p><b>This feature is only shown in the feature list to users with
|
||||
edit access.</b></p>
|
||||
<p><b>This feature is only shown in the feature list
|
||||
to users with access to edit this feature.</b></p>
|
||||
</section>
|
||||
`: nothing}
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ describe('chromedash-feature-page', () => {
|
|||
const permissionsPromise = Promise.resolve({
|
||||
can_approve: false,
|
||||
can_create_feature: true,
|
||||
can_edit: true,
|
||||
can_edit_all: true,
|
||||
is_admin: false,
|
||||
email: 'example@google.com',
|
||||
});
|
||||
|
|
|
|||
|
|
@ -295,7 +295,7 @@ class ChromedashFeature extends LitElement {
|
|||
<summary>
|
||||
${this.feature.unlisted ?
|
||||
html`<p><b>This feature is only shown in the feature list
|
||||
to users with edit access.</b></p>
|
||||
to users with access to edit this feature.</b></p>
|
||||
`: nothing }
|
||||
<p class="${this.open ? 'preformatted' : ''}"
|
||||
><span>${autolink(this.feature.summary)}</span
|
||||
|
|
|
|||
|
|
@ -11,9 +11,10 @@ class ChromedashFeaturelist extends LitElement {
|
|||
|
||||
static get properties() {
|
||||
return {
|
||||
canEdit: {type: Boolean},
|
||||
isSiteEditor: {type: Boolean},
|
||||
canApprove: {type: Boolean},
|
||||
signedInUser: {type: String},
|
||||
editableFeatures: {type: Object},
|
||||
features: {attribute: false}, // Directly edited and accessed in template/features.html
|
||||
metadataEl: {attribute: false}, // The metadata component element. Directly edited in template/features.html
|
||||
searchEl: {attribute: false}, // The search input element. Directly edited in template/features.html
|
||||
|
|
@ -26,10 +27,11 @@ class ChromedashFeaturelist extends LitElement {
|
|||
constructor() {
|
||||
super();
|
||||
this.features = [];
|
||||
this.editableFeatures = [];
|
||||
this.filtered = [];
|
||||
this.metadataEl = document.querySelector('chromedash-metadata');
|
||||
this.searchEl = document.querySelector('.search input');
|
||||
this.canEdit = false;
|
||||
this.isSiteEditor = false;
|
||||
this.canApprove = false;
|
||||
this.signedInUser = '';
|
||||
this._hasInitialized = false; // Used to check initialization code.
|
||||
|
|
@ -363,10 +365,13 @@ class ChromedashFeaturelist extends LitElement {
|
|||
render() {
|
||||
// TODO: Avoid computing values in render().
|
||||
let filteredWithState = this.filtered.map((feature) => {
|
||||
const editable = this.isSiteEditor ||
|
||||
(this.editableFeatures && this.editableFeatures.includes(feature.id));
|
||||
return {
|
||||
feature: feature,
|
||||
open: this.openFeatures.has(feature.id),
|
||||
starred: this.starredFeatures.has(feature.id),
|
||||
canEditFeature: editable,
|
||||
};
|
||||
});
|
||||
let numOverLimit = 0;
|
||||
|
|
@ -391,7 +396,7 @@ class ChromedashFeaturelist extends LitElement {
|
|||
@star-toggled="${this._onStarToggledBound}"
|
||||
@open-approvals-event="${this._onOpenApprovalsBound}"
|
||||
.feature="${item.feature}"
|
||||
?canEdit="${this.canEdit}"
|
||||
?canEdit="${item.canEditFeature}"
|
||||
?canApprove="${this.canApprove}"
|
||||
?signedIn="${this.signedInUser != ''}"
|
||||
></chromedash-feature>
|
||||
|
|
|
|||
|
|
@ -81,7 +81,7 @@ export class ChromedashMyFeaturesPage extends LitElement {
|
|||
<chromedash-feature-table
|
||||
query="${query}"
|
||||
?signedIn=${Boolean(this.user)}
|
||||
?canEdit=${this.user && this.user.can_edit}
|
||||
?canEdit=${this.user && this.user.can_edit_all}
|
||||
?canApprove=${this.user && this.user.can_approve}
|
||||
.starredFeatures=${this.starredFeatures}
|
||||
@star-toggle-event=${this.handleStarToggle}
|
||||
|
|
@ -105,9 +105,9 @@ export class ChromedashMyFeaturesPage extends LitElement {
|
|||
'Features I starred', 'starred-by:me', 'normal');
|
||||
}
|
||||
|
||||
renderIOwn() {
|
||||
renderICanEdit() {
|
||||
return this.renderBox(
|
||||
'Features I own', 'owner:me', 'normal');
|
||||
'Features I can edit', 'can_edit:me', 'normal');
|
||||
}
|
||||
|
||||
render() {
|
||||
|
|
@ -116,7 +116,7 @@ export class ChromedashMyFeaturesPage extends LitElement {
|
|||
<h2>My features</h2>
|
||||
</div>
|
||||
${this.user && this.user.can_approve ? this.renderPendingAndRecentApprovals() : nothing}
|
||||
${this.renderIOwn()}
|
||||
${this.renderICanEdit()}
|
||||
${this.renderIStarred()}
|
||||
`;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import '../js-src/cs-client';
|
|||
import sinon from 'sinon';
|
||||
|
||||
describe('chromedash-myfeatures-page', () => {
|
||||
let recentReview; let pendingReview; let featureIOwn; let featureIStarred;
|
||||
let recentReview; let pendingReview; let featureICanEdit; let featureIStarred;
|
||||
|
||||
/* window.csClient and <chromedash-toast> are initialized at _base.html
|
||||
* which are not available here, so we initialize them before each test.
|
||||
|
|
@ -24,7 +24,7 @@ describe('chromedash-myfeatures-page', () => {
|
|||
|
||||
recentReview = null;
|
||||
pendingReview = null;
|
||||
featureIOwn = null;
|
||||
featureICanEdit = null;
|
||||
featureIStarred = null;
|
||||
});
|
||||
|
||||
|
|
@ -37,9 +37,10 @@ describe('chromedash-myfeatures-page', () => {
|
|||
window.csClient.getPermissions.returns(Promise.resolve({
|
||||
can_approve: false,
|
||||
can_create_feature: true,
|
||||
can_edit: true,
|
||||
can_edit_all: true,
|
||||
is_admin: false,
|
||||
email: 'example@gmail.com',
|
||||
editable_features: [],
|
||||
}));
|
||||
const component = await fixture(
|
||||
html`<chromedash-myfeatures-page></chromedash-myfeatures-page>`);
|
||||
|
|
@ -53,7 +54,7 @@ describe('chromedash-myfeatures-page', () => {
|
|||
const slDetails = component.shadowRoot.querySelectorAll('sl-details');
|
||||
slDetails.forEach((item) => {
|
||||
const itemHTML = item.outerHTML;
|
||||
if (itemHTML.includes('summary="Features I own"')) featureIOwn = item;
|
||||
if (itemHTML.includes('summary="Features I can edit"')) featureICanEdit = item;
|
||||
if (itemHTML.includes('summary="Features I starred"')) featureIStarred = item;
|
||||
if (itemHTML.includes('summary="Features pending my approval"')) pendingReview = item;
|
||||
if (itemHTML.includes('summary="Recently reviewed features"')) recentReview = item;
|
||||
|
|
@ -63,9 +64,9 @@ describe('chromedash-myfeatures-page', () => {
|
|||
assert.notExists(pendingReview);
|
||||
assert.notExists(recentReview);
|
||||
|
||||
// Features I own sl-details exists and has a correct query
|
||||
assert.exists(featureIOwn);
|
||||
assert.include(featureIOwn.innerHTML, 'query="owner:me"');
|
||||
// "Features I can edit" sl-details exists and has a correct query
|
||||
assert.exists(featureICanEdit);
|
||||
assert.include(featureICanEdit.innerHTML, 'query="can_edit:me"');
|
||||
|
||||
// Features I starred sl-details exists and has a correct query
|
||||
assert.exists(featureIStarred);
|
||||
|
|
@ -76,9 +77,10 @@ describe('chromedash-myfeatures-page', () => {
|
|||
window.csClient.getPermissions.returns(Promise.resolve({
|
||||
can_approve: true,
|
||||
can_create_feature: true,
|
||||
can_edit: true,
|
||||
can_edit_all: true,
|
||||
is_admin: false,
|
||||
email: 'example@gmail.com',
|
||||
editable_features: [],
|
||||
}));
|
||||
const component = await fixture(
|
||||
html`<chromedash-myfeatures-page></chromedash-myfeatures-page>`);
|
||||
|
|
@ -92,7 +94,7 @@ describe('chromedash-myfeatures-page', () => {
|
|||
const slDetails = component.shadowRoot.querySelectorAll('sl-details');
|
||||
slDetails.forEach((item) => {
|
||||
const itemHTML = item.outerHTML;
|
||||
if (itemHTML.includes('summary="Features I own"')) featureIOwn = item;
|
||||
if (itemHTML.includes('summary="Features I can edit')) featureICanEdit = item;
|
||||
if (itemHTML.includes('summary="Features I starred"')) featureIStarred = item;
|
||||
if (itemHTML.includes('summary="Features pending my approval"')) pendingReview = item;
|
||||
if (itemHTML.includes('summary="Recently reviewed features"')) recentReview = item;
|
||||
|
|
@ -106,9 +108,9 @@ describe('chromedash-myfeatures-page', () => {
|
|||
assert.exists(recentReview);
|
||||
assert.include(recentReview.innerHTML, 'query="is:recently-reviewed"');
|
||||
|
||||
// "Features I own" sl-details exists and has a correct query
|
||||
assert.exists(featureIOwn);
|
||||
assert.include(featureIOwn.innerHTML, 'query="owner:me"');
|
||||
// "Features I can edit" sl-details exists and has a correct query
|
||||
assert.exists(featureICanEdit);
|
||||
assert.include(featureICanEdit.innerHTML, 'query="can_edit:me"');
|
||||
|
||||
// "Features I starred" sl-details exists and has a correct query
|
||||
assert.exists(featureIStarred);
|
||||
|
|
|
|||
|
|
@ -55,6 +55,32 @@ class ChromedashUserlist extends LitElement {
|
|||
this.users = this.users.slice(0); // Refresh the list
|
||||
}
|
||||
|
||||
sortUsers() {
|
||||
this.users.sort((a, b) => {
|
||||
if ((a.is_admin && !b.is_admin) || (a.is_site_editor && (!b.is_site_editor && !b.is_admin))) {
|
||||
return -1;
|
||||
}
|
||||
if ((b.is_admin && !a.is_admin) || (b.is_site_editor && (!a.is_site_editor && !a.is_admin))) {
|
||||
return 1;
|
||||
}
|
||||
return a.email.localeCompare(b.email);
|
||||
});
|
||||
}
|
||||
|
||||
_onAdminToggle() {
|
||||
const formEl = this.shadowRoot.querySelector('form');
|
||||
const adminCheckbox = formEl.querySelector('input[name="is_admin"]');
|
||||
const siteEditorCheckbox = formEl.querySelector('input[name="is_site_editor"]');
|
||||
// Admins will always be site editors, so if the admin box is checked,
|
||||
// the site editor box is also checked and disabled.
|
||||
if (adminCheckbox.checked) {
|
||||
siteEditorCheckbox.checked = true;
|
||||
siteEditorCheckbox.disabled = true;
|
||||
} else {
|
||||
siteEditorCheckbox.disabled = false;
|
||||
}
|
||||
}
|
||||
|
||||
// TODO(jrobbins): Change this to be a JSON API call via csClient.
|
||||
async ajaxSubmit(e) {
|
||||
e.preventDefault();
|
||||
|
|
@ -63,12 +89,14 @@ class ChromedashUserlist extends LitElement {
|
|||
if (formEl.checkValidity()) {
|
||||
const email = formEl.querySelector('input[name="email"]').value;
|
||||
const isAdmin = formEl.querySelector('input[name="is_admin"]').checked;
|
||||
window.csClient.createAccount(email, isAdmin).then((json) => {
|
||||
const isSiteEditor = formEl.querySelector('input[name="is_site_editor"]').checked;
|
||||
window.csClient.createAccount(email, isAdmin, isSiteEditor).then((json) => {
|
||||
if (json.error_message) {
|
||||
alert(json.error_message);
|
||||
} else {
|
||||
this.addUser(json);
|
||||
formEl.reset();
|
||||
formEl.querySelector('input[name="is_site_editor"]').disabled = false;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
@ -87,6 +115,7 @@ class ChromedashUserlist extends LitElement {
|
|||
}
|
||||
|
||||
render() {
|
||||
this.sortUsers();
|
||||
return html`
|
||||
<form id="form" name="user_form" method="POST">
|
||||
<div>
|
||||
|
|
@ -94,7 +123,10 @@ class ChromedashUserlist extends LitElement {
|
|||
required>
|
||||
</div>
|
||||
<div>
|
||||
<label><input type="checkbox" name="is_admin"> User is admin</label>
|
||||
<label><input type="checkbox" name="is_admin" @click="${this._onAdminToggle}"> User is admin</label>
|
||||
</div>
|
||||
<div>
|
||||
<label><input type="checkbox" name="is_site_editor"> User is site editor</label>
|
||||
</div>
|
||||
<div>
|
||||
<input type="submit" @click="${this.ajaxSubmit}" value="Add user">
|
||||
|
|
@ -108,8 +140,9 @@ class ChromedashUserlist extends LitElement {
|
|||
data-index="${index}"
|
||||
data-account="${user.id}"
|
||||
@click="${this.ajaxDelete}">delete</a>
|
||||
<span>${user.email}</span>
|
||||
${user.is_admin ? html`(admin)` : nothing}
|
||||
${!user.is_admin && user.is_site_editor ? html`(site editor)` : nothing}
|
||||
<span>${user.email}</span>
|
||||
</li>
|
||||
`)}
|
||||
</ul>
|
||||
|
|
|
|||
|
|
@ -54,6 +54,12 @@ export const ALL_FIELDS = {
|
|||
from @chromium.org are preferred.`,
|
||||
},
|
||||
|
||||
'editors': {
|
||||
help_text: html`
|
||||
Comma separated list of full email addresses. These users will be
|
||||
allowed to edit this feature, but will not be listed as feature owners.`,
|
||||
},
|
||||
|
||||
'unlisted': {
|
||||
help_text: html`
|
||||
Check this box to hide draft emails in list views. Anyone with
|
||||
|
|
|
|||
|
|
@ -189,8 +189,8 @@ class ChromeStatusClient {
|
|||
|
||||
// Accounts API
|
||||
|
||||
createAccount(email, isAdmin) {
|
||||
return this.doPost('/accounts', {email, isAdmin});
|
||||
createAccount(email, isAdmin, isSiteEditor) {
|
||||
return this.doPost('/accounts', {email, isAdmin, isSiteEditor});
|
||||
// TODO: catch((error) => { display message }
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@
|
|||
</button>
|
||||
</div>
|
||||
<div class="actionlinks">
|
||||
{% if user.can_edit %}
|
||||
{% if user.can_create_feature %}
|
||||
<a href="/guide/new" class="blue-button" title="Adds a new feature to the site">
|
||||
<iron-icon icon="chromestatus:add-circle-outline"></iron-icon><span>Add new feature</span>
|
||||
</a>
|
||||
|
|
@ -48,8 +48,9 @@
|
|||
</div>
|
||||
</div>
|
||||
<chromedash-featurelist
|
||||
{% if user %} signedInUser="{{user.email}}" {% endif %}
|
||||
{% if user.can_edit %}canEdit{% endif %}
|
||||
{% if user %} signedInUser="{{user.email}}" {% endif %}
|
||||
{% if user.can_edit_all %}isSiteEditor{% endif %}
|
||||
{% if user %} editableFeatures="{{user.editable_features}}" {% endif %}
|
||||
{% if user.can_approve %}canApprove{% endif %}>
|
||||
</chromedash-featurelist>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@
|
|||
{% if feature.unlisted %}
|
||||
<div style="padding: 8px">
|
||||
This feature is only shown in the feature list
|
||||
to users who with edit access.
|
||||
to users with access to edit this feature.
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@
|
|||
{% block content %}
|
||||
<chromedash-new-feature-list
|
||||
{% if user %} signedinuser="{{user.email}}" {% endif %}
|
||||
{% if user.can_edit %} canedit {% endif %}
|
||||
{% if user.can_edit_all %} canedit {% endif %}
|
||||
{% if user.can_approve %} canapprove {% endif %}
|
||||
>
|
||||
</chromedash-new-feature-list>
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче