custom fields: Allow list of users in user type of custom fields.

Allow user to add more than one user-value in user type of custom
fields.

Tweaked by tabbott to improve the models.py code and type annotations.
This commit is contained in:
Yashashvi Dave
2018-06-07 23:31:31 +05:30
committed by Tim Abbott
parent b0db90648a
commit 8909cb1d15
8 changed files with 68 additions and 53 deletions

View File

@@ -100,6 +100,9 @@ exports.add_custom_profile_fields_to_settings = function () {
} else if (field_type === "URL") {
type = "url";
} else if (field_type === "User") {
if (value) {
value = JSON.parse(value);
}
type = "user";
} else {
blueslip.error("Undefined field type.");
@@ -127,23 +130,21 @@ exports.add_custom_profile_fields_to_settings = function () {
function update_custom_user_field() {
var fields = [];
var user_id = user_pill.get_user_ids(pills);
if (user_id.length > 1) {
ui_report.message(i18n.t("Only one user allowed"), $("#custom-field-status"), 'alert-error');
return;
}
if (user_id.length < 1) {
var user_ids = user_pill.get_user_ids(pills);
if (user_ids.length < 1) {
fields.push(field.id);
update_user_custom_profile_fields(fields, channel.del);
} else {
fields.push({id: field.id, value: user_id[0]});
fields.push({id: field.id, value: user_ids});
update_user_custom_profile_fields(fields, channel.patch);
}
}
if (value) {
var user = people.get_person_from_user_id(value);
user_pill.append_user(user, pills);
if (value !== undefined && value.length > 0) {
value.forEach(function (user_id) {
var user = people.get_person_from_user_id(user_id);
user_pill.append_user(user, pills);
});
}
var input = pill_container.children('.input');
user_pill.set_up_typeahead_on_pills(input, pills, update_custom_user_field);

View File

@@ -4742,14 +4742,14 @@ def try_reorder_realm_custom_profile_fields(realm: Realm, order: List[int]) -> N
notify_realm_custom_profile_fields(realm, 'update')
def notify_user_update_custom_profile_data(user_profile: UserProfile,
field: Dict[str, Union[int, str, None]]) -> None:
field: Dict[str, Union[int, str, List[int], None]]) -> None:
payload = dict(user_id=user_profile.id, custom_profile_field=dict(id=field['id'],
value=field['value']))
event = dict(type="realm_user", op="update", person=payload)
send_event(event, active_user_ids(user_profile.realm.id))
def do_update_user_custom_profile_data(user_profile: UserProfile,
data: List[Dict[str, Union[int, str]]]) -> None:
data: List[Dict[str, Union[int, str, List[int]]]]) -> None:
with transaction.atomic():
update_or_create = CustomProfileFieldValue.objects.update_or_create
for field in data:

View File

@@ -7,7 +7,7 @@ ViewFuncT = TypeVar('ViewFuncT', bound=Callable[..., HttpResponse])
# including many examples
Validator = Callable[[str, object], Optional[str]]
ExtendedValidator = Callable[[str, str, object], Optional[str]]
RealmUserValidator = Callable[[int, object, bool], Optional[str]]
RealmUserValidator = Callable[[int, List[int], bool], Optional[str]]
ProfileDataElement = Dict[str, Union[int, float, Optional[str]]]
ProfileData = List[ProfileDataElement]

View File

@@ -33,7 +33,7 @@ from django.utils.translation import ugettext_lazy as _
from zerver.lib import cache
from zerver.lib.validator import check_int, check_float, \
check_short_string, check_long_string, validate_choice_field, check_date, \
check_url
check_url, check_list
from zerver.lib.name_restrictions import is_disposable_domain
from zerver.lib.types import Validator, ExtendedValidator, \
ProfileDataElement, ProfileData, FieldTypeData, FieldElement, \
@@ -1969,22 +1969,30 @@ class UserHotspot(models.Model):
class Meta:
unique_together = ("user", "hotspot")
def check_valid_user_id(realm_id: int, user_id: object,
allow_deactivated: bool=False) -> Optional[str]:
if not isinstance(user_id, int):
return _("User id is not an integer")
try:
realm = Realm.objects.get(id=realm_id)
user_profile = get_user_profile_by_id_in_realm(user_id, realm)
def check_valid_user_ids(realm_id: int, user_ids: List[int],
allow_deactivated: bool=False) -> Optional[str]:
error = check_list(check_int)("User IDs", user_ids)
if error:
return error
realm = Realm.objects.get(id=realm_id)
for user_id in user_ids:
# TODO: Structurally, we should be doing a bulk fetch query to
# get the users here, not doing these in a loop. But because
# this is a rarely used feature and likely to never have more
# than a handful of users, it's probably mostly OK.
try:
user_profile = get_user_profile_by_id_in_realm(user_id, realm)
except UserProfile.DoesNotExist:
return _('Invalid user ID: %d') % (user_id)
if not allow_deactivated:
if not user_profile.is_active:
return _('User is deactivated')
return _('User with ID %d is deactivated') % (user_id)
if (user_profile.is_bot):
return _('User with id %d is bot') % (user_id)
return None
except UserProfile.DoesNotExist:
return _('Invalid user ID: %d') % (user_id)
return _('User with ID %d is a bot') % (user_id)
return None
class CustomProfileField(models.Model):
HINT_MAX_LENGTH = 80
@@ -2010,7 +2018,7 @@ class CustomProfileField(models.Model):
(CHOICE, str(_('Choice')), validate_choice_field, str),
] # type: FieldTypeData
USER_FIELD_TYPE_DATA = [
(USER, str(_('User')), check_valid_user_id, int),
(USER, str(_('User')), check_valid_user_ids, eval),
] # type: FieldTypeData
CHOICE_FIELD_VALIDATORS = {

View File

@@ -339,12 +339,12 @@ class CustomProfileFieldTest(ZulipTestCase):
field = CustomProfileField.objects.get(name="Mentor", realm=realm)
data = [{'id': field.id,
'value': self.example_user("aaron").id}] # type: List[Dict[str, Union[int, str]]]
'value': [self.example_user("aaron").id]}] # type: List[Dict[str, Union[int, str, List[int]]]]
do_update_user_custom_profile_data(iago, data)
iago_value = CustomProfileFieldValue.objects.get(user_profile=iago, field=field)
converter = field.FIELD_CONVERTERS[field.field_type]
self.assertEqual(self.example_user("aaron").id, converter(iago_value.value))
self.assertEqual([self.example_user("aaron").id], converter(iago_value.value))
result = self.client_delete("/json/users/me/profile_data", {
'data': ujson.dumps([field.id])
@@ -377,7 +377,7 @@ class CustomProfileFieldTest(ZulipTestCase):
def test_update_invalid_user_field(self) -> None:
field_name = "Mentor"
invalid_user_id = 1000
self.assert_error_update_invalid_value(field_name, invalid_user_id,
self.assert_error_update_invalid_value(field_name, [invalid_user_id],
u"Invalid user ID: %d"
% (invalid_user_id))
@@ -399,7 +399,7 @@ class CustomProfileFieldTest(ZulipTestCase):
('Favorite editor', 'vim'),
('Birthday', '1909-3-5'),
('GitHub profile', 'https://github.com/ABC'),
('Mentor', self.example_user("cordelia").id),
('Mentor', [self.example_user("cordelia").id]),
]
data = []
@@ -460,7 +460,7 @@ class CustomProfileFieldTest(ZulipTestCase):
user_profile = self.example_user('iago')
realm = user_profile.realm
field = CustomProfileField.objects.get(name="Phone number", realm=realm)
data = [{'id': field.id, 'value': u'123456'}] # type: List[Dict[str, Union[int, str]]]
data = [{'id': field.id, 'value': u'123456'}] # type: List[Dict[str, Union[int, str, List[int]]]]
do_update_user_custom_profile_data(user_profile, data)
self.assertTrue(self.custom_field_exists_in_realm(field.id))

View File

@@ -21,7 +21,7 @@ from zerver.models import UserProfile, Recipient, \
Realm, RealmDomain, UserActivity, UserHotspot, \
get_user, get_realm, get_client, get_stream, get_stream_recipient, \
get_membership_realms, get_source_profile, \
Message, get_context_for_message, ScheduledEmail, check_valid_user_id
Message, get_context_for_message, ScheduledEmail, check_valid_user_ids
from zerver.lib.avatar import avatar_url
from zerver.lib.email_mirror import create_missed_message_address
@@ -321,32 +321,37 @@ class UserProfileTest(ZulipTestCase):
bot = self.example_user("welcome_bot")
# Invalid user ID
invalid_uid = 1000
self.assertEqual(check_valid_user_id(realm.id, invalid_uid),
invalid_uid = 1000 # type: Any
self.assertEqual(check_valid_user_ids(realm.id, invalid_uid),
"User IDs is not a list")
self.assertEqual(check_valid_user_ids(realm.id, [invalid_uid]),
"Invalid user ID: %d" % (invalid_uid))
self.assertEqual(check_valid_user_id(realm.id, "abc"),
"User id is not an integer")
self.assertEqual(check_valid_user_id(realm.id, str(othello.id)),
"User id is not an integer")
invalid_uid = "abc"
self.assertEqual(check_valid_user_ids(realm.id, [invalid_uid]),
"User IDs[0] is not an integer")
invalid_uid = str(othello.id)
self.assertEqual(check_valid_user_ids(realm.id, [invalid_uid]),
"User IDs[0] is not an integer")
# User is in different realm
self.assertEqual(check_valid_user_id(get_realm("zephyr").id, hamlet.id),
self.assertEqual(check_valid_user_ids(get_realm("zephyr").id, [hamlet.id]),
"Invalid user ID: %d" % (hamlet.id))
# User is not active
hamlet.is_active = False
hamlet.save()
self.assertEqual(check_valid_user_id(realm.id, hamlet.id),
"User is deactivated")
self.assertEqual(check_valid_user_id(realm.id, hamlet.id, allow_deactivated=True),
self.assertEqual(check_valid_user_ids(realm.id, [hamlet.id]),
"User with ID %d is deactivated" % (hamlet.id))
self.assertEqual(check_valid_user_ids(realm.id, [hamlet.id], allow_deactivated=True),
None)
# User is bot
self.assertEqual(check_valid_user_id(realm.id, bot.id),
"User with id %d is bot" % (bot.id))
# User is a bot
self.assertEqual(check_valid_user_ids(realm.id, [bot.id]),
"User with ID %d is a bot" % (bot.id))
# Succesfully get non-bot, active user belong to your realm
self.assertEqual(check_valid_user_id(realm.id, othello.id), None)
self.assertEqual(check_valid_user_ids(realm.id, [othello.id]), None)
def test_cache_invalidation(self) -> None:
hamlet = self.example_user('hamlet')

View File

@@ -1,5 +1,5 @@
from typing import Union, List, Dict, Optional
from typing import Union, List, Dict, Optional, cast
import logging
import ujson
@@ -142,7 +142,7 @@ def remove_user_custom_profile_data(request: HttpRequest, user_profile: UserProf
def update_user_custom_profile_data(
request: HttpRequest,
user_profile: UserProfile,
data: List[Dict[str, Union[int, str]]]=REQ(validator=check_list(
data: List[Dict[str, Union[int, str, List[int]]]]=REQ(validator=check_list(
check_dict([('id', check_int)])))) -> HttpResponse:
for item in data:
field_id = item['id']
@@ -153,8 +153,8 @@ def update_user_custom_profile_data(
validators = CustomProfileField.FIELD_VALIDATORS
field_type = field.field_type
value = item['value']
var_name = '{}'.format(field.name)
value = item['value']
if field_type in validators:
validator = validators[field_type]
result = validator(var_name, value)
@@ -164,7 +164,8 @@ def update_user_custom_profile_data(
result = choice_field_validator(var_name, field_data, value)
elif field_type == CustomProfileField.USER:
user_field_validator = CustomProfileField.USER_FIELD_VALIDATORS[field_type]
result = user_field_validator(user_profile.realm.id, value, False)
result = user_field_validator(user_profile.realm.id, cast(List[int], value),
False)
else:
raise AssertionError("Invalid field type")

View File

@@ -337,7 +337,7 @@ class Command(BaseCommand):
{"id": favorite_editor.id, "value": "emacs"},
{"id": birthday.id, "value": "2000-1-1"},
{"id": favorite_website.id, "value": "https://github.com/zulip/zulip"},
{"id": mentor.id, "value": hamlet.id},
{"id": mentor.id, "value": [hamlet.id]},
])
do_update_user_custom_profile_data(hamlet, [
{"id": phone_number.id, "value": "+0-11-23-456-7890"},
@@ -346,7 +346,7 @@ class Command(BaseCommand):
{"id": favorite_editor.id, "value": "vim"},
{"id": birthday.id, "value": "1900-1-1"},
{"id": favorite_website.id, "value": "https://blog.zulig.org"},
{"id": mentor.id, "value": iago.id},
{"id": mentor.id, "value": [iago.id]},
])
else:
zulip_realm = get_realm("zulip")