From fafd7d48485423e86ed2182362b2f8e85e929450 Mon Sep 17 00:00:00 2001 From: Sandro Lutz Date: Wed, 27 Nov 2019 01:04:43 +0100 Subject: [PATCH 1/3] Fix newsletter subscription when upgrading users to members via LDAP. When users are upgraded to members via LDAP, subscribe them to the newsletter. Newsletter settings of already imported members are not touched. Closes #400 --- amivapi/ldap.py | 2 +- amivapi/tests/test_ldap.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/amivapi/ldap.py b/amivapi/ldap.py index 550ab154..06174ba4 100644 --- a/amivapi/ldap.py +++ b/amivapi/ldap.py @@ -195,9 +195,9 @@ def _create_or_update_user(ldap_data): # Membership will not be downgraded and email not be overwritten # Newletter settings will also not be adjusted ldap_data.pop('email', None) - ldap_data.pop('send_newsletter', None) if db_data.get('membership') != u"none": ldap_data.pop('membership', None) + ldap_data.pop('send_newsletter', None) user = patch_internal('users', ldap_data, diff --git a/amivapi/tests/test_ldap.py b/amivapi/tests/test_ldap.py index e2ff3ce8..b87656d0 100644 --- a/amivapi/tests/test_ldap.py +++ b/amivapi/tests/test_ldap.py @@ -214,6 +214,26 @@ def test_update_user(self): else: self.assertEqual(result[field], db_value) + def test_upgrade_membership(self): + # Insert non-member and upgrade by ldap later + user = self.api.post('/users', data={ + 'nethz': 'pablo', + 'email': 'pablo@ethz.ch', # this will be auto-generated + 'firstname': 'P', + 'lastname': 'Ablo', + 'department': 'itet', + 'membership': 'none', + 'gender': 'female', + 'legi': '01234567', + 'send_newsletter': False, + }, status_code=201).json + self.assertFalse(user['send_newsletter']) + + with self.app.test_request_context(): + result = ldap._create_or_update_user(self.fake_filtered_data()) + + self.assertTrue(result['send_newsletter']) + def test_search(self): """Test that ldap is correctly queried.""" test_query = "äüáíðáßðöó" From 860b2f3647d59c5f1ebba5729b1ec451853d80cf Mon Sep 17 00:00:00 2001 From: Sandro Lutz Date: Wed, 27 Nov 2019 01:59:09 +0100 Subject: [PATCH 2/3] Update position field of eventsignups. - Add documentation for position field. - Add position field to post response. Fixes #405 Fixes #410 --- amivapi/events/__init__.py | 2 ++ amivapi/events/model.py | 6 ++++++ amivapi/events/projections.py | 5 +++++ amivapi/tests/events/test_projections.py | 6 ++++-- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/amivapi/events/__init__.py b/amivapi/events/__init__.py index 2c591f4e..6e9e6ce9 100644 --- a/amivapi/events/__init__.py +++ b/amivapi/events/__init__.py @@ -21,6 +21,7 @@ add_email_to_signup_collection, add_position_to_signup, add_position_to_signup_collection, + add_position_to_signup_on_inserted, add_signup_count_to_event, add_signup_count_to_event_collection ) @@ -49,6 +50,7 @@ def init_app(app): # Show user's position in the signup list app.on_fetched_resource_eventsignups += add_position_to_signup_collection app.on_fetched_item_eventsignups += add_position_to_signup + app.on_inserted_eventsignups += add_position_to_signup_on_inserted # Show signup count in events app.on_fetched_resource_events += add_signup_count_to_event_collection diff --git a/amivapi/events/model.py b/amivapi/events/model.py index b02ac554..65cd8d2a 100644 --- a/amivapi/events/model.py +++ b/amivapi/events/model.py @@ -600,6 +600,12 @@ 'type': 'boolean', 'admin_only': True }, + 'position': { + 'description': "Position of the signup within all signups of " + "the event.", + 'readonly': True, + 'type': 'integer', + } } } } diff --git a/amivapi/events/projections.py b/amivapi/events/projections.py index 24ae738d..1b11880b 100644 --- a/amivapi/events/projections.py +++ b/amivapi/events/projections.py @@ -37,6 +37,11 @@ def add_position_to_signup_collection(response): add_position_to_signup(item) +def add_position_to_signup_on_inserted(items): + for item in items: + add_position_to_signup(item) + + def add_signup_count_to_event(item): """After an event is fetched from the database we add the current signup count""" diff --git a/amivapi/tests/events/test_projections.py b/amivapi/tests/events/test_projections.py index a4fc2e27..7be098bd 100644 --- a/amivapi/tests/events/test_projections.py +++ b/amivapi/tests/events/test_projections.py @@ -63,8 +63,8 @@ def test_signup_count_projected(self): self.assertEqual(event['signup_count'], 100) self.assertEqual(event['unaccepted_count'], 1) - def test_waitinglist_position_projection(self): - """Test that waiting list position is correctly inserted into a + def test_signuplist_position_projection(self): + """Test that signup list position is correctly inserted into a signup information""" with freeze_time("2016-01-01 00:00:00") as frozen_time: # Create a new event @@ -93,6 +93,8 @@ def test_waitinglist_position_projection(self): 'event': str(event['_id']), 'user': str(late_user['_id']) }, status_code=201).json + # Check that position is part of the received entity + self.assertEqual(signup['position'], 4) # GET the late user's signup to check his position signup_info = self.api.get( From 9b6a9cfb65450d3e49142fc632361aad5a792639 Mon Sep 17 00:00:00 2001 From: Sandro Lutz Date: Wed, 27 Nov 2019 17:17:25 +0100 Subject: [PATCH 3/3] Allow admins to overwrite FCFS decision for POST requests. Even if FCFS in enabled, admins should be allowed to force a signup to be accepted. This worked as intended for PATCH requests already, and now also works for POST. Fixes #381 --- amivapi/events/queue.py | 6 ++-- amivapi/tests/events/test_queue.py | 45 +++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/amivapi/events/queue.py b/amivapi/events/queue.py index 3653cd8c..01b10f40 100644 --- a/amivapi/events/queue.py +++ b/amivapi/events/queue.py @@ -4,7 +4,7 @@ # you to buy us beer if we meet and you like the software. """Logic to implement different signup queues.""" -from flask import current_app +from flask import current_app, g from pymongo import ASCENDING from amivapi.events.emails import notify_signup_accepted @@ -65,7 +65,9 @@ def update_waiting_list(event_id): def add_accepted_before_insert(signups): """Add the accepted field before inserting signups.""" for signup in signups: - signup['accepted'] = False + # Admins may provide a value for `accepted`. + # If not provided or not admin set it to false`. + signup['accepted'] = g.resource_admin and signup.get('accepted', False) def update_waiting_list_after_insert(signups): diff --git a/amivapi/tests/events/test_queue.py b/amivapi/tests/events/test_queue.py index 45ab4efd..25ffa0c4 100644 --- a/amivapi/tests/events/test_queue.py +++ b/amivapi/tests/events/test_queue.py @@ -4,7 +4,41 @@ # you to buy us beer if we meet and you like the software. """Test that people are correctly added and removed from the waiting list""" -from amivapi.tests.utils import WebTestNoAuth +from amivapi.tests.utils import WebTestNoAuth, WebTest + + +class EventsignupQueuePermissionTest(WebTest): + def test_fcfs_users_cannot_provide_accepted(self): + """Test that with fcfs admins can provide accepted + field while normal users cannot""" + event = self.new_object('events', spots=1, + selection_strategy='fcfs') + + user1 = self.new_object('users') + user2 = self.new_object('users') + + user1_signup = self.api.post('/eventsignups', data={ + 'user': str(user1['_id']), + 'event': str(event['_id']) + }, token=self.get_user_token(user1['_id']), status_code=201).json + + self.assertTrue(user1_signup['accepted']) + + # Check that a normal user cannot provide the accepted field + self.api.post('/eventsignups', data={ + 'user': str(user2['_id']), + 'event': str(event['_id']), + 'accepted': True + }, token=self.get_user_token(user2['_id']), status_code=422) + + # Check that admins can always provide the accepted field + user2_signup = self.api.post('/eventsignups', data={ + 'user': str(user2['_id']), + 'event': str(event['_id']), + 'accepted': True + }, token=self.get_root_token(), status_code=201).json + + self.assertTrue(user2_signup['accepted']) class EventsignupQueueTest(WebTestNoAuth): @@ -55,6 +89,15 @@ def test_fcfs_users_get_auto_accepted(self): status_code=200).json self.assertTrue(user2_signup['accepted']) + # post accepted signup as admin + user1_signup = self.api.post('/eventsignups', data={ + 'user': str(user1['_id']), + 'event': str(event['_id']), + 'accepted': True + }, status_code=201).json + + self.assertTrue(user1_signup['accepted']) + def test_fcfs_users_get_auto_accepted_unlimited_spots(self): """Test that with fcfs the users get automatically accepted on signup for events with unlimited spaces"""