Skip to content

test(frontend): extend GmailService spec to cover all methods #5460

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

Open
mengw15 wants to merge 1 commit into
base: main
Choose a base branch
from
+67 −5
Open

test(frontend): extend GmailService spec to cover all methods #5460

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
72 changes: 67 additions & 5 deletions frontend/src/app/common/service/gmail/gmail.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe("GmailService", () => {

afterEach(() => {
httpTestingController.verify();
vi.restoreAllMocks();
});

it("should show a success toast when the backend accepts the send request", () => {
Expand All @@ -51,16 +52,77 @@ describe("GmailService", () => {
expect(notificationSpy.error).not.toHaveBeenCalled();
});

it("should show an error toast when the backend returns an HTTP error (e.g. SMTP failure)", () => {
it("sends the correct PUT body for sendEmail with an explicit receiver", () => {
service.sendEmail("subj", "body", "to@example.com");

const req = httpTestingController.expectOne(r => r.url.endsWith("/gmail/send") && r.method === "PUT");
req.flush("Failed to send email: 535-5.7.8 Username and Password not accepted", {
status: 502,
statusText: "Bad Gateway",
});
expect(req.request.body).toEqual({ receiver: "to@example.com", subject: "subj", content: "body" });
req.flush(null);
});

it("defaults the receiver to an empty string when it is omitted", () => {
service.sendEmail("subj", "body");

const req = httpTestingController.expectOne(r => r.url.endsWith("/gmail/send") && r.method === "PUT");
expect(req.request.body).toEqual({ receiver: "", subject: "subj", content: "body" });
req.flush(null);
});
Comment on lines +63 to +69

@Yicong-Huang Yicong-Huang Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this a bug or intentional? if the email receiver is empty, can an email be sent?

if it is a bug, please create an issue to follow up on fixing it. we can pin test as it is first, then in the fix, we can see it updates the test.

@mengw15 mengw15 Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not a bug — likely a vestigial default. If receiver is empty the backend sends to the requester's own address (if (receiver.isEmpty) user.getEmail), so nothing breaks. Both callers pass an explicit receiver anyway (share-access only after email validation; the admin form requires the email field), so the empty default isn't exercised in normal use. Agreed on pinning the test as-is; we can open a low-priority follow-up to clean up the unused default.


it("shows an error toast and logs to the console on a failed send", () => {
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
service.sendEmail("subj", "body", "to@example.com");

const req = httpTestingController.expectOne(r => r.url.endsWith("/gmail/send") && r.method === "PUT");
req.flush("boom", { status: 502, statusText: "Bad Gateway" });

expect(notificationSpy.error).toHaveBeenCalledWith("Failed to send email. Please try again or contact admin.");
expect(notificationSpy.success).not.toHaveBeenCalled();
expect(consoleSpy).toHaveBeenCalledWith("Send email error:", "boom");
});

it("issues a GET to the sender-email endpoint and emits the response without notifying", () => {
let emitted: string | undefined;
service.getSenderEmail().subscribe(value => (emitted = value as string));

const req = httpTestingController.expectOne(
r => r.url.endsWith("/gmail/sender/email") && r.method === "GET" && r.responseType === "text"
);
req.flush("sender@example.com");

expect(emitted).toBe("sender@example.com");
expect(notificationSpy.success).not.toHaveBeenCalled();
expect(notificationSpy.error).not.toHaveBeenCalled();
});
Comment on lines +83 to +95

@Yicong-Huang Yicong-Huang Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we notify user though?

@mengw15 mengw15 Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

getSenderEmail is only used on the admin Gmail-config page (ngOnInit) to display the configured sender address — a read-only fetch on an admin-only screen, and the component already logs failures to the console. A toast there would mostly be page-load noise, so I don't think we need to notify the user. Keeping the service a thin, non-notifying GET (unlike sendEmail, which toasts) is intentional. I'd leave it as-is.


it("sends the correct POST body for notifyUnauthorizedLogin", () => {
service.notifyUnauthorizedLogin("u@example.com", "ACME", "for research");

const req = httpTestingController.expectOne(
r => r.url.endsWith("/gmail/notify-unauthorized") && r.method === "POST"
);
expect(req.request.body).toEqual({ receiver: "u@example.com", affiliation: "ACME", reason: "for research" });
req.flush(null);
});

it("shows a success toast when the unauthorized-login notification is accepted", () => {
service.notifyUnauthorizedLogin("u@example.com", "ACME", "for research");

httpTestingController
.expectOne(r => r.url.endsWith("/gmail/notify-unauthorized") && r.method === "POST")
.flush(null);

expect(notificationSpy.success).toHaveBeenCalledWith("An admin has been notified about your account request.");
});

it("shows an error toast and logs to the console when the notification fails", () => {
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
service.notifyUnauthorizedLogin("u@example.com", "ACME", "for research");

httpTestingController
.expectOne(r => r.url.endsWith("/gmail/notify-unauthorized") && r.method === "POST")
.flush("boom", { status: 500, statusText: "Internal Server Error" });

expect(notificationSpy.error).toHaveBeenCalledWith("Failed to notify admin about your account request.");
expect(consoleSpy).toHaveBeenCalledWith("Notify error:", "boom");
});
});
Loading