Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: enable message receipts by default; closes #132 #194

Merged
merged 1 commit into from
Mar 5, 2024
Merged

fix: enable message receipts by default; closes #132 #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export const CHAT_CONFIGURATIONS = {
export const PARTICIPANT_TOKEN_HEADER = "x-amzn-connect-participant-token";
export const AUTH_HEADER = "X-Amz-Bearer";

export const DEFAULT_MESSAGE_RECEIPTS_THROTTLE_MS = 5000;

export const FEATURES = {
MESSAGE_RECEIPTS_ENABLED: "MESSAGE_RECEIPTS_ENABLED"
};
Expand Down
8 changes: 4 additions & 4 deletions src/core/chatController.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ describe("ChatController", () => {
let endResponse;

function getChatController(shouldSendMessageReceipts = true) {
GlobalConfig.update({
features: shouldSendMessageReceipts ? [FEATURES.MESSAGE_RECEIPTS_ENABLED] : [],
throttleTime: 1000
});
if (!shouldSendMessageReceipts) {
GlobalConfig.removeFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
}
GlobalConfig.updateThrottleTime(1000);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does this 1000ms throttle time come from?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was just copy/pasted from the internal code. No test coverage so it looks like the behavior won't change it either way

The README and code will default to 5000 [ref]


return new ChatController({
sessionType: SESSION_TYPES.AGENT,
Expand Down
14 changes: 9 additions & 5 deletions src/core/chatSession.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,15 @@ var setGlobalConfig = config => {
if (csmConfig) {
csmService.updateCsmConfig(csmConfig);
}
//Message Receipts enabled by default
if (!(config.features?.messageReceipts?.shouldSendMessageReceipts === false)) {
logger.warn("enabling message-receipts by default; to disable set config.features.messageReceipts.shouldSendMessageReceipts = false");
setFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
GlobalConfig.updateThrottleTime(config.features?.messageReceipts?.throttleTime);
/**
* Handle setting message receipts feature in Global Config. If no values are given will default to:
* - Message receipts enabled
* - Throttle = 5000 ms
*/
logger.warn("enabling message-receipts by default; to disable set config.features.messageReceipts.shouldSendMessageReceipts = false");
GlobalConfig.updateThrottleTime(config.features?.messageReceipts?.throttleTime);
if (config.features?.messageReceipts?.shouldSendMessageReceipts === false) {
GlobalConfig.removeFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
}
};

Expand Down
16 changes: 14 additions & 2 deletions src/globalConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FEATURES, DEFAULT_MESSAGE_RECEIPTS_THROTTLE_MS } from "./constants";
import { LogManager } from "./log";

class GlobalConfigImpl {
Expand Down Expand Up @@ -32,6 +33,8 @@ class GlobalConfigImpl {
return true;
}
});
this.setFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED); // message receipts enabled by default
this.messageReceiptThrottleTime = DEFAULT_MESSAGE_RECEIPTS_THROTTLE_MS;
this.featureChangeListeners = [];
}
update(configInput) {
Expand All @@ -42,7 +45,8 @@ class GlobalConfigImpl {
this.endpointOverride = config.endpoint || this.endpointOverride;
this.reconnect = config.reconnect === false ? false : this.reconnect;
this.messageReceiptThrottleTime = config.throttleTime ? config.throttleTime : 5000;
this.features["values"] = Array.isArray(config.features) ? [...config.features] : new Array();
const features = config.features || this.features.values;
this.features["values"] = Array.isArray(features) ? [...features] : new Array();
}

updateStageRegionCell(config) {
Expand All @@ -58,7 +62,7 @@ class GlobalConfigImpl {
}

updateThrottleTime(throttleTime) {
this.messageReceiptThrottleTime = throttleTime ? throttleTime : this.messageReceiptThrottleTime;
this.messageReceiptThrottleTime = throttleTime || this.messageReceiptThrottleTime;
}

getMessageReceiptsThrottleTime() {
Expand All @@ -77,6 +81,14 @@ class GlobalConfigImpl {
return this.endpointOverride;
}

removeFeatureFlag(feature) {
if (!this.isFeatureEnabled(feature)) {
return;
}
const index = this.features["values"].indexOf(feature);
this.features["values"].splice(index, 1);
}

setFeatureFlag(feature) {
if(this.isFeatureEnabled(feature)) {
return;
Expand Down
74 changes: 40 additions & 34 deletions src/globalConfig.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe("globalConfig", () => {
expect(GlobalConfig.getRegion()).toEqual(configInput.region);
expect(GlobalConfig.getCell()).toEqual(configInput.cell);
expect(GlobalConfig.getEndpointOverride()).toEqual(configInput.endpoint);
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED)).toEqual(false);
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED)).toEqual(true);
});
it("should update stage, region and cell and fetch correct config", () => {
GlobalConfig.updateStageRegionCell(stageRegionCell);
Expand Down Expand Up @@ -136,14 +136,14 @@ describe("globalConfig", () => {
setConfig(LogLevel.ERROR);
var logger = LogManager.getLogger({ prefix: "prefix " });
logger.error("error", 3);
expect(messages[0]).toEqual(["ERROR [2022-04-12T23:12:36.677Z] prefix : error 3 "]);
expect(messages[1]).toEqual(["ERROR [2022-04-12T23:12:36.677Z] prefix : error 3 "]);
});
it("should match log format in advanced_log level", () => {
console.error = mockFn;
setConfig(LogLevel.ADVANCED_LOG);
var logger = LogManager.getLogger({ prefix: "prefix " });
logger.advancedLog("info", 3);
expect(messages[0]).toEqual(["ADVANCED_LOG [2022-04-12T23:12:36.677Z] prefix : info 3 "]);
expect(messages[1]).toEqual(["ADVANCED_LOG [2022-04-12T23:12:36.677Z] prefix : info 3 "]);
});
test("set region", () => {
ChatSessionObject.setGlobalConfig({
Expand All @@ -156,21 +156,21 @@ describe("globalConfig", () => {
setConfig(LogLevel.INFO);
var logger = LogManager.getLogger({ prefix: "prefix ", logMetaData });
logger.info("info", 3);
expect(messages[1]).toEqual(["INFO [2022-04-12T23:12:36.677Z] prefix : info 3 {\"contactId\":\"abc\"}"]);
expect(messages[2]).toEqual(["INFO [2022-04-12T23:12:36.677Z] prefix : info 3 {\"contactId\":\"abc\"}"]);
});
it("should match log format when there is no prefix and logMetaData", () => {
console.info = mockFn;
setConfig(LogLevel.INFO);
var logger = LogManager.getLogger({ logMetaData: {contactId: "abc"}});
logger.info("info", 3);
expect(messages[1]).toEqual(["INFO [2022-04-12T23:12:36.677Z] info 3 {\"contactId\":\"abc\"}"]);
expect(messages[2]).toEqual(["INFO [2022-04-12T23:12:36.677Z] info 3 {\"contactId\":\"abc\"}"]);
});
it("should match log format when there is no prefix, but logMetaData is included", () => {
console.info = mockFn;
setConfig(LogLevel.INFO);
var logger = LogManager.getLogger({ logMetaData });
logger.info("info", 3);
expect(messages[1]).toEqual(["INFO [2022-04-12T23:12:36.677Z] info 3 {\"contactId\":\"abc\"}"]);
expect(messages[2]).toEqual(["INFO [2022-04-12T23:12:36.677Z] info 3 {\"contactId\":\"abc\"}"]);
});
});

Expand All @@ -196,7 +196,7 @@ describe("globalConfig", () => {
logger.error("error", 4);
logger.error("error", 5);

expect(testLogger.warn.mock.calls[0][0]).toEqual("WARN [\"warn\",3] ");
expect(testLogger.warn.mock.calls[1][0]).toEqual("WARN [\"warn\",3] ");
expect(testLogger.error.mock.calls[0][0]).toEqual("ERROR [\"error\",4] ");
expect(testLogger.error.mock.calls[1][0]).toEqual("ERROR [\"error\",5] ");
});
Expand All @@ -214,7 +214,7 @@ describe("globalConfig", () => {
logger.error("error", 4);
logger.error("error", 5);

expect(testLogger.warn.mock.calls[0][0]).toEqual("WARN [\"warn\",3] ");
expect(testLogger.warn.mock.calls[1][0]).toEqual("WARN [\"warn\",3] ");
expect(testLogger.error.mock.calls[0][0]).toEqual("ERROR [\"error\",4] ");
expect(testLogger.error.mock.calls[1][0]).toEqual("ERROR [\"error\",5] ");
});
Expand Down Expand Up @@ -292,55 +292,61 @@ describe("globalConfig", () => {
});

describe("feature flag test", () => {
it("should update feature Flag", () => {
it("Should have message receipts feature enabled by default", () => {
GlobalConfig.update({
features: [FEATURES.MESSAGE_RECEIPTS_ENABLED]
});
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED)).toEqual(true);
});
it("should update feature Flag and call the registered listeners only once", () => {
GlobalConfig.update({
features: []
});
const handler = jest.fn();
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED, handler)).toEqual(false);
GlobalConfig.setFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
expect(handler).toHaveBeenCalled();
GlobalConfig.update({

it("Should update feature flags according to setGlobalConfig input", () => {
ChatSessionObject.setGlobalConfig({
features: {
messageReceipts: {
shouldSendMessageReceipts: false
shouldSendMessageReceipts: false,
throttleTime: 4000,
}
}
});
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED)).toEqual(false);
expect(GlobalConfig.getMessageReceiptsThrottleTime()).toEqual(4000);
});

it("Should be able to remove feature flag", () => {
GlobalConfig.removeFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED)).toEqual(false);
});

it("Should be able to add feature flag", () => {
GlobalConfig.setFeatureFlag(FEATURES.PARTICIPANT_CONN_ACK);
expect(GlobalConfig.isFeatureEnabled(FEATURES.PARTICIPANT_CONN_ACK)).toEqual(true);
});

it("Should update feature flag and call the registered listeners only once", () => {
GlobalConfig.removeFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
const handler = jest.fn();
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED, handler)).toEqual(false);
GlobalConfig.setFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
expect(handler).toHaveBeenCalled();
GlobalConfig.setFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
GlobalConfig.setFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
expect(handler).toHaveBeenCalledTimes(1);
});

it("should update feature Flag and call multiple registered listeners only once", () => {
GlobalConfig.update({
features: {
messageReceipts: {
shouldSendMessageReceipts: true
}
}
});
it("should update feature flag and call multiple registered listeners only once", () => {
const handler = jest.fn().mockReturnValue(true);
const handler2 = jest.fn();
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED, handler)).toEqual(false);
GlobalConfig.update({
features: []
});
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED, handler)).toEqual(true);
GlobalConfig.removeFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED, handler)).toEqual(false);
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED, handler2)).toEqual(false);
GlobalConfig.setFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
GlobalConfig.setFeatureFlag(FEATURES.PARTICIPANT_CONN_ACK);
expect(GlobalConfig.isFeatureEnabled(FEATURES.MESSAGE_RECEIPTS_ENABLED, handler)).toEqual(true);
expect(handler).toHaveBeenCalled();
expect(handler2).toHaveBeenCalled();
GlobalConfig.update({
features: []
});
GlobalConfig.removeFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
GlobalConfig.removeFeatureFlag(FEATURES.PARTICIPANT_CONN_ACK);
GlobalConfig.setFeatureFlag(FEATURES.MESSAGE_RECEIPTS_ENABLED);
expect(handler).toHaveBeenCalledTimes(3);
expect(handler2).toHaveBeenCalledTimes(1);
Expand Down