showing proper error message when an image is not found (#7534)

* showing proper error message when an image is not found

* deleted old spec

* added translate component

* fixed eslint

* fixed error when detecting filemimetype

* fixed e2e

* fixed unit test

* less public-form flakyness and added retries only in CI

* Fixed media field unit test

* public form e2e refactor

* fixed EntityData spec

.github/workflows/ci_cypress_component.yml

@@ -13,6 +13,8 @@ concurrency:
 jobs:
   cypress-run:
     runs-on: ubuntu-20.04
+    env:
+      CYPRESS_RETRIES: 2
     steps:
       - name: Checkout
         uses: actions/checkout@v4

.github/workflows/ci_e2e_cypress_base.yml

@@ -13,6 +13,8 @@ concurrency:
 jobs:
   e2e-cypress:
     runs-on: ubuntu-20.04
+    env:
+      CYPRESS_RETRIES: 2
     services:
       elasticsearch:
         image: docker.elastic.co/elasticsearch/elasticsearch:7.17.7-amd64
@@ -96,7 +98,7 @@ jobs:
           ELASTICSEARCH_URL: http://localhost:${{ job.services.elasticsearch.ports[9200] }}
           DATABASE_NAME: uwazi_e2e
           INDEX_NAME: uwazi_e2e
-          TRANSPILED: true          
+          TRANSPILED: true
       - run: yarn ix-config
         env:
           DBHOST: localhost:27017
@@ -119,7 +121,7 @@ jobs:
           browser: chrome
           component: false
           config-file: cypress.config.ts
-          spec: "cypress/e2e/*.cy.ts"
+          spec: 'cypress/e2e/*.cy.ts'
         timeout-minutes: 25
         env:
           DBHOST: localhost:27017

.github/workflows/ci_e2e_cypress_pages.yml

@@ -13,6 +13,8 @@ concurrency:
 jobs:
   e2e-cypress:
     runs-on: ubuntu-20.04
+    env:
+      CYPRESS_RETRIES: 2
     services:
       elasticsearch:
         image: docker.elastic.co/elasticsearch/elasticsearch:7.17.7-amd64
@@ -96,7 +98,7 @@ jobs:
           ELASTICSEARCH_URL: http://localhost:${{ job.services.elasticsearch.ports[9200] }}
           DATABASE_NAME: uwazi_e2e
           INDEX_NAME: uwazi_e2e
-          TRANSPILED: true          
+          TRANSPILED: true
       - run: yarn ix-config
         env:
           DBHOST: localhost:27017
@@ -119,7 +121,7 @@ jobs:
           browser: chrome
           component: false
           config-file: cypress.config.ts
-          spec: "cypress/e2e/pages/*.cy.ts"
+          spec: 'cypress/e2e/pages/*.cy.ts'
         timeout-minutes: 25
         env:
           DBHOST: localhost:27017

.github/workflows/ci_e2e_cypress_ssettings.yml

@@ -13,6 +13,8 @@ concurrency:
 jobs:
   e2e-cypress:
     runs-on: ubuntu-20.04
+    env:
+      CYPRESS_RETRIES: 2
     services:
       elasticsearch:
         image: docker.elastic.co/elasticsearch/elasticsearch:7.17.7-amd64
@@ -96,7 +98,7 @@ jobs:
           ELASTICSEARCH_URL: http://localhost:${{ job.services.elasticsearch.ports[9200] }}
           DATABASE_NAME: uwazi_e2e
           INDEX_NAME: uwazi_e2e
-          TRANSPILED: true          
+          TRANSPILED: true
       - run: yarn ix-config
         env:
           DBHOST: localhost:27017
@@ -119,7 +121,7 @@ jobs:
           browser: chrome
           component: false
           config-file: cypress.config.ts
-          spec: "cypress/e2e/settings/*.cy.ts"
+          spec: 'cypress/e2e/settings/*.cy.ts'
         timeout-minutes: 25
         env:
           DBHOST: localhost:27017

app/react/App/scss/modules/_metadata-view.scss

@@ -174,7 +174,8 @@ ul.comma-separated li {
 
 @mixin media-error {
   padding: 10px;
-  border: 1px #ff775c solid;
+  border: 1px #d43f3a solid;
+  color: #444444;
 }
 
 dl.metadata-type-multimedia {
@@ -184,7 +185,9 @@ dl.metadata-type-multimedia {
   }
 
   img {
-    box-shadow: 3px 1px 6px 0px rgba(0, 0, 0, 0.2), -3px 1px 6px 0px rgba(0, 0, 0, 0.2);
+    box-shadow:
+      3px 1px 6px 0px rgba(0, 0, 0, 0.2),
+      -3px 1px 6px 0px rgba(0, 0, 0, 0.2);
 
     &.contain {
       max-width: calc(100% - 10px);

app/react/Forms/components/MediaField.tsx

@@ -6,6 +6,7 @@ import { ClientFile } from 'app/istore';
 import { prepareHTMLMediaView } from 'shared/fileUploadUtils';
 import { MediaModal, MediaModalProps, MediaModalType } from 'app/Metadata/components/MediaModal';
 import MarkdownMedia, { TimeLink } from 'app/Markdown/components/MarkdownMedia';
+import { ImageViewer } from 'app/Metadata/components/ImageViewer';
 
 type MediaFieldProps = MediaModalProps & {
   value: string | { data: string; originalFile: Partial<File> } | null;
@@ -61,11 +62,6 @@ const MediaField = (props: MediaFieldProps) => {
     multipleEdition,
   } = props;
   const [openModal, setOpenModal] = useState(false);
-  const [imageRenderError, setImageRenderError] = useState(false);
-
-  useEffect(() => {
-    setImageRenderError(false);
-  }, [localAttachments]);
 
   const handleCloseMediaModal = () => {
     setOpenModal(false);
@@ -122,35 +118,16 @@ const MediaField = (props: MediaFieldProps) => {
       </div>
 
       {(() => {
-        if (imageRenderError) {
-          return (
-            <div className="media-error">
-              <Translate>This file type is not supported on media fields</Translate>
-            </div>
-          );
-        }
         if (
           (file &&
             file.data &&
             file.supportingFile &&
             file.supportingFile.mimetype?.search(/image\/*/) !== -1) ||
           type === MediaModalType.Image
         ) {
-          return file?.fileURL ? (
-            <img
-              src={file?.fileURL}
-              alt=""
-              onError={() => {
-                if (file?.fileURL) {
-                  setImageRenderError(true);
-                }
-              }}
-            />
-          ) : (
-            // eslint-disable-next-line react/jsx-no-useless-fragment
-            <></>
-          );
+          return file?.fileURL ? <ImageViewer src={file.fileURL} alt="media" /> : null;
         }
+
         if (file?.fileURL) {
           return (
             <MarkdownMedia
@@ -161,6 +138,7 @@ const MediaField = (props: MediaFieldProps) => {
             />
           );
         }
+        return null;
       })()}
 
       <MediaModal

app/react/Forms/components/specs/MediaField.spec.tsx

@@ -67,22 +67,34 @@ describe('MediaField', () => {
     name: '',
   };
 
-  const render = (otherProps = {}) => {
-    ({ renderResult } = renderConnectedContainer(
-      <MediaField
-        formField=""
-        type={MediaModalType.Image}
-        value={null}
-        {...{ ...baseProps, ...otherProps }}
-      />,
-      () => defaultState
-    ));
+  const render = async (otherProps = {}) => {
+    await act(async () => {
+      ({ renderResult } = renderConnectedContainer(
+        <MediaField
+          formField=""
+          type={MediaModalType.Image}
+          value={null}
+          {...{ ...baseProps, ...otherProps }}
+        />,
+        () => defaultState
+      ));
+    });
   };
 
   describe('Object URL handling', () => {
     const mockedCreateObjectURL: jest.Mock = jest.fn();
     const mockedRevokeObjectURL: jest.Mock = jest.fn();
 
+    beforeEach(() => {
+      jest.clearAllMocks();
+      global.fetch = jest.fn();
+      (global.fetch as jest.Mock).mockResolvedValue({ ok: true });
+    });
+
+    afterEach(() => {
+      jest.restoreAllMocks();
+    });
+
     beforeAll(() => {
       URL.createObjectURL = mockedCreateObjectURL;
       mockedCreateObjectURL.mockReturnValue('blob:abc');
@@ -96,20 +108,21 @@ describe('MediaField', () => {
 
     it('should create an object URL with the file', async () => {
       const file = new File(['hello'], 'hello.png', { type: 'image/png' });
-      render(imageProps);
-      const img = screen.getByRole('presentation') as HTMLImageElement;
+      await render(imageProps);
+
+      const img = screen.getByRole('img') as HTMLImageElement;
       expect(img.src).toEqual('blob:abc');
       expect(mockedCreateObjectURL).toHaveBeenCalledWith(file);
     });
 
     it('should revoke the created URL', async () => {
-      render(imageProps);
+      await render(imageProps);
       renderResult.unmount();
       expect(mockedRevokeObjectURL).toHaveBeenCalledWith('blob:abc');
     });
 
     it('should change the media value according with markdownmedia variations', async () => {
-      render(mediaProps);
+      await render(mediaProps);
 
       await act(async () => {
         fireEvent.click(screen.getByText('Add timelink').parentElement!);
@@ -158,7 +171,7 @@ describe('MediaField', () => {
     });
 
     it('should show and error if the image is not valid', async () => {
-      render(imageProps);
+      await render(imageProps);
       const img = renderResult.container.getElementsByTagName('img')[0];
       fireEvent.error(img);
       expect(

app/react/Markdown/components/specs/EntityData.spec.tsx

@@ -1,75 +1,82 @@
 /**
  * @jest-environment jsdom
  */
-import React from 'react';
-import { ReactWrapper } from 'enzyme';
-import { renderConnectedMount } from 'app/utils/test/renderConnected';
+import React, { act } from 'react';
+import { screen, RenderResult } from '@testing-library/react';
+import { renderConnectedContainer } from 'app/utils/test/renderConnected';
 import { state } from './fixture/state';
-import { EntityData } from '../EntityData';
+import { EntityData, EntityDataProps } from '../EntityData';
 
 describe('EntityData Markdown', () => {
-  let component: ReactWrapper<
-    Readonly<{}> & Readonly<{ children?: React.ReactNode }>,
-    Readonly<{}>,
-    React.Component<{}, {}, any>
-  >;
   let consoleErrorSpy: jasmine.Spy;
+  let renderResult: RenderResult;
 
   beforeEach(() => {
+    jest.clearAllMocks();
     consoleErrorSpy = jasmine.createSpy('consoleErrorSpy');
     spyOn(console, 'error').and.callFake(consoleErrorSpy);
+    global.fetch = jest.fn();
+    (global.fetch as jest.Mock).mockResolvedValue({ ok: true });
   });
 
-  const render = (innerComponent: any) => {
-    component = renderConnectedMount(() => innerComponent, state);
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  const render = async (props: EntityDataProps) => {
+    await act(async () => {
+      ({ renderResult } = renderConnectedContainer(<EntityData {...props} />, () => state));
+    });
   };
 
   describe('root properties Values', () => {
-    it('should print title and root dates from root of entity', () => {
-      render(<EntityData value-of="title" />);
-      expect(component.html()).toBe('Entity 1');
+    it('should print title and root dates from root of entity', async () => {
+      await render({ 'value-of': 'title' });
+      expect(screen.getByText('Entity 1')).toBeInTheDocument();
 
-      render(<EntityData value-of="creationDate" />);
-      expect(component.html()).toBe('1234');
+      await render({ 'value-of': 'creationDate' });
+      expect(screen.getByText('1234')).toBeInTheDocument();
     });
   });
 
   describe('metadata property Values', () => {
-    it('should print formatted metadata properties (sanitizing names)', () => {
-      render(<EntityData value-of="description" />);
-      expect(component.html()).toBe('A long description');
+    it('should print formatted metadata properties (sanitizing names)', async () => {
+      await render({ 'value-of': 'description' });
+      expect(screen.getByText('A long description')).toBeInTheDocument();
 
-      render(<EntityData value-of="date" />);
-      expect(component.html()).toBe('Jul 13, 1977');
+      await render({ 'value-of': 'date' });
+      expect(screen.getByText('Jul 13, 1977')).toBeInTheDocument();
 
-      render(<EntityData value-of="Main Image" />);
-      expect(component.html()).toContain('src="https://www.google.com"');
+      await render({ 'value-of': 'Main Image' });
+      expect(screen.getByRole('img').getAttribute('src')).toBe('https://www.google.com');
     });
   });
 
   describe('labels (property names)', () => {
-    it('should print translated labels (sanitizing names)', () => {
-      render(<EntityData label-of="title" />);
-      expect(component.html()).toContain('Title translated');
+    it('should print translated labels (sanitizing names)', async () => {
+      await render({ 'label-of': 'title' });
+      expect(screen.getByText('Title translated')).toBeInTheDocument();
 
-      render(<EntityData label-of="Main Image" />);
-      expect(component.html()).toContain('Main Image translated');
+      await render({ 'label-of': 'Main Image' });
+      expect(screen.getByText('Main Image translated')).toBeInTheDocument();
     });
   });
 
   describe('Error handling (until upstream implementation is implemented)', () => {
-    it('should fail if no value or propertyName is provided', () => {
-      render(<EntityData />);
-      expect(component.html()).toEqual('');
+    it('should fail if no value or propertyName is provided', async () => {
+      await render({});
+      //get the body
+      expect(renderResult.container.innerHTML).toBe('');
       expect(consoleErrorSpy).toHaveBeenCalledWith('Error on EntityData: ');
       expect(consoleErrorSpy.calls.all()[2].args[0].message).toBe(
         '"value-of" or "label-of" must be provided.'
       );
     });
 
-    it('should fail if both value and propertyName are provided', () => {
-      render(<EntityData value-of="something" label-of="something else" />);
-      expect(component.html()).toEqual('');
+    it('should fail if both value and propertyName are provided', async () => {
+      await render({ 'value-of': 'something', 'label-of': 'something else' });
+      //assert empty html
+      expect(renderResult.container.innerHTML).toBe('');
       expect(consoleErrorSpy).toHaveBeenCalledWith('Error on EntityData: ');
       expect(consoleErrorSpy.calls.all()[2].args[0].message).toBe(
         'Can\'t provide both "value-of" and "label-of".'