Skip to content
Closed
Changes from all commits
Commits
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
253 changes: 253 additions & 0 deletions server/tests/services/inviteService.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
import sinon from "sinon";
import InviteService from "../../src/service/business/inviteService.js";

describe("InviteService", function () {
let inviteService;
let mockDb, mockSettingsService, mockEmailService, mockStringService, mockErrorService;

const mockInviteToken = {
_id: "invite123",
token: "abc123token",
email: "[email protected]",
teamId: "team123",
role: ["member"],
};

const mockInvite = {
email: "[email protected]",
role: ["member"],
};

beforeEach(function () {
mockDb = {
inviteModule: {
requestInviteToken: sinon.stub(),
getInviteToken: sinon.stub(),
},
};

mockSettingsService = {
getSettings: sinon.stub(),
};

mockEmailService = {
buildEmail: sinon.stub(),
sendEmail: sinon.stub(),
};

mockStringService = {
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: Medium

The mock string service only includes two strings, but the actual service might use additional strings from this dependency. This could lead to false positive tests if the service under test uses untested string values. The minimal mock doesn't validate whether all required strings are properly handled by the tests.

Code Suggestion:

// Consider using a more complete mock or a real instance if practical
const mockStringService = {
  inviteIssued: "Invite issued successfully",
  inviteVerified: "Invite verified successfully",
  // Add other potentially used strings
  emailSendingFailed: "Failed to send email"
};

inviteIssued: "Invite issued successfully",
inviteVerified: "Invite verified successfully",
};

mockErrorService = {
createServerError: sinon.stub(),
};

inviteService = new InviteService({
db: mockDb,
settingsService: mockSettingsService,
emailService: mockEmailService,
stringService: mockStringService,
errorService: mockErrorService,
});
});

afterEach(function () {
sinon.restore();
});

describe("getInviteToken", function () {
it("should generate invite token with teamId", async function () {
const teamId = "team123";
const invite = { ...mockInvite };
mockDb.inviteModule.requestInviteToken.resolves(mockInviteToken);

const result = await inviteService.getInviteToken({ invite, teamId });

expect(invite.teamId).to.equal(teamId);
expect(mockDb.inviteModule.requestInviteToken).to.have.been.calledWith(invite);
expect(result).to.equal(mockInviteToken);
});
Comment on lines +60 to +71
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid asserting on mutation of input invite

Asserting invite.teamId mutates couples the test to an implementation detail. Validate the argument passed to requestInviteToken instead.

-			expect(invite.teamId).to.equal(teamId);
-			expect(mockDb.inviteModule.requestInviteToken).to.have.been.calledWith(invite);
+			expect(mockDb.inviteModule.requestInviteToken)
+				.to.have.been.calledWithMatch(sinon.match.has("teamId", teamId));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe("getInviteToken", function () {
it("should generate invite token with teamId", async function () {
const teamId = "team123";
const invite = { ...mockInvite };
mockDb.inviteModule.requestInviteToken.resolves(mockInviteToken);
const result = await inviteService.getInviteToken({ invite, teamId });
expect(invite.teamId).to.equal(teamId);
expect(mockDb.inviteModule.requestInviteToken).to.have.been.calledWith(invite);
expect(result).to.equal(mockInviteToken);
});
describe("getInviteToken", function () {
it("should generate invite token with teamId", async function () {
const teamId = "team123";
const invite = { ...mockInvite };
mockDb.inviteModule.requestInviteToken.resolves(mockInviteToken);
const result = await inviteService.getInviteToken({ invite, teamId });
// Instead of asserting on input mutation, verify the call includes the correct teamId
expect(mockDb.inviteModule.requestInviteToken)
.to.have.been.calledWithMatch(sinon.match.has("teamId", teamId));
expect(result).to.equal(mockInviteToken);
});
});
🤖 Prompt for AI Agents
In server/tests/services/inviteService.test.js around lines 60 to 71, the test
currently asserts that invite.teamId was mutated which couples the test to an
implementation detail; remove the assertion that checks invite.teamId and
instead assert that mockDb.inviteModule.requestInviteToken was called with an
object that includes the expected teamId (use your test helper like sinon.match
or deep match/calledWithMatch to validate the argument contents), keeping the
existing assertion that the result equals mockInviteToken.


it("should handle database errors", async function () {
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: High

The test validates that database errors propagate unchanged, but doesn't verify if the service adds context or handles specific error types differently. This could miss opportunities to test error wrapping, logging, or transformation behavior that might be important for production debugging and error handling.

const error = new Error("Database error");
const teamId = "team123";
const invite = { ...mockInvite };
mockDb.inviteModule.requestInviteToken.rejects(error);

try {
await inviteService.getInviteToken({ invite, teamId });
expect.fail("Should have thrown an error");
} catch (err) {
expect(err).to.equal(error);
}
});
});

describe("sendInviteEmail", function () {
const inviteRequest = {
email: "[email protected]",
role: ["member"],
};
const firstName = "John";
const clientHost = "https://example.com";
const emailHtml = "<html>Welcome email</html>";

beforeEach(function () {
mockSettingsService.getSettings.returns({ clientHost });
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: Medium

The test assumes the settings service returns a specific structure ({ clientHost }) but doesn't validate if the actual service might return different shapes or handle missing values. This could hide bugs related to settings parsing or default value handling in the actual service implementation.

Code Suggestion:

// Test with different settings structures
it("should handle missing clientHost setting", async function () {
  mockSettingsService.getSettings.returns({});
  // Add appropriate assertions
});

mockEmailService.buildEmail.resolves(emailHtml);
mockEmailService.sendEmail.resolves("message-id-12345"); // Returns messageId on success
mockDb.inviteModule.requestInviteToken.resolves(mockInviteToken);
});

it("should send invite email successfully", async function () {
await inviteService.sendInviteEmail({ inviteRequest, firstName });

expect(mockDb.inviteModule.requestInviteToken).to.have.been.calledWith(inviteRequest);
expect(mockSettingsService.getSettings).to.have.been.called;
expect(mockEmailService.buildEmail).to.have.been.calledWith("employeeActivationTemplate", {
name: firstName,
link: `${clientHost}/register/${mockInviteToken.token}`,
});
expect(mockEmailService.sendEmail).to.have.been.calledWith(inviteRequest.email, "Welcome to Uptime Monitor", emailHtml);
});
Comment on lines +88 to +114
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation tests (missing email/role/firstName and bad email)

“Comprehensive” should include invalid inputs. Right now only upstream failures are covered.

I can draft negative-path tests for:

  • inviteRequest missing email / invalid email
  • empty roles array
  • missing firstName
  • missing clientHost in settings
🤖 Prompt for AI Agents
In server/tests/services/inviteService.test.js around lines 88 to 114, add
negative-path tests to validate input handling: create separate it() cases for
(1) inviteRequest missing email, (2) inviteRequest with invalid email format,
(3) inviteRequest with empty roles array, (4) missing firstName, and (5)
settings missing clientHost. For each test, set up mocks as appropriate, call
inviteService.sendInviteEmail and assert it rejects or throws the expected
validation error, and verify that downstream calls
(mockDb.inviteModule.requestInviteToken, mockEmailService.buildEmail,
mockEmailService.sendEmail) are not called when validation fails; for the
missing clientHost case, simulate mockSettingsService.getSettings returning {}
and assert the function errors and no email is sent.


it("should handle invite token generation failure", async function () {
const error = new Error("Token generation failed");
mockDb.inviteModule.requestInviteToken.rejects(error);

try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (err) {
expect(err).to.equal(error);
}
});

it("should handle email template building failure", async function () {
const error = new Error("Template build failed");
mockEmailService.buildEmail.rejects(error);

try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (err) {
expect(err).to.equal(error);
}
});

it("should handle buildEmail returning undefined", async function () {
mockEmailService.buildEmail.resolves(undefined); // buildEmail can return undefined on error

try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (err) {
// sendEmail will be called with undefined html, which should cause issues
expect(mockEmailService.sendEmail).to.have.been.calledWith(inviteRequest.email, "Welcome to Uptime Monitor", undefined);
}
});
Comment on lines +140 to +150
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

“buildEmail returns undefined” should not call sendEmail

Asserting sendEmail is invoked with undefined HTML encodes a bug as desired behavior. The service should detect undefined and fail early via errorService.

-			} catch (err) {
-				// sendEmail will be called with undefined html, which should cause issues
-				expect(mockEmailService.sendEmail).to.have.been.calledWith(inviteRequest.email, "Welcome to Uptime Monitor", undefined);
-			}
+			} catch (_err) {
+				expect(mockEmailService.sendEmail).not.to.have.been.called;
+				expect(mockErrorService.createServerError).to.have.been.called;
+			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("should handle buildEmail returning undefined", async function () {
mockEmailService.buildEmail.resolves(undefined); // buildEmail can return undefined on error
try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (err) {
// sendEmail will be called with undefined html, which should cause issues
expect(mockEmailService.sendEmail).to.have.been.calledWith(inviteRequest.email, "Welcome to Uptime Monitor", undefined);
}
});
it("should handle buildEmail returning undefined", async function () {
mockEmailService.buildEmail.resolves(undefined); // buildEmail can return undefined on error
try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (_err) {
expect(mockEmailService.sendEmail).not.to.have.been.called;
expect(mockErrorService.createServerError).to.have.been.called;
}
});
🤖 Prompt for AI Agents
In server/tests/services/inviteService.test.js around lines 140-150, the test
currently asserts that sendEmail is called with undefined HTML which encodes a
bug; change the test so that when mockEmailService.buildEmail resolves to
undefined the inviteService should fail early by invoking the errorService and
not calling sendEmail. Specifically, keep
mockEmailService.buildEmail.resolves(undefined), call await
inviteService.sendInviteEmail(...) inside an assertion that it rejects (or
wrapped in try/catch asserting error), assert that mockErrorService (or
errorService mock) was called with an appropriate error/detail, and assert that
mockEmailService.sendEmail was not called.


it("should throw error when email sending returns false (verification failed)", async function () {
mockEmailService.sendEmail.resolves(false); // Returns false when verification fails
const serverError = new Error("Failed to send invite e-mail... Please verify your settings.");
mockErrorService.createServerError.returns(serverError);

try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (err) {
expect(mockErrorService.createServerError).to.have.been.calledWith("Failed to send invite e-mail... Please verify your settings.");
expect(err).to.equal(serverError);
}
});

it("should throw error when email sending returns undefined (send failed)", async function () {
mockEmailService.sendEmail.resolves(undefined); // Returns undefined when send fails
const serverError = new Error("Failed to send invite e-mail... Please verify your settings.");
mockErrorService.createServerError.returns(serverError);

try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (err) {
expect(mockErrorService.createServerError).to.have.been.calledWith("Failed to send invite e-mail... Please verify your settings.");
expect(err).to.equal(serverError);
}
});

it("should handle sendEmail throwing error", async function () {
const error = new Error("SMTP connection failed");
mockEmailService.sendEmail.rejects(error);

try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (err) {
expect(err).to.equal(error);
}
});

it("should handle settings service failure", async function () {
const error = new Error("Settings service failed");
mockSettingsService.getSettings.throws(error);

try {
await inviteService.sendInviteEmail({ inviteRequest, firstName });
expect.fail("Should have thrown an error");
} catch (err) {
expect(err).to.equal(error);
}
});
});

describe("verifyInviteToken", function () {
const inviteToken = "abc123token";

it("should verify invite token successfully", async function () {
mockDb.inviteModule.getInviteToken.resolves(mockInviteToken);

const result = await inviteService.verifyInviteToken({ inviteToken });

expect(mockDb.inviteModule.getInviteToken).to.have.been.calledWith(inviteToken);
expect(result).to.equal(mockInviteToken);
});

it("should handle token not found", async function () {
const error = new Error("Invite not found");
mockDb.inviteModule.getInviteToken.rejects(error);

try {
await inviteService.verifyInviteToken({ inviteToken });
expect.fail("Should have thrown an error");
} catch (err) {
expect(err).to.equal(error);
}
});

it("should handle database errors", async function () {
const error = new Error("Database connection failed");
mockDb.inviteModule.getInviteToken.rejects(error);

try {
await inviteService.verifyInviteToken({ inviteToken });
expect.fail("Should have thrown an error");
} catch (err) {
expect(err).to.equal(error);
}
});
});

describe("serviceName getter", function () {
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: High

These tests verify constant values rather than behavior, providing minimal value while adding maintenance overhead. Testing hardcoded strings creates brittle tests that break if refactoring changes service names but not actual functionality. The tests don't validate any business logic or error paths, focusing instead on implementation details that could be verified through static typing or other mechanisms.

it("should return correct service name", function () {
expect(inviteService.serviceName).to.equal("inviteService");
});
});

describe("static SERVICE_NAME", function () {
it("should have correct static service name", function () {
expect(InviteService.SERVICE_NAME).to.equal("inviteService");
});
});
});
Comment on lines +1 to +253
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verification script to align tests with real service behavior

Let’s confirm subject text, error wrapping, template name, and not-found behavior in InviteService to de-flake expectations.


🏁 Script executed:

#!/bin/bash
# Examine the real service to align tests with its contract
set -euo pipefail

# 1) Check subject and template usage
rg -n -C2 -e 'employeeActivationTemplate' -e 'sendEmail\s*\(' -e 'inviteEmailSubject' -- 'src/**/inviteService.*'

# 2) Check error wrapping
rg -n -C2 -e 'createServerError\s*\(' -e 'throw new' -- 'src/**/inviteService.*'

# 3) Check behavior when template HTML is falsy
rg -n -C3 -e 'buildEmail' -- 'src/**/inviteService.*'

# 4) Check not-found handling in verify flow
rg -n -C3 -e 'getInviteToken\s*\(' -e 'verifyInviteToken' -- 'src/**/inviteService.*'

Length of output: 194


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Locate the InviteService implementation
rg -l "class InviteService" -g "*.js"

Length of output: 104


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Locate sendInviteEmail definition
rg -n "async sendInviteEmail" server/src/service/business/inviteService.js
# Locate verifyInviteToken definition
rg -n "async verifyInviteToken" server/src/service/business/inviteService.js

Length of output: 96


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Display beginning of InviteService implementation to locate methods and template names
sed -n '1,200p' server/src/service/business/inviteService.js

Length of output: 1475


Enforce template HTML presence in sendInviteEmail

Add a guard after calling buildEmail so that a falsy html triggers a server error (otherwise the test for undefined HTML will never throw):

 sendInviteEmail = async ({ inviteRequest, firstName }) => {
   const inviteToken = await this.db.inviteModule.requestInviteToken({ ...inviteRequest });
   const { clientHost } = this.settingsService.getSettings();

   const html = await this.emailService.buildEmail("employeeActivationTemplate", {
     name: firstName,
     link: `${clientHost}/register/${inviteToken.token}`,
   });
+  if (!html) {
+    throw this.errorService.createServerError("Failed to send invite e-mail... Please verify your settings.");
+  }

   const result = await this.emailService.sendEmail(inviteRequest.email, "Welcome to Uptime Monitor", html);
   if (!result) {
     throw this.errorService.createServerError("Failed to send invite e-mail... Please verify your settings.");
   }
 };

This aligns the code with the “buildEmail returning undefined” test. (And yes, even Canadians know you shouldn’t mail an empty poutine… 🇨🇦🇺🇸)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In server/tests/services/inviteService.test.js lines 1 to 253, the
sendInviteEmail flow needs to enforce that the HTML returned from buildEmail is
present; add a guard right after calling buildEmail so that if html is falsy you
call this.errorService.createServerError("Failed to send invite e-mail... Please
verify your settings.") (or the existing error message used for send failures)
and throw the resulting server error, ensuring the "buildEmail returning
undefined" test path throws instead of proceeding to sendEmail with undefined.