gabotorresruiz commented on code in PR #35254:
URL: https://github.com/apache/superset/pull/35254#discussion_r2402383629
##########
superset-frontend/src/features/themes/ThemeModal.test.tsx:
##########
@@ -87,201 +57,547 @@ const mockTheme: ThemeObject = {
},
};
-// Mock theme API endpoints
-fetchMock.get('glob:*/api/v1/theme/1', {
- result: mockTheme,
+const mockSystemTheme: ThemeObject = {
+ ...mockTheme,
+ id: 2,
+ theme_name: 'System Theme',
+ is_system: true,
+};
+
+const setupFetchMocks = () => {
+ fetchMock.reset();
+ fetchMock.get('glob:*/api/v1/theme/1', { result: mockTheme });
+ fetchMock.get('glob:*/api/v1/theme/2', { result: mockSystemTheme });
+ fetchMock.get('glob:*/api/v1/theme/*', { result: mockTheme });
+ fetchMock.post('glob:*/api/v1/theme/', { result: { ...mockTheme, id: 3 } });
+ fetchMock.put('glob:*/api/v1/theme/*', { result: mockTheme });
+};
+
+afterEach(() => {
+ fetchMock.restore();
+ jest.clearAllMocks();
});
-fetchMock.post('glob:*/api/v1/theme/', {
- result: { ...mockTheme, id: 2 },
+test('renders modal with add theme dialog when show is true', () => {
+ setupFetchMocks();
+ render(
+ <ThemeModal
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ onThemeAdd={jest.fn()}
+ onHide={jest.fn()}
+ show
+ canDevelop={false}
+ />,
+ { useRedux: true, useRouter: true },
+ );
+
+ expect(screen.getByRole('dialog')).toBeInTheDocument();
+ expect(screen.getByText('Add theme')).toBeInTheDocument();
});
-fetchMock.put('glob:*/api/v1/theme/1', {
- result: mockTheme,
+test('does not render modal when show is false', () => {
+ setupFetchMocks();
+ render(
+ <ThemeModal
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ onThemeAdd={jest.fn()}
+ onHide={jest.fn()}
+ show={false}
+ canDevelop={false}
+ />,
+ { useRedux: true, useRouter: true },
+ );
+
+ expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
});
-// These are defined but not used in the simplified tests
-// const mockUser = {
-// userId: 1,
-// firstName: 'Admin',
-// lastName: 'User',
-// roles: { Admin: [['can_write', 'Dashboard']] },
-// permissions: {},
-// isActive: true,
-// isAnonymous: false,
-// username: 'admin',
-// email: '[email protected]',
-// user_id: 1,
-// first_name: 'Admin',
-// last_name: 'User',
-// };
-
-// const defaultProps = {
-// addDangerToast: jest.fn(),
-// addSuccessToast: jest.fn(),
-// onThemeAdd: jest.fn(),
-// onHide: jest.fn(),
-// show: true,
-// };
-
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('ThemeModal', () => {
- beforeEach(() => {
- fetchMock.resetHistory();
- jest.clearAllMocks();
- });
+test('renders edit mode title when theme is provided', async () => {
+ setupFetchMocks();
+ render(
+ <ThemeModal
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ onThemeAdd={jest.fn()}
+ onHide={jest.fn()}
+ show
+ canDevelop={false}
+ theme={mockTheme}
+ />,
+ { useRedux: true, useRouter: true },
+ );
+
+ // Wait for theme name to be loaded in the input field
+ expect(await screen.findByDisplayValue('Test Theme')).toBeInTheDocument();
+ expect(screen.getByText('Edit theme properties')).toBeInTheDocument();
+});
- afterEach(() => {
- fetchMock.restore();
- });
+test('renders view mode title for system themes', async () => {
+ setupFetchMocks();
+ render(
+ <ThemeModal
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ onThemeAdd={jest.fn()}
+ onHide={jest.fn()}
+ show
+ canDevelop={false}
+ theme={mockSystemTheme}
+ />,
+ { useRedux: true, useRouter: true },
+ );
+
+ // Wait for system theme indicator to appear
+ expect(
+ await screen.findByText('System Theme - Read Only'),
+ ).toBeInTheDocument();
+ expect(screen.getByText('View theme properties')).toBeInTheDocument();
+});
- test('should export ThemeModal component', () => {
- const ThemeModalModule = jest.requireActual('./ThemeModal');
- expect(ThemeModalModule.default).toBeDefined();
- expect(typeof ThemeModalModule.default).toBe('object'); // HOC wrapped
component
- });
+test('renders theme name input field', () => {
+ setupFetchMocks();
+ render(
+ <ThemeModal
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ onThemeAdd={jest.fn()}
+ onHide={jest.fn()}
+ show
+ canDevelop={false}
+ />,
+ { useRedux: true, useRouter: true },
+ );
+
+ expect(screen.getByPlaceholderText('Enter theme name')).toBeInTheDocument();
+});
- test('should have correct type definitions', () => {
- expect(mockTheme).toMatchObject({
- id: expect.any(Number),
- theme_name: expect.any(String),
- json_data: expect.any(String),
- changed_on_delta_humanized: expect.any(String),
- changed_by: expect.objectContaining({
- first_name: expect.any(String),
- last_name: expect.any(String),
- }),
- });
- });
+test('renders JSON configuration field', () => {
+ setupFetchMocks();
+ render(
+ <ThemeModal
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ onThemeAdd={jest.fn()}
+ onHide={jest.fn()}
+ show
+ canDevelop={false}
+ />,
+ { useRedux: true, useRouter: true },
+ );
+
+ expect(screen.getByText('JSON Configuration')).toBeInTheDocument();
+});
- test('should validate JSON data structure', () => {
- const isValidJson = (str: string) => {
- try {
- JSON.parse(str);
- return true;
- } catch (e) {
- return false;
- }
- };
-
- expect(isValidJson(mockTheme.json_data || '')).toBe(true);
- expect(isValidJson('invalid json')).toBe(false);
- expect(isValidJson('{"valid": "json"}')).toBe(true);
- });
+test('disables inputs for read-only system themes', async () => {
+ setupFetchMocks();
+ render(
+ <ThemeModal
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ onThemeAdd={jest.fn()}
+ onHide={jest.fn()}
+ show
+ canDevelop={false}
+ theme={mockSystemTheme}
+ />,
+ { useRedux: true, useRouter: true },
+ );
+
+ const nameInput = await screen.findByPlaceholderText('Enter theme name');
+
+ expect(nameInput).toHaveAttribute('readOnly');
+});
- test('should handle theme data parsing', () => {
- const parsedTheme = JSON.parse(mockTheme.json_data || '{}');
- expect(parsedTheme).toMatchObject({
- colors: {
- primary: '#1890ff',
- secondary: '#52c41a',
- },
- typography: {
- fontSize: 14,
- },
- });
- });
+test('shows Apply button when canDevelop is true and theme exists', async ()
=> {
+ setupFetchMocks();
Review Comment:
You're right!
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]