diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/CustomThemeConfig.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/CustomThemeConfig.spec.ts index 9e1b9f7bb7a..8b68a70c78b 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/CustomThemeConfig.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/CustomThemeConfig.spec.ts @@ -10,7 +10,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import test, { expect } from '@playwright/test'; +import test, { expect, Request } from '@playwright/test'; import { GlobalSettingOptions } from '../../constant/settings'; import { redirectToHomePage } from '../../utils/common'; import { settingClick } from '../../utils/sidebar'; @@ -107,4 +107,43 @@ test.describe('Custom Theme Config Page', () => { 'rgb(21, 112, 239)' ); }); + + test('Should call customMonogramUrlPath only once after save if the monogram is not valid', async ({ + page, + }) => { + let monogramUrlCallCount = 0; + const monogramRequests: Request[] = []; + + // Track all network requests to the monogram URL + page.on('request', (request) => { + if (request.url().includes('custom-monogram.png')) { + monogramRequests.push(request); + monogramUrlCallCount++; + } + }); + + // Fill the monogram URL field + await page + .locator('[data-testid="customMonogramUrlPath"]') + .fill(config.monogram); + + // Fill other required fields to make form valid + await page.locator('[data-testid="customLogoUrlPath"]').fill(config.logo); + + // Reset counter before save action + monogramUrlCallCount = 0; + monogramRequests.length = 0; + + // Click save button and wait for API response + const saveResponse = page.waitForResponse('/api/v1/system/settings'); + await page.locator('[data-testid="save-btn"]').click(); + await saveResponse; + + // Wait a bit more to catch any additional requests + await page.waitForTimeout(2000); + + // Assert monogram URL was called at most once after save + expect(monogramUrlCallCount).toBeLessThanOrEqual(1); + expect(monogramRequests.length).toBeLessThanOrEqual(1); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/BrandImage/BrandImage.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/BrandImage/BrandImage.test.tsx index 5bfc51d3f88..6239abe304c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/BrandImage/BrandImage.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/BrandImage/BrandImage.test.tsx @@ -10,21 +10,38 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { render, screen } from '@testing-library/react'; -import BrandImage from './BrandImage'; +import { fireEvent, render, screen } from '@testing-library/react'; -jest.mock('../../../hooks/useApplicationStore', () => ({ - useApplicationStore: jest.fn().mockImplementation(() => ({ - applicationConfig: { - customLogoConfig: { - customLogoUrlPath: 'https://custom-logo.png', - customMonogramUrlPath: 'https://custom-monogram.png', - }, - }, - })), +jest.mock('../../../hooks/useApplicationStore'); +jest.mock('../../../utils/BrandData/BrandClassBase', () => ({ + getMonogram: jest.fn().mockReturnValue({ + src: '/default-monogram.svg', + }), + getLogo: jest.fn().mockReturnValue({ + src: '/default-logo.svg', + }), })); +import { useApplicationStore } from '../../../hooks/useApplicationStore'; +import BrandImage from './BrandImage'; + +const mockUseApplicationStore = useApplicationStore as jest.MockedFunction< + typeof useApplicationStore +>; + describe('Test Brand Logo', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockUseApplicationStore.mockReturnValue({ + applicationConfig: { + customLogoConfig: { + customLogoUrlPath: 'https://custom-logo.png', + customMonogramUrlPath: 'https://custom-monogram.png', + }, + }, + }); + }); + it('Should render the brand logo with default props value', () => { render(); @@ -34,6 +51,7 @@ describe('Test Brand Logo', () => { expect(image).toHaveAttribute('alt', 'OpenMetadata Logo'); expect(image).toHaveAttribute('height', 'auto'); expect(image).toHaveAttribute('width', '152'); + expect(image).toHaveAttribute('id', 'brand-image'); }); it('Should render the brand logo with passed props value', () => { @@ -59,18 +77,18 @@ describe('Test Brand Logo', () => { it('Should render the brand logo based on custom logo config', () => { render( ); - const image = screen.getByTestId('brand-monogram'); + const image = screen.getByTestId('brand-logo'); expect(image).toBeInTheDocument(); - expect(image).toHaveAttribute('alt', 'brand-monogram'); + expect(image).toHaveAttribute('alt', 'brand-logo'); expect(image).toHaveAttribute('height', 'auto'); expect(image).toHaveAttribute('width', '152'); expect(image).toHaveAttribute('src', 'https://custom-logo.png'); @@ -96,4 +114,247 @@ describe('Test Brand Logo', () => { expect(image).toHaveAttribute('width', '30'); expect(image).toHaveAttribute('src', 'https://custom-monogram.png'); }); + + it('Should use default logo when no custom logo config is provided', () => { + mockUseApplicationStore.mockReturnValue({ + applicationConfig: { + customLogoConfig: { + customLogoUrlPath: '', + customMonogramUrlPath: '', + }, + }, + }); + + render( + + ); + + const image = screen.getByTestId('default-logo'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', '/default-logo.svg'); + }); + + it('Should use default monogram when no custom monogram config is provided', () => { + mockUseApplicationStore.mockReturnValue({ + applicationConfig: { + customLogoConfig: { + customLogoUrlPath: '', + customMonogramUrlPath: '', + }, + }, + }); + + render( + + ); + + const image = screen.getByTestId('default-monogram'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', '/default-monogram.svg'); + }); + + it('Should handle missing customLogoConfig gracefully', () => { + mockUseApplicationStore.mockReturnValue({ + applicationConfig: {}, + }); + + render( + + ); + + const image = screen.getByTestId('fallback-logo'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', '/default-logo.svg'); + }); + + it('Should handle missing applicationConfig gracefully', () => { + mockUseApplicationStore.mockReturnValue({}); + + render( + + ); + + const image = screen.getByTestId('no-config-logo'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', '/default-logo.svg'); + }); + + it('Should use provided src prop when given', () => { + const customSrc = 'https://example.com/custom-image.png'; + + render( + + ); + + const image = screen.getByTestId('custom-src-logo'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', customSrc); + }); + + it('Should handle onError and fallback to default logo when image fails to load', () => { + render( + + ); + + const image = screen.getByTestId('error-logo'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', 'https://custom-logo.png'); + + // Simulate image load error + fireEvent.error(image); + + expect(image).toHaveAttribute('src', '/default-logo.svg'); + }); + + it('Should handle onError and fallback to default monogram when monogram fails to load', () => { + render( + + ); + + const image = screen.getByTestId('error-monogram'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', 'https://custom-monogram.png'); + + // Simulate image load error + fireEvent.error(image); + + expect(image).toHaveAttribute('src', '/default-monogram.svg'); + }); + + it('Should handle onError with custom src and fallback to default logo', () => { + const customSrc = 'https://example.com/broken-image.png'; + + render( + + ); + + const image = screen.getByTestId('broken-custom-logo'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', customSrc); + + // Simulate image load error + fireEvent.error(image); + + expect(image).toHaveAttribute('src', '/default-logo.svg'); + }); + + it('Should handle onError with custom src and monogram fallback', () => { + const customSrc = 'https://example.com/broken-monogram.png'; + + render( + + ); + + const image = screen.getByTestId('broken-custom-monogram'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', customSrc); + + // Simulate image load error + fireEvent.error(image); + + expect(image).toHaveAttribute('src', '/default-monogram.svg'); + }); + + it('Should render with all props including optional ones', () => { + render( + + ); + + const image = screen.getByTestId('comprehensive-test'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('alt', 'comprehensive test logo'); + expect(image).toHaveAttribute('width', '100%'); + expect(image).toHaveAttribute('height', '50px'); + expect(image).toHaveAttribute('src', 'https://example.com/test-logo.png'); + expect(image).toHaveAttribute('id', 'brand-image'); + expect(image).toHaveClass('test-class', 'custom-brand'); + }); + + it('Should prioritize src prop over custom logo configuration', () => { + const prioritySrc = 'https://priority.com/logo.png'; + + render( + + ); + + const image = screen.getByTestId('priority-test'); + + expect(image).toBeInTheDocument(); + expect(image).toHaveAttribute('src', prioritySrc); + expect(image).not.toHaveAttribute('src', 'https://custom-logo.png'); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/BrandImage/BrandImage.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/BrandImage/BrandImage.tsx index 1aea1a2272d..2d2b5b6d78b 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/BrandImage/BrandImage.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/BrandImage/BrandImage.tsx @@ -33,20 +33,21 @@ const BrandImage: FC = ({ src, isMonoGram = false, }) => { - const { MonoGram, Logo } = useMemo( - () => ({ - MonoGram: brandClassBase.getMonogram().src, - Logo: brandClassBase.getLogo().src, - }), - [] - ); const { applicationConfig } = useApplicationStore(); - const { customLogoUrlPath = '', customMonogramUrlPath = '' } = - applicationConfig?.customLogoConfig ?? {}; - const logoSource = isMonoGram - ? customMonogramUrlPath || MonoGram - : customLogoUrlPath || Logo; + const { defaultLogo, logoSource } = useMemo(() => { + const { customLogoUrlPath = '', customMonogramUrlPath = '' } = + applicationConfig?.customLogoConfig ?? {}; + const monoGram = brandClassBase.getMonogram().src; + const logo = brandClassBase.getLogo().src; + + const defaultLogo = isMonoGram ? monoGram : logo; + const logoSource = isMonoGram + ? customMonogramUrlPath || monoGram + : customLogoUrlPath || logo; + + return { defaultLogo, logoSource }; + }, [isMonoGram, applicationConfig?.customLogoConfig]); return ( = ({ src={src ?? logoSource} width={width} onError={(e) => { - e.currentTarget.src = logoSource; + e.currentTarget.src = defaultLogo; }} /> );