mirror of https://github.com/zulip/zulip.git
reactions: Prevent multiple simultaneous requests for reaction update.
Prevents multiple simultaneous requests to the API when adding or removing reactions. This commit blocks emoji state changes until the request is executed. Fixes part of #21213
This commit is contained in:
parent
b9dd109e93
commit
a80f96dde6
|
@ -14,7 +14,9 @@ import * as people from "./people";
|
||||||
import * as spectators from "./spectators";
|
import * as spectators from "./spectators";
|
||||||
import {user_settings} from "./user_settings";
|
import {user_settings} from "./user_settings";
|
||||||
|
|
||||||
export const view = {}; // function namespace
|
export const view = {
|
||||||
|
waiting_for_server_request_ids: new Set(),
|
||||||
|
}; // function namespace
|
||||||
|
|
||||||
export function get_local_reaction_id(reaction_info) {
|
export function get_local_reaction_id(reaction_info) {
|
||||||
return [reaction_info.reaction_type, reaction_info.emoji_code].join(",");
|
return [reaction_info.reaction_type, reaction_info.emoji_code].join(",");
|
||||||
|
@ -78,6 +80,14 @@ function update_ui_and_send_reaction_ajax(message_id, reaction_info) {
|
||||||
const operation = has_reacted ? "remove" : "add";
|
const operation = has_reacted ? "remove" : "add";
|
||||||
const reaction = create_reaction(message_id, reaction_info);
|
const reaction = create_reaction(message_id, reaction_info);
|
||||||
|
|
||||||
|
// To avoid duplicate requests to the server, we construct a
|
||||||
|
// unique request ID combining the message ID and the local ID,
|
||||||
|
// which identifies just which emoji to use.
|
||||||
|
const reaction_request_id = [message_id, local_id].join(",");
|
||||||
|
if (view.waiting_for_server_request_ids.has(reaction_request_id)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (operation === "add") {
|
if (operation === "add") {
|
||||||
add_reaction(reaction);
|
add_reaction(reaction);
|
||||||
} else {
|
} else {
|
||||||
|
@ -87,17 +97,17 @@ function update_ui_and_send_reaction_ajax(message_id, reaction_info) {
|
||||||
const args = {
|
const args = {
|
||||||
url: "/json/messages/" + message_id + "/reactions",
|
url: "/json/messages/" + message_id + "/reactions",
|
||||||
data: reaction_info,
|
data: reaction_info,
|
||||||
success() {},
|
success() {
|
||||||
|
view.waiting_for_server_request_ids.delete(reaction_request_id);
|
||||||
|
},
|
||||||
error(xhr) {
|
error(xhr) {
|
||||||
|
view.waiting_for_server_request_ids.delete(reaction_request_id);
|
||||||
const response = channel.xhr_error_message("Error sending reaction", xhr);
|
const response = channel.xhr_error_message("Error sending reaction", xhr);
|
||||||
// Errors are somewhat common here, due to race conditions
|
blueslip.error(response);
|
||||||
// where the user tries to add/remove the reaction when there is already
|
|
||||||
// an in-flight request. We eventually want to make this a blueslip
|
|
||||||
// error, rather than a warning, but we need to implement either
|
|
||||||
// #4291 or #4295 first.
|
|
||||||
blueslip.warn(response);
|
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
|
view.waiting_for_server_request_ids.add(reaction_request_id);
|
||||||
if (operation === "add") {
|
if (operation === "add") {
|
||||||
channel.post(args);
|
channel.post(args);
|
||||||
} else if (operation === "remove") {
|
} else if (operation === "remove") {
|
||||||
|
|
|
@ -308,7 +308,7 @@ test("sending", ({override, override_rewire}) => {
|
||||||
|
|
||||||
// similarly, we only exercise the failure codepath
|
// similarly, we only exercise the failure codepath
|
||||||
// Since this path calls blueslip.warn, we need to handle it.
|
// Since this path calls blueslip.warn, we need to handle it.
|
||||||
blueslip.expect("warn", "XHR error message.");
|
blueslip.expect("error", "XHR error message.");
|
||||||
channel.xhr_error_message = () => "XHR error message.";
|
channel.xhr_error_message = () => "XHR error message.";
|
||||||
args.error();
|
args.error();
|
||||||
}
|
}
|
||||||
|
@ -369,6 +369,24 @@ test("sending", ({override, override_rewire}) => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("prevent_simultaneous_requests_updating_reaction", ({override, override_rewire}) => {
|
||||||
|
const message = {...sample_message};
|
||||||
|
override(message_store, "get", (message_id) => {
|
||||||
|
assert.equal(message_id, message.id);
|
||||||
|
return message;
|
||||||
|
});
|
||||||
|
override_rewire(reactions, "add_reaction", () => {});
|
||||||
|
const stub = make_stub();
|
||||||
|
channel.post = stub.f;
|
||||||
|
|
||||||
|
// Verify that two requests to add the same reaction in a row only
|
||||||
|
// result in a single request to the server.
|
||||||
|
reactions.toggle_emoji_reaction(message.id, "cow");
|
||||||
|
reactions.toggle_emoji_reaction(message.id, "cow");
|
||||||
|
|
||||||
|
assert.equal(stub.num_calls, 1);
|
||||||
|
});
|
||||||
|
|
||||||
function stub_reactions(message_id) {
|
function stub_reactions(message_id) {
|
||||||
const $message_row = $.create("message-row-stub");
|
const $message_row = $.create("message-row-stub");
|
||||||
const $message_reactions = $.create("reactions-stub");
|
const $message_reactions = $.create("reactions-stub");
|
||||||
|
|
Loading…
Reference in New Issue