diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 8e61ff297b..41e865f444 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -10,6 +10,12 @@ below features are supported. ## Changes in Zulip 3.0 +**Feature level 23** + +* `GET/PUT/POST /users/me/pointer`: Eliminated. We eliminated + the whole concept of the "global" user pointer leading up to + this commit. + **Feature level 22** * `GET /attachments`: The date when a message using the attachment was diff --git a/version.py b/version.py index a023dffc78..fbc8fad881 100644 --- a/version.py +++ b/version.py @@ -29,7 +29,7 @@ DESKTOP_WARNING_VERSION = "5.2.0" # # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md. -API_FEATURE_LEVEL = 22 +API_FEATURE_LEVEL = 23 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 5c6c0203a8..4044a61b31 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -4031,42 +4031,6 @@ def update_user_presence(user_profile: UserProfile, client: Client, log_time: da if new_user_input: update_user_activity_interval(user_profile, log_time) -def do_update_pointer(user_profile: UserProfile, client: Client, - pointer: int, update_flags: bool=False) -> None: - prev_pointer = user_profile.pointer - user_profile.pointer = pointer - user_profile.save(update_fields=["pointer"]) - - if update_flags: # nocoverage - # This block of code is compatibility code for the - # legacy/original Zulip Android app natively. It's a shim - # that will mark as read any messages up until the pointer - # move; we expect to remove this feature entirely before long, - # when we drop support for the old Android app entirely. - app_message_ids = UserMessage.objects.filter( - user_profile=user_profile, - message__id__gt=prev_pointer, - message__id__lte=pointer).extra(where=[ - UserMessage.where_unread(), - UserMessage.where_active_push_notification(), - ]).values_list("message_id", flat=True) - - UserMessage.objects.filter(user_profile=user_profile, - message__id__gt=prev_pointer, - message__id__lte=pointer).extra(where=[UserMessage.where_unread()]) \ - .update(flags=F('flags').bitor(UserMessage.flags.read)) - do_clear_mobile_push_notifications_for_ids([user_profile.id], app_message_ids) - event_time = timezone_now() - - count = len(app_message_ids) - do_increment_logging_stat(user_profile, COUNT_STATS['messages_read::hour'], - None, event_time, increment=count) - do_increment_logging_stat(user_profile, COUNT_STATS['messages_read_interactions::hour'], - None, event_time, increment=min(1, count)) - - event = dict(type='pointer', pointer=pointer) - send_event(user_profile.realm, event, [user_profile.id]) - def do_update_user_status(user_profile: UserProfile, away: Optional[bool], status_text: Optional[str], diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 6a1b22ac3b..ec3506959f 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -131,9 +131,6 @@ def fetch_initial_state_data(user_profile: UserProfile, if want('muted_topics'): state['muted_topics'] = get_topic_mutes(user_profile) - if want('pointer'): - state['pointer'] = user_profile.pointer - if want('presence'): state['presences'] = get_presences_for_realm(realm, slim_presence) @@ -398,8 +395,6 @@ def apply_event(state: Dict[str, Any], state['hotspots'] = event['hotspots'] elif event['type'] == "custom_profile_fields": state['custom_profile_fields'] = event['fields'] - elif event['type'] == "pointer": - state['pointer'] = max(state['pointer'], event['pointer']) elif event['type'] == "realm_user": person = event['person'] person_user_id = person['user_id'] diff --git a/zerver/openapi/openapi.py b/zerver/openapi/openapi.py index 5c5c3bff1c..0a54e2e1c1 100644 --- a/zerver/openapi/openapi.py +++ b/zerver/openapi/openapi.py @@ -20,7 +20,7 @@ EXCLUDE_PROPERTIES = { }, '/register': { 'post': { - '200': ['max_message_id', 'realm_emoji', 'pointer'], + '200': ['max_message_id', 'realm_emoji'], }, }, '/settings/notifications': { diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index a7c4405d96..2e9a11b21a 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -86,7 +86,6 @@ from zerver.lib.actions import ( do_update_message, do_update_message_flags, do_update_outgoing_webhook_service, - do_update_pointer, do_update_user_custom_profile_data_if_changed, do_update_user_group_description, do_update_user_group_name, @@ -293,9 +292,7 @@ class EventsEndpointTest(ZulipTestCase): # Check that we didn't fetch the messages data self.assertNotIn('max_message_id', result_dict) - # Check that the realm_emoji data is in there, and is correctly - # updated (presering our atomicity guaranteed), though of - # course any future pointer events won't be distributed + # Check that the realm_emoji data is in there. self.assertIn('realm_emoji', result_dict) self.assertEqual(result_dict['realm_emoji'], []) self.assertEqual(result_dict['queue_id'], '15:13') @@ -1252,14 +1249,6 @@ class EventsRegisterTest(ZulipTestCase): self.user_profile, get_client("ZulipAndroid/1.0"), timezone_now(), UserPresence.IDLE)) schema_checker_android('events[0]', events[0]) - def test_pointer_events(self) -> None: - schema_checker = self.check_events_dict([ - ('type', equals('pointer')), - ('pointer', check_int), - ]) - events = self.do_test(lambda: do_update_pointer(self.user_profile, get_client("website"), 1500)) - schema_checker('events[0]', events[0]) - def test_register_events(self) -> None: realm_user_add_checker = self.check_events_dict([ ('type', equals('realm_user')), @@ -3801,7 +3790,6 @@ class FetchQueriesTest(ZulipTestCase): hotspots=0, message=1, muted_topics=1, - pointer=0, presence=1, realm=0, realm_bot=1, diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index ad8b148a63..00ae60fee6 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -122,7 +122,6 @@ class HomeTest(ZulipTestCase): "password_min_guesses", "password_min_length", "pm_content_in_desktop_notifications", - "pointer", "poll_timeout", "presence_enabled", "presences", @@ -456,15 +455,6 @@ class HomeTest(ZulipTestCase): self.assertEqual(mock.call_args_list[0][0][0], "Invalid narrow requested, ignoring") self._sanity_check(result) - def test_bad_pointer(self) -> None: - user_profile = self.example_user('hamlet') - user_profile.pointer = 999999 - user_profile.save() - - self.login_user(user_profile) - result = self._get_home_page() - self._sanity_check(result) - def test_topic_narrow(self) -> None: self.login('hamlet') result = self._get_home_page(stream='Denmark', topic='lunch') @@ -658,7 +648,6 @@ class HomeTest(ZulipTestCase): page_params = self._get_page_params(result) self.assertEqual(page_params['narrow_stream'], stream_name) self.assertEqual(page_params['narrow'], [dict(operator='stream', operand=stream_name)]) - self.assertEqual(page_params['pointer'], -1) self.assertEqual(page_params['max_message_id'], -1) def test_invites_by_admins_only(self) -> None: diff --git a/zerver/tests/test_openapi.py b/zerver/tests/test_openapi.py index 6fd05f3ea9..0a0b403408 100644 --- a/zerver/tests/test_openapi.py +++ b/zerver/tests/test_openapi.py @@ -283,8 +283,6 @@ class OpenAPIArgumentsTest(ZulipTestCase): '/settings/display', # Much more valuable would be an org admin bulk-upload feature. '/users/me/profile_data', - # To be deprecated and deleted. - '/users/me/pointer', #### Should be documented as part of interactive bots documentation '/bot_storage', diff --git a/zerver/tests/test_unread.py b/zerver/tests/test_unread.py index c054d31c07..afb9d9f1fd 100644 --- a/zerver/tests/test_unread.py +++ b/zerver/tests/test_unread.py @@ -8,77 +8,19 @@ from zerver.lib.fix_unreads import fix, fix_unsubscribed from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import get_subscription, tornado_redirected_to_list from zerver.lib.topic_mutes import add_topic_mute -from zerver.models import Subscription, UserMessage, UserProfile, get_realm, get_stream, get_user +from zerver.models import Subscription, UserMessage, UserProfile, get_realm, get_stream -class PointerTest(ZulipTestCase): +class FirstUnreadAnchorTests(ZulipTestCase): + ''' + HISTORICAL NOTE: - def test_update_pointer(self) -> None: - """ - Posting a pointer to /update (in the form {"pointer": pointer}) changes - the pointer we store for your UserProfile. - """ + The two tests in this class were originally written when + we had the concept of a "pointer", and they may be a bit + redundant in what they now check. + ''' + def test_use_first_unread_anchor(self) -> None: self.login('hamlet') - self.assertEqual(self.example_user('hamlet').pointer, -1) - msg_id = self.send_stream_message(self.example_user("othello"), "Verona") - result = self.client_post("/json/users/me/pointer", {"pointer": msg_id}) - self.assert_json_success(result) - self.assertEqual(self.example_user('hamlet').pointer, msg_id) - - def test_api_update_pointer(self) -> None: - """ - Same as above, but for the API view - """ - user = self.example_user('hamlet') - email = user.email - self.assertEqual(user.pointer, -1) - msg_id = self.send_stream_message(self.example_user("othello"), "Verona") - result = self.api_post(user, "/api/v1/users/me/pointer", {"pointer": msg_id}) - self.assert_json_success(result) - self.assertEqual(get_user(email, user.realm).pointer, msg_id) - - def test_missing_pointer(self) -> None: - """ - Posting json to /json/users/me/pointer which does not contain a pointer key/value pair - returns a 400 and error message. - """ - self.login('hamlet') - self.assertEqual(self.example_user('hamlet').pointer, -1) - result = self.client_post("/json/users/me/pointer", {"foo": 1}) - self.assert_json_error(result, "Missing 'pointer' argument") - self.assertEqual(self.example_user('hamlet').pointer, -1) - - def test_invalid_pointer(self) -> None: - """ - Posting json to /json/users/me/pointer with an invalid pointer returns a 400 and error - message. - """ - self.login('hamlet') - self.assertEqual(self.example_user('hamlet').pointer, -1) - result = self.client_post("/json/users/me/pointer", {"pointer": "foo"}) - self.assert_json_error(result, "Bad value for 'pointer': foo") - self.assertEqual(self.example_user('hamlet').pointer, -1) - - def test_pointer_out_of_range(self) -> None: - """ - Posting json to /json/users/me/pointer with an out of range (< 0) pointer returns a 400 - and error message. - """ - self.login('hamlet') - self.assertEqual(self.example_user('hamlet').pointer, -1) - result = self.client_post("/json/users/me/pointer", {"pointer": -2}) - self.assert_json_error(result, "Bad value for 'pointer': -2") - self.assertEqual(self.example_user('hamlet').pointer, -1) - - def test_use_first_unread_anchor_interaction_with_pointer(self) -> None: - """ - Getting old messages (a get request to /json/messages) should never - return an unread message older than the current pointer, when there's - no narrow set. - """ - self.login('hamlet') - # Ensure the pointer is not set (-1) - self.assertEqual(self.example_user('hamlet').pointer, -1) # Mark all existing messages as read result = self.client_post("/json/mark_all_as_read") @@ -107,7 +49,6 @@ class PointerTest(ZulipTestCase): messages = self.get_messages( anchor=0, num_before=0, num_after=2, use_first_unread_anchor=False) old_message_id = messages[0]['id'] - next_old_message_id = messages[1]['id'] # Verify the message is marked as read user_message = UserMessage.objects.get( @@ -135,23 +76,8 @@ class PointerTest(ZulipTestCase): self.assertEqual(messages_response['messages'][0]['id'], old_message_id) self.assertEqual(messages_response['anchor'], old_message_id) - # Let's update the pointer to be *after* this old unread message (but - # still on or before the new unread message we just sent) - result = self.client_post("/json/users/me/pointer", - {"pointer": next_old_message_id}) - self.assert_json_success(result) - self.assertEqual(self.example_user('hamlet').pointer, - next_old_message_id) - - # Verify that moving the pointer didn't mark our message as read. - user_message = UserMessage.objects.get( - message_id=old_message_id, - user_profile=self.example_user('hamlet')) - self.assertFalse(user_message.flags.read) - def test_visible_messages_use_first_unread_anchor(self) -> None: self.login('hamlet') - self.assertEqual(self.example_user('hamlet').pointer, -1) result = self.client_post("/json/mark_all_as_read") self.assert_json_success(result) @@ -184,6 +110,7 @@ class PointerTest(ZulipTestCase): anchor="first_unread", num_before=0, num_after=1) self.assert_length(messages, 1) + class UnreadCountTests(ZulipTestCase): def setUp(self) -> None: super().setUp() diff --git a/zerver/tests/test_urls.py b/zerver/tests/test_urls.py index f30c6a5c41..9b4747a4dc 100644 --- a/zerver/tests/test_urls.py +++ b/zerver/tests/test_urls.py @@ -64,7 +64,6 @@ class PublicURLTest(ZulipTestCase): "/json/subscriptions/exists", "/api/v1/users/me/subscriptions/properties", "/json/fetch_api_key", - "/json/users/me/pointer", "/json/users/me/subscriptions", "/api/v1/users/me/subscriptions", "/json/export/realm", @@ -76,16 +75,13 @@ class PublicURLTest(ZulipTestCase): patch_urls = { 401: ["/json/settings"], } - put_urls = {401: ["/json/users/me/pointer"], - } + for status_code, url_set in get_urls.items(): self.fetch("client_get", url_set, status_code) for status_code, url_set in post_urls.items(): self.fetch("client_post", url_set, status_code) for status_code, url_set in patch_urls.items(): self.fetch("client_patch", url_set, status_code) - for status_code, url_set in put_urls.items(): - self.fetch("client_put", url_set, status_code) def test_get_gcid_when_not_configured(self) -> None: with self.settings(GOOGLE_CLIENT_ID=None): diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 423e073671..bc45dfa87d 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -1531,25 +1531,6 @@ class BulkUsersTest(ZulipTestCase): class GetProfileTest(ZulipTestCase): - def common_update_pointer(self, user: UserProfile, pointer: int) -> None: - self.login_user(user) - result = self.client_post("/json/users/me/pointer", {"pointer": pointer}) - self.assert_json_success(result) - - def common_get_pointer(self, user_id: str) -> Dict[str, Any]: - user_profile = self.example_user(user_id) - result = self.api_get(user_profile, "/json/users/me/pointer") - self.assert_json_success(result) - json = result.json() - return json - - def test_get_pointer(self) -> None: - user = self.example_user("hamlet") - self.login_user(user) - result = self.client_get("/json/users/me/pointer") - self.assert_json_success(result) - self.assertIn("pointer", result.json()) - def test_cache_behavior(self) -> None: """Tests whether fetching a user object the normal way, with `get_user`, makes 1 cache query and 1 database query. @@ -1617,33 +1598,6 @@ class GetProfileTest(ZulipTestCase): self.assertEqual(result['user']['email'], bot.email) self.assertTrue(result['user']['is_bot']) - def test_api_get_empty_profile(self) -> None: - """ - Ensure GET /users/me returns a max message id and returns successfully - """ - json = self.common_get_pointer("othello") - self.assertEqual(json['pointer'], -1) - - def test_profile_with_pointer(self) -> None: - """ - Ensure GET /users/me returns a proper pointer id after the pointer is updated - """ - - id1 = self.send_stream_message(self.example_user("othello"), "Verona") - id2 = self.send_stream_message(self.example_user("othello"), "Verona") - - hamlet = self.example_user('hamlet') - self.common_update_pointer(hamlet, id2) - json = self.common_get_pointer("hamlet") - self.assertEqual(json["pointer"], id2) - - self.common_update_pointer(hamlet, id1) - json = self.common_get_pointer("hamlet") - self.assertEqual(json["pointer"], id2) # pointer does not move backwards - - result = self.client_post("/json/users/me/pointer", {"pointer": 99999999}) - self.assert_json_error(result, "Invalid message ID") - def test_get_all_profiles_avatar_urls(self) -> None: hamlet = self.example_user('hamlet') result = self.api_get(hamlet, "/api/v1/users") diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index 1dcb4b42ad..67bd648dbc 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -303,7 +303,7 @@ class EventQueue: event['id'] = self.next_event_id self.next_event_id += 1 full_event_type = compute_full_event_type(event) - if (full_event_type in ["pointer", "restart"] or + if (full_event_type == "restart" or full_event_type.startswith("flags/")): if full_event_type not in self.virtual_events: self.virtual_events[full_event_type] = copy.deepcopy(event) @@ -313,9 +313,8 @@ class EventQueue: virtual_event["id"] = event["id"] if "timestamp" in event: virtual_event["timestamp"] = event["timestamp"] - if full_event_type == "pointer": - virtual_event["pointer"] = event["pointer"] - elif full_event_type == "restart": + + if full_event_type == "restart": virtual_event["server_generation"] = event["server_generation"] elif full_event_type.startswith("flags/"): virtual_event["messages"] += event["messages"] diff --git a/zerver/views/home.py b/zerver/views/home.py index 03774178da..8969358e17 100644 --- a/zerver/views/home.py +++ b/zerver/views/home.py @@ -212,15 +212,6 @@ def home_real(request: HttpRequest) -> HttpResponse: ) needs_tutorial = user_profile.tutorial_status == UserProfile.TUTORIAL_WAITING - if user_profile.pointer == -1: - # Put the new user's pointer at the bottom - # - # This improves performance, because we limit backfilling of messages - # before the pointer. It's also likely that someone joining an - # organization is interested in recent messages more than the very - # first messages on the system. - register_ret['pointer'] = register_ret['max_message_id'] - else: # nocoverage first_in_realm = False prompt_for_invites = False @@ -291,7 +282,6 @@ def home_real(request: HttpRequest) -> HttpResponse: page_params["narrow_topic"] = narrow_topic page_params["narrow"] = [dict(operator=term[0], operand=term[1]) for term in narrow] page_params["max_message_id"] = initial_pointer - page_params["pointer"] = initial_pointer page_params["enable_desktop_notifications"] = False statsd.incr('views.home') diff --git a/zerver/views/pointer.py b/zerver/views/pointer.py deleted file mode 100644 index 08e9a0756e..0000000000 --- a/zerver/views/pointer.py +++ /dev/null @@ -1,27 +0,0 @@ -from django.http import HttpRequest, HttpResponse -from django.utils.translation import ugettext as _ - -from zerver.lib.actions import do_update_pointer -from zerver.lib.request import REQ, JsonableError, has_request_variables -from zerver.lib.response import json_success -from zerver.lib.validator import to_non_negative_int -from zerver.models import UserProfile, get_usermessage_by_message_id - - -def get_pointer_backend(request: HttpRequest, user_profile: UserProfile) -> HttpResponse: - return json_success({'pointer': user_profile.pointer}) - -@has_request_variables -def update_pointer_backend(request: HttpRequest, user_profile: UserProfile, - pointer: int=REQ(converter=to_non_negative_int)) -> HttpResponse: - if pointer <= user_profile.pointer: - return json_success() - - if get_usermessage_by_message_id(user_profile, pointer) is None: - raise JsonableError(_("Invalid message ID")) - - request._log_data["extra"] = f"[{pointer}]" - update_flags = (request.client.name.lower() in ['android', "zulipandroid"]) - do_update_pointer(user_profile, request.client, pointer, update_flags=update_flags) - - return json_success() diff --git a/zproject/urls.py b/zproject/urls.py index c0ba941a2a..f3b0a06a3d 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -260,12 +260,6 @@ v1_api_and_json_patterns = [ path('users/me', rest_dispatch, {'GET': 'zerver.views.users.get_profile_backend', 'DELETE': 'zerver.views.users.deactivate_user_own_backend'}), - # PUT is currently used by mobile apps, we intend to remove the PUT version - # as soon as possible. POST exists to correct the erroneous use of PUT. - path('users/me/pointer', rest_dispatch, - {'GET': 'zerver.views.pointer.get_pointer_backend', - 'PUT': 'zerver.views.pointer.update_pointer_backend', - 'POST': 'zerver.views.pointer.update_pointer_backend'}), path('users/me/presence', rest_dispatch, {'POST': 'zerver.views.presence.update_active_status_backend'}), path('users/me/status', rest_dispatch,