diff --git a/.travis.yml b/.travis.yml index d9c90f81c..472e31682 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,6 +28,6 @@ script: - coverage report --fail-under=44 - DJANGO_SETTINGS_MODULE='tests.settings' coverage run ./manage.py test tests.integration - - coverage report --fail-under=73 + - coverage report --fail-under=74 - DJANGO_SETTINGS_MODULE='tests.old.settings' ./manage.py test tests.old diff --git a/CHANGELOG b/CHANGELOG index 6d1607f67..d18e67e07 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -87,6 +87,7 @@ Elections: Users: - Added new matrix-interface for managing groups and their permissions. +- Added autoupdate on permission change (permission added). - Improved password reset view for administrators. - Changed field for initial password to an unchangeable field. - Added new field for participant number. diff --git a/openslides/agenda/apps.py b/openslides/agenda/apps.py index 8c94160b0..e0123cfe9 100644 --- a/openslides/agenda/apps.py +++ b/openslides/agenda/apps.py @@ -1,5 +1,7 @@ from django.apps import AppConfig +from ..utils.collection import Collection + class AgendaAppConfig(AppConfig): name = 'openslides.agenda' @@ -37,5 +39,8 @@ class AgendaAppConfig(AppConfig): router.register(self.get_model('Item').get_collection_string(), ItemViewSet) def get_startup_elements(self): - from ..utils.collection import Collection - return [Collection(self.get_model('Item').get_collection_string())] + """ + Yields all collections required on startup i. e. opening the websocket + connection. + """ + yield Collection(self.get_model('Item').get_collection_string()) diff --git a/openslides/assignments/apps.py b/openslides/assignments/apps.py index 8dc515f69..16634c46e 100644 --- a/openslides/assignments/apps.py +++ b/openslides/assignments/apps.py @@ -1,5 +1,7 @@ from django.apps import AppConfig +from ..utils.collection import Collection + class AssignmentsAppConfig(AppConfig): name = 'openslides.assignments' @@ -26,15 +28,18 @@ class AssignmentsAppConfig(AppConfig): # Connect signals. permission_change.connect( get_permission_change_data, - dispatch_uid='assignment_get_permission_change_data') + dispatch_uid='assignments_get_permission_change_data') # Register viewsets. router.register(self.get_model('Assignment').get_collection_string(), AssignmentViewSet) router.register('assignments/poll', AssignmentPollViewSet) def get_startup_elements(self): - from ..utils.collection import Collection - return [Collection(self.get_model('Assignment').get_collection_string())] + """ + Yields all collections required on startup i. e. opening the websocket + connection. + """ + yield Collection(self.get_model('Assignment').get_collection_string()) def get_angular_constants(self): assignment = self.get_model('Assignment') diff --git a/openslides/assignments/signals.py b/openslides/assignments/signals.py index 00b353e41..22182ba51 100644 --- a/openslides/assignments/signals.py +++ b/openslides/assignments/signals.py @@ -3,11 +3,10 @@ from django.apps import apps def get_permission_change_data(sender, permissions=None, **kwargs): """ - Returns all necessary collections if a 'can_see' permission changes. + Yields all necessary collections if 'assignments.can_see' permission changes. """ assignments_app = apps.get_app_config(app_label='assignments') for permission in permissions: # There could be only one 'assignment.can_see' and then we want to return data. - if permission.content_type.app_label == assignment_app.label and permission.codename == 'can_see': - return assignment_app.get_startup_elements() - return None + if permission.content_type.app_label == assignments_app.label and permission.codename == 'can_see': + yield from assignments_app.get_startup_elements() diff --git a/openslides/core/apps.py b/openslides/core/apps.py index 76e809f6e..389316682 100644 --- a/openslides/core/apps.py +++ b/openslides/core/apps.py @@ -1,6 +1,8 @@ from django.apps import AppConfig from django.conf import settings +from ..utils.collection import Collection + class CoreAppConfig(AppConfig): name = 'openslides.core' @@ -51,8 +53,11 @@ class CoreAppConfig(AppConfig): router.register(self.get_model('Countdown').get_collection_string(), CountdownViewSet) def get_startup_elements(self): + """ + Yields all collections required on startup i. e. opening the websocket + connection. + """ from .config import config - from ..utils.collection import Collection for model in ('Projector', 'ChatMessage', 'Tag', 'ProjectorMessage', 'Countdown'): yield Collection(self.get_model(model).get_collection_string()) yield Collection(config.get_collection_string()) diff --git a/openslides/core/signals.py b/openslides/core/signals.py index 5a5019017..2d3f2f8e7 100644 --- a/openslides/core/signals.py +++ b/openslides/core/signals.py @@ -4,12 +4,15 @@ from django.contrib.contenttypes.models import ContentType from django.db.models import Q from django.dispatch import Signal +from ..utils.collection import Collection + # This signal is send when the migrate command is done. That means it is sent # after post_migrate sending and creating all Permission objects. Don't use it # for other things than dealing with Permission objects. post_permission_creation = Signal() -# This signal is send, if a permission is changed. +# This signal is sent if a permission is changed (e. g. a group gets a new +# permission). Connected receivers may yield Collections. permission_change = Signal() @@ -25,14 +28,12 @@ def delete_django_app_permissions(sender, **kwargs): Permission.objects.filter(content_type__in=contenttypes).delete() -def get_permission_change_data(sender, permissions=None, **kwargs): +def get_permission_change_data(sender, permissions, **kwargs): """ - Returns all necessary collections if the coupled permission changes. + Yields all necessary collections if the respective permissions change. """ - from ..utils.collection import Collection core_app = apps.get_app_config(app_label='core') for permission in permissions: - # There could be only one 'users.can_see' and then we want to return data. if permission.content_type.app_label == core_app.label: if permission.codename == 'can_see_projector': yield Collection(core_app.get_model('Projector').get_collection_string()) diff --git a/openslides/mediafiles/apps.py b/openslides/mediafiles/apps.py index f86fcf329..76ef8b5b6 100644 --- a/openslides/mediafiles/apps.py +++ b/openslides/mediafiles/apps.py @@ -1,5 +1,7 @@ from django.apps import AppConfig +from ..utils.collection import Collection + class MediafilesAppConfig(AppConfig): name = 'openslides.mediafiles' @@ -21,11 +23,14 @@ class MediafilesAppConfig(AppConfig): # Connect signals. permission_change.connect( get_permission_change_data, - dispatch_uid='mediafile_get_permission_change_data') + dispatch_uid='mediafiles_get_permission_change_data') # Register viewsets. router.register(self.get_model('Mediafile').get_collection_string(), MediafileViewSet) def get_startup_elements(self): - from ..utils.collection import Collection - return [Collection(self.get_model('Mediafile').get_collection_string())] + """ + Yields all collections required on startup i. e. opening the websocket + connection. + """ + yield Collection(self.get_model('Mediafile').get_collection_string()) diff --git a/openslides/mediafiles/signals.py b/openslides/mediafiles/signals.py index 096e3ef6a..ad297ac19 100644 --- a/openslides/mediafiles/signals.py +++ b/openslides/mediafiles/signals.py @@ -3,11 +3,10 @@ from django.apps import apps def get_permission_change_data(sender, permissions=None, **kwargs): """ - Returns all necessary collections if a 'can_see' permission changes. + Yields all necessary collections if 'mediafiles.can_see' permission changes. """ - mediafile_app = apps.get_app_config(app_label='mediafiles') + mediafiles_app = apps.get_app_config(app_label='mediafiles') for permission in permissions: # There could be only one 'mediafiles.can_see' and then we want to return data. - if permission.content_type.app_label == mediafile_app.label and permission.codename == 'can_see': - return mediafile_app.get_startup_elements() - return None + if permission.content_type.app_label == mediafiles_app.label and permission.codename == 'can_see': + yield from mediafiles_app.get_startup_elements() diff --git a/openslides/motions/apps.py b/openslides/motions/apps.py index ce484caf8..5a037e16b 100644 --- a/openslides/motions/apps.py +++ b/openslides/motions/apps.py @@ -1,6 +1,8 @@ from django.apps import AppConfig from django.db.models.signals import post_migrate +from ..utils.collection import Collection + class MotionsAppConfig(AppConfig): name = 'openslides.motions' @@ -28,7 +30,7 @@ class MotionsAppConfig(AppConfig): post_migrate.connect(create_builtin_workflows, dispatch_uid='motion_create_builtin_workflows') permission_change.connect( get_permission_change_data, - dispatch_uid='motion_get_permission_change_data') + dispatch_uid='motions_get_permission_change_data') # Register viewsets. router.register(self.get_model('Category').get_collection_string(), CategoryViewSet) @@ -40,6 +42,9 @@ class MotionsAppConfig(AppConfig): router.register('motions/motionpoll', MotionPollViewSet) def get_startup_elements(self): - from ..utils.collection import Collection + """ + Yields all collections required on startup i. e. opening the websocket + connection. + """ for model in ('Category', 'Motion', 'MotionBlock', 'Workflow', 'MotionChangeRecommendation'): yield Collection(self.get_model(model).get_collection_string()) diff --git a/openslides/motions/signals.py b/openslides/motions/signals.py index 3c9a6c49d..64a0cdb9e 100644 --- a/openslides/motions/signals.py +++ b/openslides/motions/signals.py @@ -105,13 +105,12 @@ def create_builtin_workflows(sender, **kwargs): workflow_2.save() -def get_permission_change_data(sender, permissions=None, **kwargs): +def get_permission_change_data(sender, permissions, **kwargs): """ - Returns all necessary collections if a 'can_see' permission changes. + Yields all necessary collections if 'motions.can_see' permission changes. """ - motion_app = apps.get_app_config(app_label='motions') + motions_app = apps.get_app_config(app_label='motions') for permission in permissions: - # There could be only one 'motion.can_see' and then we want to return data. - if permission.content_type.app_label == motion_app.label and permission.codename == 'can_see': - return motion_app.get_startup_elements() - return None + # There could be only one 'motions.can_see' and then we want to return data. + if permission.content_type.app_label == motions_app.label and permission.codename == 'can_see': + yield from motions_app.get_startup_elements() diff --git a/openslides/topics/apps.py b/openslides/topics/apps.py index 49eba7af8..f7f246288 100644 --- a/openslides/topics/apps.py +++ b/openslides/topics/apps.py @@ -1,5 +1,7 @@ from django.apps import AppConfig +from ..utils.collection import Collection + class TopicsAppConfig(AppConfig): name = 'openslides.topics' @@ -20,5 +22,8 @@ class TopicsAppConfig(AppConfig): router.register(self.get_model('Topic').get_collection_string(), TopicViewSet) def get_startup_elements(self): - from ..utils.collection import Collection - return [Collection(self.get_model('Topic').get_collection_string())] + """ + Yields all collections required on startup i. e. opening the websocket + connection. + """ + yield Collection(self.get_model('Topic').get_collection_string()) diff --git a/openslides/users/apps.py b/openslides/users/apps.py index d0f99424a..7bbd60678 100644 --- a/openslides/users/apps.py +++ b/openslides/users/apps.py @@ -1,5 +1,7 @@ from django.apps import AppConfig +from ..utils.collection import Collection + class UsersAppConfig(AppConfig): name = 'openslides.users' @@ -29,13 +31,16 @@ class UsersAppConfig(AppConfig): dispatch_uid='create_builtin_groups_and_admin') permission_change.connect( get_permission_change_data, - dispatch_uid='user_get_permission_change_data') + dispatch_uid='users_get_permission_change_data') # Register viewsets. router.register(self.get_model('User').get_collection_string(), UserViewSet) router.register(self.get_model('Group').get_collection_string(), GroupViewSet) def get_startup_elements(self): - from ..utils.collection import Collection + """ + Yields all collections required on startup i. e. opening the websocket + connection. + """ for model in ('User', 'Group'): yield Collection(self.get_model(model).get_collection_string()) diff --git a/openslides/users/signals.py b/openslides/users/signals.py index 318308764..157473780 100644 --- a/openslides/users/signals.py +++ b/openslides/users/signals.py @@ -8,14 +8,13 @@ from .models import Group, User def get_permission_change_data(sender, permissions=None, **kwargs): """ - Returns all necessary collections if a 'can_see' permission changes. + Yields all necessary collections if 'users.can_see' permission changes. """ - user_app = apps.get_app_config(app_label='users') + users_app = apps.get_app_config(app_label='users') for permission in permissions: # There could be only one 'users.can_see' and then we want to return data. - if permission.content_type.app_label == user_app.label and permission.codename == 'can_see': - return user_app.get_startup_elements() - return None + if permission.content_type.app_label == users_app.label and permission.codename == 'can_see': + yield from users_app.get_startup_elements() def create_builtin_groups_and_admin(**kwargs): diff --git a/openslides/users/views.py b/openslides/users/views.py index 240be742b..191d4e413 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -8,8 +8,8 @@ from django.utils.translation import ugettext as _ from ..core.config import config from ..core.signals import permission_change from ..utils.auth import anonymous_is_enabled, has_perm -from ..utils.autoupdate import send_collections_to_users, inform_changed_data -from ..utils.collection import CollectionElement +from ..utils.autoupdate import inform_data_collection_element_list +from ..utils.collection import CollectionElement, CollectionElementList from ..utils.rest_api import ( ModelViewSet, Response, @@ -161,6 +161,51 @@ class GroupViewSet(ModelViewSet): result = False return result + def update(self, request, *args, **kwargs): + """ + Customized endpoint to update a group. Send the signal + 'permission_change' if group permissions change. + """ + group = self.get_object() + + # Collect old and new (given) permissions to get the difference. + old_permissions = list(group.permissions.all()) # Force evaluation so the perms don't change anymore. + permission_names = request.data['permissions'] + if isinstance(permission_names, str): + permission_names = [permission_names] + given_permissions = [ + PermissionRelatedField(read_only=True).to_internal_value(data=perm) for perm in permission_names] + + # Run super to update the group. + response = super().update(request, *args, **kwargs) + + # Check status code and send 'permission_change' signal. + if response.status_code == 200: + + def diff(full, part): + """ + This helper function calculates the difference of two lists: + The result is a list of all elements of 'full' that are + not in 'part'. + """ + part = set(part) + return [item for item in full if item not in part] + + new_permissions = diff(given_permissions, old_permissions) + + # Some permissions are added. + if len(new_permissions) > 0: + collection_elements = CollectionElementList() + signal_results = permission_change.send(None, permissions=new_permissions, action='added') + for receiver, signal_collections in signal_results: + for collection in signal_collections: + collection_elements.extend(collection.element_generator()) + inform_data_collection_element_list(collection_elements) + + # TODO: Some permissions are deleted. + + return response + def destroy(self, request, *args, **kwargs): """ Protects builtin groups 'Default' (pk=1) from being deleted. @@ -171,42 +216,6 @@ class GroupViewSet(ModelViewSet): self.perform_destroy(instance) return Response(status=status.HTTP_204_NO_CONTENT) - def update(self, request, *args, **kwargs): - # Collect old and new permissions to get the difference. - permission_names = request.data['permissions'] - if isinstance(permission_names, str): - permission_names = [permission_names] - - permissionSerializer = PermissionRelatedField(read_only=True) - given_permissions = [ - permissionSerializer.to_internal_value(data=perm) for perm in permission_names] - group = Group.objects.get(pk=int(kwargs['pk'])) - old_permissions = list(group.permissions.all()) # Force evaluation so the perms doesn't change. - - response = super().update(request, *args, **kwargs) - - def diff(first, second): - """ - This helper function calculates the difference of two lists: first-second. - """ - second = set(second) - return [item for item in first if item not in second] - - if response.status_code == 200: - new_permissions = diff(given_permissions, old_permissions) - - # some permissions are added - if(len(new_permissions) > 0): - signal_results = permission_change.send(None, action='added', permissions=new_permissions) - collections_to_send = [] - for (receiver, signal_collections) in signal_results: - if signal_collections: - collections_to_send.extend(signal_collections) - #send_collections_to_users(collections_to_send, [user.id for user in group.user_set.all()]) - # inform_changed_data(what?) - - return response - # Special API views diff --git a/openslides/utils/autoupdate.py b/openslides/utils/autoupdate.py index 55fab43eb..4218fb2ce 100644 --- a/openslides/utils/autoupdate.py +++ b/openslides/utils/autoupdate.py @@ -146,6 +146,8 @@ def send_data(message): Informs all site users and projector clients about changed data. """ collection_elements = CollectionElementList.from_channels_message(message) + + # Send data to site users. for user_id, channel_names in websocket_user_cache.get_all().items(): if not user_id: # Anonymous user @@ -187,21 +189,6 @@ def send_data(message): {'text': json.dumps(output)}) -def send_collections_to_users(collections, users_ids): - for user_id, channel_names in websocket_user_cache.get_all().items(): - if user_id in users_ids: - try: - user = user_to_collection_user(user_id) - except ObjectDoesNotExist: - # The user does not exist. Skip him/her. - continue - output = [] - for collection in collections: - output.extend(collection.as_autoupdate_for_user(user)) - for channel_name in channel_names: - send_or_wait(Channel(channel_name).send, {'text': json.dumps(output)}) - - def inform_changed_data(instances, information=None): """ Informs the autoupdate system and the caching system about the creation or @@ -268,6 +255,18 @@ def inform_deleted_data(*args, information=None): transaction.on_commit(lambda: send_autoupdate(collection_elements)) +def inform_data_collection_element_list(collection_elements, information=None): + """ + Informs the autoupdate system about some collection elements. This is + used just to send some data to all users. + """ + # If currently there is an open database transaction, then the + # send_autoupdate function is only called, when the transaction is + # commited. If there is currently no transaction, then the function + # is called immediately. + transaction.on_commit(lambda: send_autoupdate(collection_elements)) + + def send_autoupdate(collection_elements): """ Helper function, that sends collection_elements through a channel to the diff --git a/setup.cfg b/setup.cfg index 39c01bcdc..8ad6eaacc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,8 @@ [coverage:run] source = openslides -omit = openslides/core/management/commands/getgeiss.py +omit = + openslides/core/management/commands/*.py + openslides/users/management/commands/*.py [coverage:html] directory = personal_data/tmp/htmlcov diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index 06a2e94ea..859ab0b1a 100644 --- a/tests/integration/users/test_viewset.py +++ b/tests/integration/users/test_viewset.py @@ -430,6 +430,53 @@ class GroupUpdate(TestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.data, {'name': ['This field is required.']}) + def test_update_via_put_with_new_permissions(self): + admin_client = APIClient() + admin_client.login(username='admin', password='admin') + group = Group.objects.create(name='group_name_inooThe3dii4mahWeeSe') + # This contains all permissions. + permissions = [ + 'agenda.can_be_speaker', + 'agenda.can_manage', + 'agenda.can_see', + 'agenda.can_see_hidden_items', + 'assignments.can_manage', + 'assignments.can_nominate_other', + 'assignments.can_nominate_self', + 'assignments.can_see', + 'core.can_manage_config', + 'core.can_manage_projector', + 'core.can_manage_tags', + 'core.can_manage_chat', + 'core.can_see_frontpage', + 'core.can_see_projector', + 'core.can_use_chat', + 'mediafiles.can_manage', + 'mediafiles.can_see', + 'mediafiles.can_see_hidden', + 'mediafiles.can_upload', + 'motions.can_create', + 'motions.can_manage', + 'motions.can_see', + 'motions.can_see_and_manage_comments', + 'motions.can_support', + 'users.can_manage', + 'users.can_see_extra_data', + 'users.can_see_name', + ] + + response = admin_client.put( + reverse('group-detail', args=[group.pk]), + {'name': 'new_group_name_Chie6duwaepoo8aech7r', + 'permissions': permissions}, + format='json') + + self.assertEqual(response.status_code, status.HTTP_200_OK) + group = Group.objects.get(pk=group.pk) + for permission in permissions: + app_label, codename = permission.split('.') + self.assertTrue(group.permissions.get(content_type__app_label=app_label, codename=codename)) + class GroupDelete(TestCase): """