From 48d2783559dd5970fc3c5c0459a5f058e7682f81 Mon Sep 17 00:00:00 2001 From: Dinesh Date: Thu, 17 Jun 2021 00:45:47 +0530 Subject: [PATCH] read_receipts: Add support for displaying read receipts. Adds an API endpoint for accessing read receipts for other users, as well as a modal UI for displaying that information. Enables the previously merged privacy settings UI for managing whether a user makes read receipts data available to other users. Documentation is pending, and we'll likely want to link to the documentation with help_settings_link once it is complete. Fixes #3618. Co-authored-by: Tim Abbott --- static/js/people.js | 2 +- static/js/popovers.js | 13 ++ static/js/read_receipts.js | 63 ++++++ static/shared/icons/readreceipts.svg | Bin 0 -> 5461 bytes static/styles/dark_theme.css | 11 + static/styles/modal.css | 1 + static/styles/zulip.css | 81 +++++++ static/templates/actions_popover_content.hbs | 8 + static/templates/read_receipts.hbs | 8 + static/templates/read_receipts_modal.hbs | 19 ++ .../templates/settings/account_settings.hbs | 2 +- templates/zerver/api/changelog.md | 2 + .../zerver/help/include/rest-endpoints.md | 1 + tools/test-js-with-node | 1 + version.py | 2 +- zerver/actions/create_realm.py | 14 ++ zerver/models.py | 4 + zerver/openapi/python_examples.py | 10 + zerver/openapi/zulip.yaml | 64 ++++++ zerver/tests/test_read_receipts.py | 208 ++++++++++++++++++ zerver/views/read_receipts.py | 75 +++++++ zilencer/management/commands/populate_db.py | 1 + zproject/urls.py | 3 + 23 files changed, 590 insertions(+), 3 deletions(-) create mode 100644 static/js/read_receipts.js create mode 100644 static/shared/icons/readreceipts.svg create mode 100644 static/templates/read_receipts.hbs create mode 100644 static/templates/read_receipts_modal.hbs create mode 100644 zerver/tests/test_read_receipts.py create mode 100644 zerver/views/read_receipts.py diff --git a/static/js/people.js b/static/js/people.js index fd5634d1cd..88acdbeddb 100644 --- a/static/js/people.js +++ b/static/js/people.js @@ -1362,7 +1362,7 @@ export function is_my_user_id(user_id) { return user_id === my_user_id; } -function compare_by_name(a, b) { +export function compare_by_name(a, b) { return util.strcmp(a.full_name, b.full_name); } diff --git a/static/js/popovers.js b/static/js/popovers.js index e9295c0e00..699e506873 100644 --- a/static/js/popovers.js +++ b/static/js/popovers.js @@ -38,6 +38,7 @@ import * as overlays from "./overlays"; import {page_params} from "./page_params"; import * as people from "./people"; import * as popover_menus from "./popover_menus"; +import * as read_receipts from "./read_receipts"; import * as realm_playground from "./realm_playground"; import * as reminder from "./reminder"; import * as resize from "./resize"; @@ -509,6 +510,9 @@ export function toggle_actions_popover(element, id) { const should_display_delete_option = message_edit.get_deletability(message) && not_spectator; + const should_display_read_receipts_option = + page_params.realm_enable_read_receipts && not_spectator; + const args = { message_id: message.id, historical: message.historical, @@ -523,6 +527,7 @@ export function toggle_actions_popover(element, id) { conversation_time_uri, narrowed: narrow_state.active(), should_display_delete_option, + should_display_read_receipts_option, should_display_reminder_option: feature_flags.reminders_in_message_action_menu, should_display_edit_and_view_source, should_display_quote_and_reply, @@ -1235,6 +1240,14 @@ export function register_click_handlers() { e.preventDefault(); }); + $("body").on("click", ".view_read_receipts", (e) => { + const message_id = $(e.currentTarget).data("message-id"); + hide_actions_popover(); + read_receipts.show_user_list(message_id); + e.stopPropagation(); + e.preventDefault(); + }); + $("body").on("click", ".delete_message", (e) => { const message_id = $(e.currentTarget).data("message-id"); hide_actions_popover(); diff --git a/static/js/read_receipts.js b/static/js/read_receipts.js new file mode 100644 index 0000000000..c862ffdb4a --- /dev/null +++ b/static/js/read_receipts.js @@ -0,0 +1,63 @@ +import $ from "jquery"; +import SimpleBar from "simplebar"; + +import render_read_receipts from "../templates/read_receipts.hbs"; +import render_read_receipts_modal from "../templates/read_receipts_modal.hbs"; + +import * as channel from "./channel"; +import {$t} from "./i18n"; +import * as loading from "./loading"; +import * as overlays from "./overlays"; +import * as people from "./people"; +import * as popovers from "./popovers"; +import * as ui_report from "./ui_report"; + +export function show_user_list(message_id) { + $("body").append(render_read_receipts_modal()); + overlays.open_modal("read_receipts_modal", { + autoremove: true, + on_show: () => { + loading.make_indicator($("#read_receipts_modal .loading_indicator")); + channel.get({ + url: `/json/messages/${message_id}/read_receipts`, + idempotent: true, + success(data) { + const users = data.user_ids.map((id) => people.get_by_user_id(id)); + users.sort(people.compare_by_name); + + loading.destroy_indicator($("#read_receipts_modal .loading_indicator")); + if (users.length === 0) { + $("#read_receipts_modal .read_receipts_info").text( + $t({defaultMessage: "No one has read this message yet."}), + ); + } else { + $("#read_receipts_modal .read_receipts_info").text( + $t( + { + defaultMessage: + "This message has been read by {num_of_people} people:", + }, + {num_of_people: users.length}, + ), + ); + $("#read_receipts_modal .modal__container").addClass( + "showing_read_receipts_list", + ); + $("#read_receipts_modal .modal__content").append( + render_read_receipts({users}), + ); + new SimpleBar($("#read_receipts_modal .modal__content")[0]); + } + }, + error(xhr) { + ui_report.error("", xhr, $("#read_receipts_error")); + loading.destroy_indicator($("#read_receipts_modal .loading_indicator")); + }, + }); + }, + on_hide: () => { + // Ensure any user info popovers are closed + popovers.hide_all(); + }, + }); +} diff --git a/static/shared/icons/readreceipts.svg b/static/shared/icons/readreceipts.svg new file mode 100644 index 0000000000000000000000000000000000000000..fb59f33c9550437228445d1ff838ae4dc28ae139 GIT binary patch literal 5461 zcmbW5+j1Mn5r*&e6dU-)t^$^GJ}{zONnB1+W#1%@FMJ09D-tCVU;&^g>FM)z&n`eq zqRNR?uI2R1^!(HP_hInz`}cRZ-Me9b*zC4fGuxY4H*8nC^=5m0HT%ymKQ79wI~vFQG`AC|{q-92uOH{CzCZx5^G{m^}Tb3EQ(UR*prKK7eY z*zfk&7vFV@#j6)DULM|Ezvw!4t&cZXvuNE+hBw3J`sR2ZzuOFtf8D*mnwidYERz3q z;S;P@1)KHN?BBos{a?G?QO5_s-}C?Fdbipx@4)b_ltp}eWX zI){1ZjI#?97cP9KTd>jga5>%PYWCa}<+#KBay1+-ro~V8Pry$5CqaMMJ?vM*8>}?+ z+u?Ze!!JKvOp^=KuaE1qn$vDE@S|7LI%CYmbiW3c(N|Z#!ssujGcE=AT1+mk=sGd! zNn}GdUeE8k`{nhp+THH-Tk$>aF>)n1m?5C+g|NGt44zp~IAE4MMo(yy;IuAF? z_3ja8e`)fx^^3Esn_=~K*uUN__n`i42_$&98@9*A!*+8#T+Qy^k4GGCc8}M4x#@BL zFnoIM$KCEu!XXsra}A$PuOO!PrY|Mc_}L`4bUyYu)%5Ay<7Uf67bE#?b)NwqS4{C> zjV;e8k>Ge{3Ixwh-7Vj5?lvC>F8-M-RuB8V*y?uqVZg=unHHVn6%z`3;Lpj2V#r3F zt~-9XC+6ON5ZMT?F-inNraJ2`lzu0UpZTZq_3-9!CZddNNR>p?wrTo6jotrdv2X~)4Sp(r*|D%e(qc!ZH?Z#xn;hjY-{&x7gFh6tT}i8=t3OV zAa~yOE?3_zyPEszO2~6-VxL^9fmCQZ+bu%weTaGPLvL-hLG-!Wr(|preYLL!K}K$4 z?L!JBAm_Z^aB%5Xo#~BnbCQemrk+P3x$JZpWHMFnQ^y}B`K*GRy%zzN@nybn?DszC z5@+Ll!6Ze*hvMh_5sD23br>L|`67-`Y;RLA2vVPX$cYsUYnt0W*BBK*vR2_4?;`U~ zjHn2bvsP3RnNyK^a#e{D+XYx=UmxSO|Gs3D3?&6z_1YkFbfxT zeTdc;y_OfHVgYc-w^(*?zO6-%wayv^Vrp%+0@0j>!Dii%aXrJFbU)6-rLx~UMQLM* z0f7s6TQ*pat*|||Q^Q(Us+AqsNKNC#bZ8V03!tQ~F_28`qN_#CwA{%R1y9uFsrlU$ z8DXHP${HM2iN$hXhJ_f+7(Y+Fp9a9x1A}_Pz*002B5EK^TSVYoYARg_P_tbqsM)hA z&;dy-3C$3sMv99_#8>dqLF}5uzn<$WL9<*i#f{TY#UJT)Lxd-(*&WBkR*VULFAJ-9s9u* zWtq5Ltr2SqV!!GLRpODzDozBB)S_^6gn9_LL{k#7BO1s}?Y>x4@)ZK3ehxV{W+lQ# z%u!3bh4)d;lrS$)m-RJiB)A%lD0HDjEy-mFDr=>ybKm%xJ3r_)FrZbq6L zRgH6GAq7NTVN=cu##MyVT{83Fw9ws;8OdMHSPDLZRc z3ss-V2C^d;1eQNcl=R@e6W)?h_^J&dtD3-(uT&}7P)Vst98)${IFhq^!T6{~wZ4*H zxTMc54VQ=`8Rw+qAepKA*ik={!ghFfhohMBc$dBQP(@J=$mB@KjmL{)GC zk6k21QBjph5*m!8R`HQa0X9;uOHlgz+S`KRzG<60F$YZXTK!e4YU(fpe=MQ}uOuq14m z*Gej8Q{wRrSuNgj8-Uc}W9B>61Yt32P0L&QH)&MVKr$T?sLi>thqC0s+DEeWh>iWS zonAyfiW6NjkV-}5wh2wgnhFfXxgFUu=SU$#fyjN-xaS}#G^(+gZ_?k`h#p}ZacT)* zj){0CPJlGBPYN7?EI69F(9o)DPGALVP>!|)BbiBLUO71e(Bw?-(M&>w%Zk>B2#Y6j z#ilh;u$B8XE27Ig&0|%WWuj6?f=umIXi``sHB3Hj&>9Jw6za(p#F?y#P*A>(#eA&^ zqZ@;1H9&bUE2LKuTUq{=h@`QaB2%VYnQVo3@?PszB(z!3Vs>(+>}VEXS}ibokKh`c zavlSp_Rlh=MAzx<_y_vu5XrPPpI!~xKa-35>~fkrPrDF#3Uq(zBo#*bU7BM_ikaf4 z&9R|@CYmUH+&Rg{N(1}+>a+3+zs*SoFO2~hd@#iEYv($iOvOP|$AQx7R5kN-iPK>Y3OT6qdhPopn9f=>QA zJ)3BxPQvM}o2M7)`Ny7c7V(&^=aIaiS>GOfaO?j7aAwg6fZpqbX?k6M4S?szjT3;U zi-Z&{pCySd9E6k(lqIyP8Ne)YDLCCkSMfx`K|a#8n2sJ) zQWu}#DPBBYbENaikegpux?3brlLni$s-`?wM7-R(|K>8O<{^P`6}bx{ literal 0 HcmV?d00001 diff --git a/static/styles/dark_theme.css b/static/styles/dark_theme.css index dd6b9e37ef..b400bbabdc 100644 --- a/static/styles/dark_theme.css +++ b/static/styles/dark_theme.css @@ -1205,6 +1205,17 @@ body.dark-theme { color: hsl(0, 0%, 100%); } } + + #read_receipts_modal #read_receipts_list li { + &:hover { + background-color: hsla(0, 0%, 100%, 0.05); + } + + &:active, + &:focus { + background-color: hsla(0, 0%, 100%, 0.1); + } + } } @supports (-moz-appearance: none) { diff --git a/static/styles/modal.css b/static/styles/modal.css index 4d64b6af12..0c621fc58c 100644 --- a/static/styles/modal.css +++ b/static/styles/modal.css @@ -120,6 +120,7 @@ color: hsl(0, 0%, 100%) !important; } +#read_receipts_error, #dialog_error { margin-bottom: 10px; } diff --git a/static/styles/zulip.css b/static/styles/zulip.css index e1559bfa13..52be2103c3 100644 --- a/static/styles/zulip.css +++ b/static/styles/zulip.css @@ -650,6 +650,78 @@ strong { display: none; } +#read_receipts_modal { + .modal__container { + width: 360px; + + .modal__content { + /* When showing read receipts, we use simplebar + to make the list scrollable. It requires this to + be flex. */ + display: flex; + + /* Setting a minimum height prevents the loading indicator + appearing/disappearing from resizing the modal in the + common case that one is requesting read receipts for + PMs. */ + min-height: 120px; + /* Setting a maximum height is just for aesthetics; the modal looks + weird if its aspect ratio gets too stretched. */ + max-height: 480px; + } + } + + .modal__header { + padding-bottom: 0; + } + + hr { + margin: 10px 0; + } + + .modal__content { + padding: 0 24px 8px; + } + + .loading_indicator { + margin: auto; + } + + #read_receipts_list { + margin-left: 0; + + li { + .read_receipts_user_avatar { + display: inline-block; + height: 20px; + width: 20px; + position: relative; + right: 8px; + border-radius: 4px; + } + + margin: 2px 0; + list-style-type: none; + overflow-x: hidden; + padding-left: 10px; + white-space: nowrap; + text-overflow: ellipsis; + cursor: pointer; + line-height: 26px; + + &:hover { + background-color: hsla(0, 0%, 0%, 0.05); + } + + &:active, + &:focus { + background-color: hsla(0, 0%, 0%, 0.1); + outline: none; + } + } + } +} + /* .dropdown-menu from v2.3.2 + https://github.com/zulip/zulip/commit/7a3a3be7e547d3e8f0ed00820835104867f2433d basic idea of this fix is to remove decorations from :hover and apply them only @@ -931,6 +1003,15 @@ td.pointer { } } + > .view_read_receipts { + font-size: 14px; + height: 16px; + + &:hover { + color: hsl(200, 100%, 40%); + } + } + .edit_content { width: 12px; diff --git a/static/templates/actions_popover_content.hbs b/static/templates/actions_popover_content.hbs index 189640ad33..0e46b1b04c 100644 --- a/static/templates/actions_popover_content.hbs +++ b/static/templates/actions_popover_content.hbs @@ -79,6 +79,14 @@ {{/if}} + {{#if should_display_read_receipts_option}} +
  • + + {{t "View read receipts" }} + +
    • + {{/if}} +
  • {{t "Copy link to message" }} diff --git a/static/templates/read_receipts.hbs b/static/templates/read_receipts.hbs new file mode 100644 index 0000000000..53b7f1837a --- /dev/null +++ b/static/templates/read_receipts.hbs @@ -0,0 +1,8 @@ +
      + {{#each users}} + + {{/each}} +
    diff --git a/static/templates/read_receipts_modal.hbs b/static/templates/read_receipts_modal.hbs new file mode 100644 index 0000000000..285e13737b --- /dev/null +++ b/static/templates/read_receipts_modal.hbs @@ -0,0 +1,19 @@ + diff --git a/static/templates/settings/account_settings.hbs b/static/templates/settings/account_settings.hbs index 5b8c6118ed..1cbf08886e 100644 --- a/static/templates/settings/account_settings.hbs +++ b/static/templates/settings/account_settings.hbs @@ -64,6 +64,7 @@ is_checked=settings_object.send_stream_typing_notifications label=settings_label.send_stream_typing_notifications }} + {{/if}} {{> settings_checkbox setting_name="send_read_receipts" is_checked=settings_object.send_read_receipts @@ -71,7 +72,6 @@ tooltip_text=send_read_receipts_tooltip hide_tooltip=page_params.realm_enable_read_receipts }} - {{/if}} diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 4e3ebba48a..ce1e7b522f 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -22,6 +22,8 @@ format used by the Zulip server that they are interacting with. **Feature level 137** +* [`GET /messages/{message_id}/read_receipts`](/api/get-read-receipts): + Added new endpoint to fetch read receipts for a message. * [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events), `PATCH /realm`: Added new `enable_read_receipts` realm setting. diff --git a/templates/zerver/help/include/rest-endpoints.md b/templates/zerver/help/include/rest-endpoints.md index b00553bd85..bf5c78282d 100644 --- a/templates/zerver/help/include/rest-endpoints.md +++ b/templates/zerver/help/include/rest-endpoints.md @@ -14,6 +14,7 @@ * [Get a message's edit history](/api/get-message-history) * [Update personal message flags](/api/update-message-flags) * [Mark messages as read in bulk](/api/mark-all-as-read) +* [Get a message's read receipts](/api/get-read-receipts) #### Drafts diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 0b678833d2..c7edff0a12 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -128,6 +128,7 @@ EXEMPT_FILES = make_set( "static/js/poll_widget.js", "static/js/popover_menus.js", "static/js/popovers.js", + "static/js/read_receipts.js", "static/js/ready.ts", "static/js/realm_icon.js", "static/js/realm_logo.js", diff --git a/version.py b/version.py index 1bf81ae238..bd2d484eb1 100644 --- a/version.py +++ b/version.py @@ -48,4 +48,4 @@ API_FEATURE_LEVEL = 137 # historical commits sharing the same major version, in which case a # minor version bump suffices. -PROVISION_VERSION = (197, 1) +PROVISION_VERSION = (198, 0) diff --git a/zerver/actions/create_realm.py b/zerver/actions/create_realm.py index e7c5bb37f8..cdb6a48ba3 100644 --- a/zerver/actions/create_realm.py +++ b/zerver/actions/create_realm.py @@ -142,6 +142,7 @@ def do_create_realm( org_type: Optional[int] = None, date_created: Optional[datetime.datetime] = None, is_demo_organization: bool = False, + enable_read_receipts: Optional[bool] = None, enable_spectator_access: Optional[bool] = None, ) -> Realm: if string_id == settings.SOCIAL_AUTH_SUBDOMAIN: @@ -178,6 +179,19 @@ def do_create_realm( assert not settings.PRODUCTION kwargs["date_created"] = date_created + # Generally, closed organizations like companies want read + # receipts, whereas it's unclear what an open organization's + # preferences will be. We enable the setting by default only for + # closed organizations. + if enable_read_receipts is not None: + kwargs["enable_read_receipts"] = enable_read_receipts + else: + # Hacky: The default of invited_required is True, so we need + # to check for None too. + kwargs["enable_read_receipts"] = ( + invite_required is None or invite_required is True or emails_restricted_to_domains + ) + with transaction.atomic(): realm = Realm(string_id=string_id, name=name, **kwargs) if is_demo_organization: diff --git a/zerver/models.py b/zerver/models.py index f29d2b7f50..317ee80127 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -3246,6 +3246,10 @@ class AbstractUserMessage(models.Model): def where_unread() -> str: return AbstractUserMessage.where_flag_is_absent(getattr(AbstractUserMessage.flags, "read")) + @staticmethod + def where_read() -> str: + return AbstractUserMessage.where_flag_is_present(getattr(AbstractUserMessage.flags, "read")) + @staticmethod def where_starred() -> str: return AbstractUserMessage.where_flag_is_present( diff --git a/zerver/openapi/python_examples.py b/zerver/openapi/python_examples.py index cfe22bd412..617b6e10e0 100644 --- a/zerver/openapi/python_examples.py +++ b/zerver/openapi/python_examples.py @@ -1015,6 +1015,15 @@ def remove_reaction(client: Client, message_id: int) -> None: validate_against_openapi_schema(result, "/messages/{message_id}/reactions", "delete", "200") +@openapi_test_function("/messages/{message_id}/read_receipts:get") +def get_read_receipts(client: Client, message_id: int) -> None: + # {code_example|start} + # Get read receipts for a message + result = client.call_endpoint(f"/messages/{message_id}/read_receipts", method="GET") + # {code_example|end} + validate_against_openapi_schema(result, "/messages/{message_id}/read_receipts", "get", "200") + + def test_nonexistent_stream_error(client: Client) -> None: request = { "type": "stream", @@ -1498,6 +1507,7 @@ def test_messages(client: Client, nonadmin_client: Client) -> None: get_messages(client) check_messages_match_narrow(client) get_message_history(client, message_id) + get_read_receipts(client, message_id) delete_message(client, message_id) mark_all_as_read(client) mark_stream_as_read(client) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 54b21449e0..3c45f740bb 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -5811,6 +5811,70 @@ paths: } description: | An example JSON error response for when the emoji code is invalid: + + /messages/{message_id}/read_receipts: + get: + operationId: get-read-receipts + summary: Get the list of IDs of users who have read a message. + tags: ["messages"] + description: | + Returns a list containing the IDs for all users who have + marked the message as read (and whose privacy settings allow + sharing that information). + + The list of users IDs will include any bots who have marked + the message as read via the API (providing a way for bots to + indicate whether they have processed a message successfully in + a way that can be easily inspected in a Zulip client). Bots + for which this behavior is not desired may disable the + `send_read_receipts` setting via the API. + + It will never contain the message's sender. + + **Changes**: New in Zulip 6.0 (feature level 137). + parameters: + - $ref: "#/components/parameters/MessageId" + responses: + "200": + description: Success. + content: + application/json: + schema: + allOf: + - $ref: "#/components/schemas/JsonSuccessBase" + - $ref: "#/components/schemas/SuccessDescription" + - additionalProperties: false + properties: + result: {} + msg: {} + user_ids: + type: array + description: | + An array of IDs of users who have marked the target message as + read and whose read status is available to the current user. + + The IDs of users who have disabled sending read receipts + (`send_read_receipts=false`) will never appear in the response, + nor will the message's sender. The current user's ID will, if + they have marked the read target message as read. + items: + type: integer + example: + {"msg": "", "result": "success", "user_ids": [3, 7, 9]} + "400": + description: Bad request. + content: + application/json: + schema: + allOf: + - $ref: "#/components/schemas/JsonError" + - example: {"msg": "Invalid message(s)", "result": "error"} + description: | + A typical JSON response when attempting to + access read receipts for a message ID that + either does not exist or is not accessible to + the current user. + /messages/matches_narrow: get: operationId: check-messages-match-narrow diff --git a/zerver/tests/test_read_receipts.py b/zerver/tests/test_read_receipts.py new file mode 100644 index 0000000000..9761e0274c --- /dev/null +++ b/zerver/tests/test_read_receipts.py @@ -0,0 +1,208 @@ +import orjson + +from zerver.actions.create_user import do_reactivate_user +from zerver.actions.realm_settings import do_set_realm_property +from zerver.actions.user_settings import do_change_user_setting +from zerver.actions.users import do_deactivate_user +from zerver.lib.test_classes import ZulipTestCase +from zerver.models import UserMessage, UserProfile + + +class TestReadReceipts(ZulipTestCase): + def mark_message_read(self, user: UserProfile, message_id: int) -> None: + result = self.api_post( + user, + "/json/messages/flags", + {"messages": orjson.dumps([message_id]).decode(), "op": "add", "flag": "read"}, + ) + self.assert_json_success(result) + + def test_stream_message(self) -> None: + hamlet = self.example_user("hamlet") + sender = self.example_user("othello") + + message_id = self.send_stream_message(sender, "Verona", "read receipts") + self.login("hamlet") + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id not in result.json()["user_ids"]) + + self.mark_message_read(hamlet, message_id) + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id in result.json()["user_ids"]) + self.assertTrue(sender.id not in result.json()["user_ids"]) + + def test_personal_message(self) -> None: + hamlet = self.example_user("hamlet") + sender = self.example_user("othello") + + message_id = self.send_personal_message(sender, hamlet) + self.login("hamlet") + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id not in result.json()["user_ids"]) + + self.mark_message_read(hamlet, message_id) + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id in result.json()["user_ids"]) + self.assertTrue(sender.id not in result.json()["user_ids"]) + + def test_huddle_message(self) -> None: + hamlet = self.example_user("hamlet") + sender = self.example_user("othello") + cordelia = self.example_user("cordelia") + + message_id = self.send_huddle_message(sender, [hamlet, cordelia]) + self.login("hamlet") + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id not in result.json()["user_ids"]) + self.assertTrue(sender.id not in result.json()["user_ids"]) + + self.mark_message_read(hamlet, message_id) + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id in result.json()["user_ids"]) + self.assertTrue(sender.id not in result.json()["user_ids"]) + + def test_inaccessible_stream_message(self) -> None: + sender = self.example_user("othello") + + private_stream = "private stream" + self.make_stream(stream_name=private_stream, invite_only=True) + self.subscribe(sender, private_stream) + + message_id = self.send_stream_message(sender, private_stream, "read receipts") + + self.login("hamlet") + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_error(result, "Invalid message(s)") + + self.login_user(sender) + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + + def test_filter_deactivated_users(self) -> None: + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + sender = self.example_user("othello") + + message_id = self.send_stream_message(sender, "Verona", "read receipts") + + # Mark message as read for hamlet and cordelia. + self.mark_message_read(hamlet, message_id) + self.mark_message_read(cordelia, message_id) + + # Login as cordelia and make sure hamlet is in read receipts before deactivation. + self.login("cordelia") + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id in result.json()["user_ids"]) + self.assertTrue(cordelia.id in result.json()["user_ids"]) + + # Deactivate hamlet and verify hamlet is not in read receipts. + do_deactivate_user(hamlet, acting_user=None) + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id not in result.json()["user_ids"]) + self.assertTrue(cordelia.id in result.json()["user_ids"]) + + # Reactivate hamlet and verify hamlet appears again in read recipts. + do_reactivate_user(hamlet, acting_user=None) + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertTrue(hamlet.id in result.json()["user_ids"]) + self.assertTrue(cordelia.id in result.json()["user_ids"]) + + def test_send_read_receipts_privacy_setting(self) -> None: + hamlet = self.example_user("hamlet") + sender = self.example_user("othello") + cordelia = self.example_user("cordelia") + + message_id = self.send_stream_message(sender, "Verona", "read receipts") + + self.mark_message_read(hamlet, message_id) + self.mark_message_read(cordelia, message_id) + + self.login("aaron") + do_set_realm_property(sender.realm, "enable_read_receipts", False, acting_user=None) + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_error(result, "Read receipts are disabled in this organization.") + + do_set_realm_property(sender.realm, "enable_read_receipts", True, acting_user=None) + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertIn(hamlet.id, result.json()["user_ids"]) + self.assertIn(cordelia.id, result.json()["user_ids"]) + + # Disable read receipts setting; confirm Cordelia no longer appears. + do_change_user_setting(cordelia, "send_read_receipts", False, acting_user=cordelia) + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertIn(hamlet.id, result.json()["user_ids"]) + self.assertNotIn(cordelia.id, result.json()["user_ids"]) + + def test_send_read_receipts_privacy_setting_bot(self) -> None: + hamlet = self.example_user("hamlet") + sender = self.example_user("othello") + bot = self.example_user("default_bot") + + message_id = self.send_stream_message(sender, "Verona", "read receipts") + + self.mark_message_read(hamlet, message_id) + self.mark_message_read(bot, message_id) + + self.login("aaron") + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertIn(hamlet.id, result.json()["user_ids"]) + self.assertIn(bot.id, result.json()["user_ids"]) + + # Disable read receipts setting; confirm bot no longer appears. + do_change_user_setting(bot, "send_read_receipts", False, acting_user=bot) + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertIn(hamlet.id, result.json()["user_ids"]) + self.assertNotIn(bot.id, result.json()["user_ids"]) + + def test_historical_usermessages_read_flag_not_considered(self) -> None: + """ + Ensure UserMessage rows with historical flag are also + considered for read receipts. + """ + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + + stream_name = "test stream" + self.subscribe(cordelia, stream_name) + + message_id = self.send_stream_message(cordelia, stream_name, content="foo") + + self.login("hamlet") + + # Have hamlet react to the message to + # create a historical UserMessage row. + reaction_info = { + "emoji_name": "smile", + } + result = self.client_post(f"/json/messages/{message_id}/reactions", reaction_info) + self.assert_json_success(result) + + # Ensure UserMessage row with historical and read flags exists + user_message = UserMessage.objects.get(user_profile=hamlet, message_id=message_id) + self.assertTrue(user_message.flags.historical) + self.assertTrue(user_message.flags.read) + + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + self.assert_json_success(result) + self.assertIn(hamlet.id, result.json()["user_ids"]) diff --git a/zerver/views/read_receipts.py b/zerver/views/read_receipts.py new file mode 100644 index 0000000000..606cd432b1 --- /dev/null +++ b/zerver/views/read_receipts.py @@ -0,0 +1,75 @@ +from django.http.request import HttpRequest +from django.http.response import HttpResponse +from django.utils.translation import gettext as _ + +from zerver.lib.exceptions import JsonableError +from zerver.lib.message import access_message +from zerver.lib.request import REQ, has_request_variables +from zerver.lib.response import json_success +from zerver.lib.validator import to_non_negative_int +from zerver.models import UserMessage, UserProfile + + +@has_request_variables +def read_receipts( + request: HttpRequest, + user_profile: UserProfile, + message_id: int = REQ(converter=to_non_negative_int, path_only=True), +) -> HttpResponse: + message = access_message(user_profile, message_id)[0] + + if not user_profile.realm.enable_read_receipts: + raise JsonableError(_("Read receipts are disabled in this organization.")) + + # This query implements a few decisions: + # * Most importantly, this is where we enforce the + # send_read_receipts privacy setting. + # + # * The message sender is never included, since presumably they + # read the message before sending it, and showing the sender as + # having read their own message is likely to be confusing. + # + # * Deactivated users are excluded. While in theory someone + # could be interested in the information, not including them + # is a cleaner policy, and usually read receipts are only of + # interest shortly after a message was sent. + # + # * We do not filter on the historical flag. This means that a user + # who stars a public stream message that they did not originally + # receive will appear in read receipts for that message. + # + # * Marking a message as unread causes it to appear as not read + # via read receipts, as well. This is consistent with the fact + # that users can view a message without leaving it marked as + # read in other ways (desktop/email/push notifications). + # + # * Bots are included. Most bots never mark any messages as read, + # but one could imagine having them to do so via the API to + # communicate useful information. For example, the `read` flag + # could be used by a bot to track which messages have been + # bridged to another chat system or otherwise processed + # successfully by the bot, and users might find it useful to be + # able to inspect that in the UI. If this behavior is not + # desired for a bot, it can be disabled using the + # send_read_receipts privacy setting. + # + # Note that we do not attempt to present how many users received a + # message but have NOT marked the message as read. There are + # tricky corner cases involved in doing so, such as the + # `historical` flag for public stream messages; but the most + # important one is how to handle users who read a message and then + # later unsubscribed from a stream. + user_ids = ( + UserMessage.objects.filter( + message_id=message.id, + user_profile__is_active=True, + user_profile__send_read_receipts=True, + ) + .exclude(user_profile_id=message.sender_id) + .extra( + where=[UserMessage.where_read()], + ) + .values_list("user_profile_id", flat=True) + ) + + return json_success(request, {"user_ids": list(user_ids)}) diff --git a/zilencer/management/commands/populate_db.py b/zilencer/management/commands/populate_db.py index 35f8414292..2b0f3134c2 100644 --- a/zilencer/management/commands/populate_db.py +++ b/zilencer/management/commands/populate_db.py @@ -322,6 +322,7 @@ class Command(BaseCommand): invite_required=False, plan_type=Realm.PLAN_TYPE_SELF_HOSTED, org_type=Realm.ORG_TYPES["business"]["id"], + enable_read_receipts=True, enable_spectator_access=True, ) RealmDomain.objects.create(realm=zulip_realm, domain="zulip.com") diff --git a/zproject/urls.py b/zproject/urls.py index 34cf3a5ef8..9959eca7cb 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -98,6 +98,7 @@ from zerver.views.push_notifications import ( remove_apns_device_token, ) from zerver.views.reactions import add_reaction, remove_reaction +from zerver.views.read_receipts import read_receipts from zerver.views.realm import ( check_subdomain_available, deactivate_realm, @@ -344,6 +345,8 @@ v1_api_and_json_patterns = [ # POST adds a reaction to a message # DELETE removes a reaction from a message rest_path("messages//reactions", POST=add_reaction, DELETE=remove_reaction), + # read_receipts -> zerver.views.read_receipts + rest_path("messages//read_receipts", GET=read_receipts), # attachments -> zerver.views.attachments rest_path("attachments", GET=list_by_user), rest_path("attachments/", DELETE=remove),