mengw15 commented on code in PR #6527:
URL: https://github.com/apache/texera/pull/6527#discussion_r3609389889
##########
frontend/src/app/dashboard/component/admin/settings/admin-settings.component.spec.ts:
##########
@@ -49,4 +81,179 @@ describe("AdminSettingsComponent", () => {
expect(el.textContent?.trim()).toBe("MiB");
});
});
+
+ describe("branding", () => {
+ it("saveLogos persists only the branding assets that are set and notifies
success", () => {
+ component.logoData = "data:image/png;base64,LOGO";
+ component.miniLogoData = "data:image/png;base64,MINI";
+ component.faviconData = null;
+
+ component.saveLogos();
+
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("logo",
"data:image/png;base64,LOGO");
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("mini_logo",
"data:image/png;base64,MINI");
+ expect(adminSettings.updateSetting).not.toHaveBeenCalledWith("favicon",
expect.anything());
+ expect(message.success).toHaveBeenCalledWith("Branding saved
successfully.");
+ });
+
+ it("saveLogos does nothing when no branding asset is set", () => {
+ component.logoData = null;
+ component.miniLogoData = null;
+ component.faviconData = null;
+
+ component.saveLogos();
+
+ expect(adminSettings.updateSetting).not.toHaveBeenCalled();
+ expect(message.success).not.toHaveBeenCalled();
+ });
+
+ it("saveLogos notifies an error when the request fails", () => {
+ component.logoData = "data:image/png;base64,LOGO";
+ adminSettings.updateSetting.mockReturnValue(throwError(() => new
Error("nope")));
+
+ component.saveLogos();
+
+ expect(message.error).toHaveBeenCalledWith("Failed to save branding.");
+ });
+
+ it("resetBranding resets all three branding settings", () => {
+ component.resetBranding();
+
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("logo");
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("mini_logo");
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("favicon");
+ expect(message.info).toHaveBeenCalledWith("Resetting branding...");
+ });
+ });
+
+ describe("tabs", () => {
+ it("saveTabs persists every sidebar tab and notifies success", () => {
+ const tabCount = Object.keys(component.sidebarTabs).length;
+
+ component.saveTabs();
+
+ expect(adminSettings.updateSetting).toHaveBeenCalledTimes(tabCount);
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("hub_enabled",
"false");
+ expect(message.success).toHaveBeenCalledWith("Tabs saved successfully.");
+ });
+
+ it("resetTabs resets every sidebar tab", () => {
+ const tabCount = Object.keys(component.sidebarTabs).length;
+
+ component.resetTabs();
+
+ expect(adminSettings.resetSetting).toHaveBeenCalledTimes(tabCount);
+ expect(message.info).toHaveBeenCalledWith("Resetting tabs...");
+ });
+ });
Review Comment:
Added — `saveTabs` now has a test that forces `updateSetting` to error and
asserts `message.error("Failed to save tabs.")`.
##########
frontend/src/app/dashboard/component/admin/settings/admin-settings.component.spec.ts:
##########
@@ -49,4 +81,179 @@ describe("AdminSettingsComponent", () => {
expect(el.textContent?.trim()).toBe("MiB");
});
});
+
+ describe("branding", () => {
+ it("saveLogos persists only the branding assets that are set and notifies
success", () => {
+ component.logoData = "data:image/png;base64,LOGO";
+ component.miniLogoData = "data:image/png;base64,MINI";
+ component.faviconData = null;
+
+ component.saveLogos();
+
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("logo",
"data:image/png;base64,LOGO");
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("mini_logo",
"data:image/png;base64,MINI");
+ expect(adminSettings.updateSetting).not.toHaveBeenCalledWith("favicon",
expect.anything());
+ expect(message.success).toHaveBeenCalledWith("Branding saved
successfully.");
+ });
+
+ it("saveLogos does nothing when no branding asset is set", () => {
+ component.logoData = null;
+ component.miniLogoData = null;
+ component.faviconData = null;
+
+ component.saveLogos();
+
+ expect(adminSettings.updateSetting).not.toHaveBeenCalled();
+ expect(message.success).not.toHaveBeenCalled();
+ });
+
+ it("saveLogos notifies an error when the request fails", () => {
+ component.logoData = "data:image/png;base64,LOGO";
+ adminSettings.updateSetting.mockReturnValue(throwError(() => new
Error("nope")));
+
+ component.saveLogos();
+
+ expect(message.error).toHaveBeenCalledWith("Failed to save branding.");
+ });
+
+ it("resetBranding resets all three branding settings", () => {
+ component.resetBranding();
+
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("logo");
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("mini_logo");
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("favicon");
+ expect(message.info).toHaveBeenCalledWith("Resetting branding...");
+ });
+ });
+
+ describe("tabs", () => {
+ it("saveTabs persists every sidebar tab and notifies success", () => {
+ const tabCount = Object.keys(component.sidebarTabs).length;
+
+ component.saveTabs();
+
+ expect(adminSettings.updateSetting).toHaveBeenCalledTimes(tabCount);
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("hub_enabled",
"false");
+ expect(message.success).toHaveBeenCalledWith("Tabs saved successfully.");
+ });
+
+ it("resetTabs resets every sidebar tab", () => {
+ const tabCount = Object.keys(component.sidebarTabs).length;
+
+ component.resetTabs();
+
+ expect(adminSettings.resetSetting).toHaveBeenCalledTimes(tabCount);
+ expect(message.info).toHaveBeenCalledWith("Resetting tabs...");
+ });
+ });
+
+ describe("dataset settings", () => {
+ beforeEach(() => {
+ component.maxConcurrentFiles = 3;
+ component.maxFileSizeMiB = 20;
+ component.maxConcurrentChunks = 10;
+ component.chunkSizeMiB = 50;
+ });
+
+ it("saveDatasetSettings persists the four upload settings and notifies
success", () => {
+ component.saveDatasetSettings();
+
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("max_number_of_concurrent_uploading_file",
"3");
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("single_file_upload_max_size_mib",
"20");
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("max_number_of_concurrent_uploading_file_chunks",
"10");
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("multipart_upload_chunk_size_mib",
"50");
+ expect(message.success).toHaveBeenCalledWith("Dataset upload settings
saved successfully.");
+ });
+
+ it("saveDatasetSettings rejects non-positive values without saving", () =>
{
+ component.maxFileSizeMiB = 0;
+
+ component.saveDatasetSettings();
+
+ expect(adminSettings.updateSetting).not.toHaveBeenCalled();
+ expect(message.error).toHaveBeenCalledWith("Please enter valid integer
values.");
+ });
+
Review Comment:
Added — with valid inputs, a test forces `updateSetting` to error and
asserts `message.error("Failed to save dataset settings.")`.
##########
frontend/src/app/dashboard/component/admin/settings/admin-settings.component.spec.ts:
##########
@@ -49,4 +81,179 @@ describe("AdminSettingsComponent", () => {
expect(el.textContent?.trim()).toBe("MiB");
});
});
+
+ describe("branding", () => {
+ it("saveLogos persists only the branding assets that are set and notifies
success", () => {
+ component.logoData = "data:image/png;base64,LOGO";
+ component.miniLogoData = "data:image/png;base64,MINI";
+ component.faviconData = null;
+
+ component.saveLogos();
+
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("logo",
"data:image/png;base64,LOGO");
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("mini_logo",
"data:image/png;base64,MINI");
+ expect(adminSettings.updateSetting).not.toHaveBeenCalledWith("favicon",
expect.anything());
+ expect(message.success).toHaveBeenCalledWith("Branding saved
successfully.");
+ });
+
+ it("saveLogos does nothing when no branding asset is set", () => {
+ component.logoData = null;
+ component.miniLogoData = null;
+ component.faviconData = null;
+
+ component.saveLogos();
+
+ expect(adminSettings.updateSetting).not.toHaveBeenCalled();
+ expect(message.success).not.toHaveBeenCalled();
+ });
+
+ it("saveLogos notifies an error when the request fails", () => {
+ component.logoData = "data:image/png;base64,LOGO";
+ adminSettings.updateSetting.mockReturnValue(throwError(() => new
Error("nope")));
+
+ component.saveLogos();
+
+ expect(message.error).toHaveBeenCalledWith("Failed to save branding.");
+ });
+
+ it("resetBranding resets all three branding settings", () => {
+ component.resetBranding();
+
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("logo");
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("mini_logo");
+ expect(adminSettings.resetSetting).toHaveBeenCalledWith("favicon");
+ expect(message.info).toHaveBeenCalledWith("Resetting branding...");
+ });
+ });
+
+ describe("tabs", () => {
+ it("saveTabs persists every sidebar tab and notifies success", () => {
+ const tabCount = Object.keys(component.sidebarTabs).length;
+
+ component.saveTabs();
+
+ expect(adminSettings.updateSetting).toHaveBeenCalledTimes(tabCount);
+ expect(adminSettings.updateSetting).toHaveBeenCalledWith("hub_enabled",
"false");
+ expect(message.success).toHaveBeenCalledWith("Tabs saved successfully.");
+ });
+
+ it("resetTabs resets every sidebar tab", () => {
+ const tabCount = Object.keys(component.sidebarTabs).length;
+
+ component.resetTabs();
+
+ expect(adminSettings.resetSetting).toHaveBeenCalledTimes(tabCount);
+ expect(message.info).toHaveBeenCalledWith("Resetting tabs...");
+ });
+ });
+
+ describe("dataset settings", () => {
+ beforeEach(() => {
+ component.maxConcurrentFiles = 3;
+ component.maxFileSizeMiB = 20;
+ component.maxConcurrentChunks = 10;
+ component.chunkSizeMiB = 50;
+ });
+
+ it("saveDatasetSettings persists the four upload settings and notifies
success", () => {
+ component.saveDatasetSettings();
+
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("max_number_of_concurrent_uploading_file",
"3");
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("single_file_upload_max_size_mib",
"20");
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("max_number_of_concurrent_uploading_file_chunks",
"10");
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("multipart_upload_chunk_size_mib",
"50");
+ expect(message.success).toHaveBeenCalledWith("Dataset upload settings
saved successfully.");
+ });
+
+ it("saveDatasetSettings rejects non-positive values without saving", () =>
{
+ component.maxFileSizeMiB = 0;
+
+ component.saveDatasetSettings();
+
+ expect(adminSettings.updateSetting).not.toHaveBeenCalled();
+ expect(message.error).toHaveBeenCalledWith("Please enter valid integer
values.");
+ });
+
+ it("saveDatasetSettings rejects a configuration that would exceed the
10,000-part limit", () => {
+ component.maxFileSizeMiB = 100000;
+ component.chunkSizeMiB = 1;
+
+ component.saveDatasetSettings();
+
+ expect(adminSettings.updateSetting).not.toHaveBeenCalled();
+ expect(message.error).toHaveBeenCalled();
+ });
+
+ it("resetDatasetSettings resets all four upload settings", () => {
+ component.resetDatasetSettings();
+
+ expect(adminSettings.resetSetting).toHaveBeenCalledTimes(4);
+
expect(adminSettings.resetSetting).toHaveBeenCalledWith("multipart_upload_chunk_size_mib");
+ expect(message.info).toHaveBeenCalledWith("Resetting dataset
settings...");
+ });
+ });
+
+ describe("csv (result panel) settings", () => {
+ it("saveCsvSettings persists the max-columns value and notifies success",
() => {
+ component.csvMaxColumns = 256;
+
+ component.saveCsvSettings();
+
+
expect(adminSettings.updateSetting).toHaveBeenCalledWith("csv_parser_max_columns",
"256");
+ expect(notification.success).toHaveBeenCalledWith("Result panel settings
saved.");
+ });
+
+ it("saveCsvSettings notifies an error when the request fails", () => {
+ adminSettings.updateSetting.mockReturnValue(throwError(() => new
Error("nope")));
+
+ component.saveCsvSettings();
+
+ expect(notification.error).toHaveBeenCalledWith("Could not save result
panel settings.");
+ });
+
+ it("resetCsvSettings resets the max-columns setting", () => {
+ component.resetCsvSettings();
+
+
expect(adminSettings.resetSetting).toHaveBeenCalledWith("csv_parser_max_columns");
+ expect(notification.info).toHaveBeenCalledWith("Resetting result panel
settings...");
+ });
+ });
+
+ describe("onFileChange", () => {
+ it("rejects a non-image file with an error message", () => {
+ const event = {
+ target: { files: [new File(["x"], "notes.txt", { type: "text/plain"
})] },
+ } as unknown as Event;
+
+ component.onFileChange("logo", event);
+
+ expect(message.error).toHaveBeenCalledWith("Please upload a valid image
file.");
+ expect(component.logoData).toBeNull();
+ });
Review Comment:
Good catch — the test now seeds `logoData` with a sentinel first and asserts
the invalid file leaves it untouched (fails if the guard is removed), instead
of the tautological null check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]