Skip to content

test: add comprehensive test coverage for InviteService #2874

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

Closed
gorkem-bwl wants to merge 3 commits into from
Closed

test: add comprehensive test coverage for InviteService #2874

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3fd0df5
test: add comprehensive test coverage for InviteService
gorkem-bwl Aug 29, 2025
19f4c85
style: fix prettier formatting in inviteService test
gorkem-bwl Aug 29, 2025
b7f67b5
fix: correct InviteService test to match actual EmailService behavior
gorkem-bwl Aug 29, 2025
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
221 changes: 221 additions & 0 deletions server/tests/services/inviteService.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
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

@llamapreview llamapreview bot Aug 29, 2025

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

@coderabbitai coderabbitai bot Aug 29, 2025

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

@llamapreview llamapreview bot Aug 29, 2025

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

@llamapreview llamapreview bot Aug 29, 2025

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(true);
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
);
});

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 throw error when email sending fails", async function () {
Copy link

@llamapreview llamapreview bot Aug 29, 2025

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 assumes the service throws a server error when sendEmail returns false, but this behavior isn't verified against the actual service implementation. The test hardcodes error message expectations, creating fragile tests that could break if error messages change. This approach tests the mock implementation rather than validating the service's actual error handling contract. The test should verify behavior based on the service's public API rather than implementation details of error message formatting.

Code Suggestion:

it("should throw error when email sending fails", async function () {
  mockEmailService.sendEmail.resolves(false);
  
  await expect(inviteService.sendInviteEmail({ inviteRequest, firstName }))
    .to.be.rejectedWith(Error);
  
  // Verify error service was called without hardcoding message
  expect(mockErrorService.createServerError).to.have.been.called;
});

mockEmailService.sendEmail.resolves(false);
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 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

@llamapreview llamapreview bot Aug 29, 2025

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");
});
});
});
Loading