[MM-61166] Cherrypick to release-0.29 (#889)

* [MM-61166] Ensure full validation of Calls post props (#885) * Ensure full validation of Calls post props * [MM-61228] Surface transcriptions in call post (#886) * Surface transcriptions in call post * Add proper job metadata validation * [MM-61298] Fix e2e pipeline (#888) * Fix e2e pipeline * We don't need ES * Fix docker login step (#881) * Fix docker login step * Update .github/workflows/e2e.yml Co-authored-by: Mario Vitale <[email protected]> --------- Co-authored-by: Mario Vitale <[email protected]> * Fix e2e pipeline flakyness (#867) --------- Co-authored-by: Mario Vitale <[email protected]>
mattermost · Oct 25, 2024 · fefebc6 · fefebc6
1 parent eb9c42a
commit fefebc6
Show file tree

Hide file tree

Showing 9 changed files with 229 additions and 50 deletions.
diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml
@@ -158,7 +158,7 @@ jobs:
       - name: e2e/docker-login
         uses: docker/login-action@0d4c9c5ea7693da7b068278f7b52bda2a190a446 # v3.2.0
         # Do not authenticate on Forks
-        if: github.event.pull_request.head.repo.full_name == github.repository
+        if: "github.event.repository.full_name == github.repository"
         with:
           username: ${{ secrets.DOCKERHUB_DEV_USERNAME }}
           password: ${{ secrets.DOCKERHUB_DEV_TOKEN }}
@@ -167,8 +167,7 @@ jobs:
         env:
           DOCKER_CLIENT_TIMEOUT: 120
           COMPOSE_HTTP_TIMEOUT: 120
-          ## Should relative to mattermost/server/build/
-          DOCKER_COMPOSE_FILE: gitlab-dc.postgres.yml
+          DOCKER_COMPOSE_FILE: ${{ github.workspace }}/e2e/docker/docker-compose.yaml
           TRANSCRIBER_IMAGE_PATH: ${{ github.workspace }}/calls-transcriber.tar
         run: |
           mkdir -p ${{ github.workspace }}/logs

diff --git a/e2e/docker/docker-compose.yaml b/e2e/docker/docker-compose.yaml
@@ -0,0 +1,35 @@
+version: '2.4'
+services:
+  postgres:
+    image: "postgres:11"
+    restart: always
+    tmpfs: /var/lib/postgresql/data
+    networks:
+      default:
+        aliases:
+          - postgres
+    environment:
+      POSTGRES_USER: mmuser
+      POSTGRES_PASSWORD: mostest
+      POSTGRES_DB: mattermost_test
+    command: postgres -c 'config_file=/etc/postgresql/postgresql.conf'
+    volumes:
+     - "./postgres.conf:/etc/postgresql/postgresql.conf"
+    healthcheck:
+      test: [ "CMD", "pg_isready", "-h", "localhost" ]
+      interval: 5s
+      timeout: 10s
+      retries: 3
+
+  start_dependencies:
+    image: mattermost/mattermost-wait-for-dep:latest
+    depends_on:
+      - postgres
+    command: postgres:5432
+    networks:
+      default:
+
+networks:
+  default:
+    name: ${DOCKER_NETWORK}
+    external: true
diff --git a/e2e/docker/postgres.conf b/e2e/docker/postgres.conf
@@ -0,0 +1,7 @@
+max_connections = 300
+listen_addresses = '*'
+fsync = off
+full_page_writes = off
+default_text_search_config = 'pg_catalog.english'
+commit_delay=1000
+logging_collector=off
diff --git a/e2e/scripts/prepare-server.sh b/e2e/scripts/prepare-server.sh
@@ -6,9 +6,8 @@ docker network create ${DOCKER_NETWORK}
 
 # Start server dependencies
 echo "Starting server dependencies ... "
-docker compose -f ${DOCKER_COMPOSE_FILE} run -d --rm start_dependencies
+DOCKER_NETWORK=${DOCKER_NETWORK} docker compose -f ${DOCKER_COMPOSE_FILE} run -d --rm start_dependencies
 timeout --foreground 90s bash -c "until docker compose -f ${DOCKER_COMPOSE_FILE} exec -T postgres pg_isready ; do sleep 5 ; done"
-docker compose -f ${DOCKER_COMPOSE_FILE} exec -d -T minio sh -c 'mkdir -p /data/mattermost-test'
 
 echo "Pulling ${IMAGE_CALLS_RECORDER} in order to be quickly accessible ... "
 # Pull calls-recorder image to be used by calls-offloader.
@@ -40,6 +39,3 @@ docker run -d --quiet --user root --name "${COMPOSE_PROJECT_NAME}_callsoffloader
 
 # Check that calls-offloader is up and ready
 docker run --rm --quiet --name "${COMPOSE_PROJECT_NAME}_curl_callsoffloader" --net ${DOCKER_NETWORK} ${IMAGE_CURL} sh -c "until curl -fs http://calls-offloader:4545/version; do echo Waiting for calls-offloader; sleep 5; done; echo calls-offloader is up"
-
-# Check that elasticsearch is ready
-docker run --rm --quiet --name "${COMPOSE_PROJECT_NAME}_curl_elasticsearch" --net ${DOCKER_NETWORK} ${IMAGE_CURL} sh -c "until curl --max-time 5 --output - http://elasticsearch:9200; do echo Waiting for elasticsearch; sleep 5; done; echo elasticsearch is up"
diff --git a/e2e/scripts/run.sh b/e2e/scripts/run.sh
@@ -58,13 +58,13 @@ docker ps -a
 docker logs "${COMPOSE_PROJECT_NAME}_callsoffloader"
 
 # Print transcriber job logs in case of failure.
-for ID in $(docker ps -a --filter=ancestor="calls-transcriber:master" --format "{{.ID}}")
+for ID in $(docker ps -a --filter=ancestor="calls-transcriber:master" --filter=status="exited" --format "{{.ID}}")
 do
   docker logs $ID
 done
 
 # Print recorder job logs in case of failure.
-for ID in $(docker ps -a --filter=ancestor="calls-recorder:master" --format "{{.ID}}")
+for ID in $(docker ps -a --filter=ancestor="calls-recorder:master" --filter=status="exited" --format "{{.ID}}")
 do
   docker logs $ID
 done

diff --git a/webapp/i18n/en.json b/webapp/i18n/en.json
@@ -218,6 +218,7 @@
   "yL9vD/": "Welcome to your Mattermost Enterprise trial! It expires on {trialExpirationDate}. You now have access to <rtcdDocsLink>RTCD services</rtcdDocsLink>, <recordingsDocsLink>call recordings</recordingsDocsLink>, <guestAccountsLink>guest accounts</guestAccountsLink>, <autoComplianceReportsLink>automated compliance reports</autoComplianceReportsLink>, and <mobileSecureNotificationsLink>mobile secure-ID push notifications</mobileSecureNotificationsLink>, among many other features. View all features in our <documentationLink>documentation</documentationLink>.",
   "yO68rk": "Calls are currently running in test mode and only system admins can start them. Reach out directly to your system admin for assistance",
   "yjQNKt": "Calls are disabled in this channel.",
+  "yx0b7N": "{count, plural, =1 {# transcription} other {# transcriptions}}",
   "zUH89x": "The call recording will be processed and posted in the call thread. Are you sure you want to stop the recording?",
   "zx0myy": "Participants"
 }
diff --git a/webapp/src/components/custom_post_types/post_type/component.tsx b/webapp/src/components/custom_post_types/post_type/component.tsx
@@ -80,19 +80,30 @@ const PostType = ({
         }
     };
 
-    const recordings = callProps.recording_files?.length || 0;
+    const recordings = Object.values(callProps.recordings).filter(job => Boolean(job.file_id)).map(job => job.file_id);
+    const transcriptions = Object.values(callProps.transcriptions).filter(job => Boolean(job.file_id)).map(job => job.file_id);
 
-    const recordingsSubMessage = recordings > 0 ? (
-        <RecordingsContainer>
+    const recordingsSubMessage = recordings.length > 0 ? (
+        <ArtifactsContainer>
             <CompassIcon
                 icon='file-video-outline'
                 style={{display: 'inline', fontSize: '16px'}}
             />
-            <span>{formatMessage({defaultMessage: '{count, plural, =1 {# recording} other {# recordings}}'}, {count: recordings})}</span>
-        </RecordingsContainer>
+            <span>{formatMessage({defaultMessage: '{count, plural, =1 {# recording} other {# recordings}}'}, {count: recordings.length})}</span>
+        </ArtifactsContainer>
     ) : null;
 
-    const subMessage = callProps.start_at && callProps.end_at ? (
+    const transcriptionsSubMessage = recordings.length > 0 ? (
+        <ArtifactsContainer>
+            <CompassIcon
+                icon='file-text-outline'
+                style={{display: 'inline', fontSize: '16px'}}
+            />
+            <span>{formatMessage({defaultMessage: '{count, plural, =1 {# transcription} other {# transcriptions}}'}, {count: transcriptions.length})}</span>
+        </ArtifactsContainer>
+    ) : null;
+
+    const subMessage = callProps.start_at > 0 && callProps.end_at > 0 ? (
         <>
             <span>
                 {formatMessage(
@@ -159,7 +170,7 @@ const PostType = ({
 
     const compactTitle = compactDisplay && !isRHS ? <br/> : <></>;
     const title = callProps.title ? <h3 className='markdown__heading'>{callProps.title}</h3> : compactTitle;
-    const callActive = !callProps.end_at;
+    const callActive = callProps.end_at === 0;
     const inCall = connectedID === post.channel_id;
     const iconAndText = (
         <>
@@ -189,14 +200,14 @@ const PostType = ({
             <Main data-testid={'call-thread'}>
                 <SubMain>
                     <Left>
-                        <CallIndicator $ended={Boolean(callProps.end_at)}>
-                            {!callProps.end_at &&
+                        <CallIndicator $ended={!callActive}>
+                            {callActive &&
                                 <ActiveCallIcon
                                     fill='var(--center-channel-bg)'
                                     style={{width: '20px', height: '20px'}}
                                 />
                             }
-                            {callProps.end_at &&
+                            {!callActive &&
                                 <LeaveCallIcon
                                     fill={'rgba(var(--center-channel-color-rgb), 0.72)'}
                                     style={{width: '24px', height: '20px'}}
@@ -205,17 +216,17 @@ const PostType = ({
                         </CallIndicator>
                         <MessageWrapper>
                             <Message>
-                                {!callProps.end_at &&
+                                {callActive &&
                                     formatMessage({defaultMessage: 'Call started'})
                                 }
-                                {callProps.end_at &&
+                                {!callActive &&
                                     formatMessage({defaultMessage: 'Call ended'})
                                 }
                             </Message>
                             <SubMessage>{subMessage}</SubMessage>
                         </MessageWrapper>
                     </Left>
-                    { (recordings > 0 || callActive) && <RowDivider/> }
+                    { (recordings.length > 0 || callActive) && <RowDivider/> }
                     <Right>
                         {callActive &&
                             <>
@@ -231,7 +242,8 @@ const PostType = ({
                                 {button}
                             </>
                         }
-                        {recordingsSubMessage}
+                        {recordings.length > 0 && recordingsSubMessage}
+                        {transcriptions.length > 0 && transcriptionsSubMessage}
                     </Right>
                 </SubMain>
             </Main>
@@ -428,7 +440,7 @@ const Divider = styled.span`
     margin: 0 4px;
 `;
 
-const RecordingsContainer = styled.div`
+const ArtifactsContainer = styled.div`
     display: flex;
     align-items: center;
     font-size: 12px;

diff --git a/webapp/src/utils.test.ts b/webapp/src/utils.test.ts
@@ -255,11 +255,12 @@ describe('utils', () => {
 
             const props = getCallPropsFromPost(post);
 
-            expect(props.start_at).toBeUndefined();
-            expect(props.end_at).toBeUndefined();
-            expect(props.recordings.length).toBe(0);
+            expect(props.title).toBe('');
+            expect(props.start_at).toBe(0);
+            expect(props.end_at).toBe(0);
+            expect(props.recordings).toStrictEqual({});
             expect(props.recording_files.length).toBe(0);
-            expect(props.transcriptions.length).toBe(0);
+            expect(props.transcriptions).toStrictEqual({});
             expect(props.participants.length).toBe(0);
         });
 
@@ -270,14 +271,107 @@ describe('utils', () => {
 
             const props = getCallPropsFromPost(post);
 
-            expect(props.start_at).toBeUndefined();
-            expect(props.end_at).toBeUndefined();
-            expect(props.recordings.length).toBe(0);
+            expect(props.title).toBe('');
+            expect(props.start_at).toBe(0);
+            expect(props.end_at).toBe(0);
+            expect(props.recordings).toStrictEqual({});
             expect(props.recording_files.length).toBe(0);
-            expect(props.transcriptions.length).toBe(0);
+            expect(props.transcriptions).toStrictEqual({});
             expect(props.participants.length).toBe(0);
         });
 
+        test('invalid props', () => {
+            const callProps = {
+                title: {},
+                start_at: 'invalid',
+                end_at: [],
+                recordings: null,
+                transcriptions: 45,
+                participants: 'invalid',
+                recording_files: 45,
+            };
+
+            const post = {
+                props: callProps as unknown,
+            } as Post;
+
+            const props = getCallPropsFromPost(post);
+
+            expect(props.title).toBe('');
+            expect(props.start_at).toBe(0);
+            expect(props.end_at).toBe(0);
+            expect(props.recordings).toStrictEqual({});
+            expect(props.recording_files.length).toBe(0);
+            expect(props.transcriptions).toStrictEqual({});
+            expect(props.participants.length).toBe(0);
+        });
+
+        test('invalid job data', () => {
+            const callProps = {
+                recordings: {
+                    recA: {
+                        file_id: true,
+                        post_id: null,
+                        tr_id: 45,
+                        rec_id: 45,
+                    },
+                    45: {
+                    },
+                    recB: {
+                        file_id: 'recFileID',
+                    },
+                },
+                transcriptions: {
+                    trA: {
+                        file_id: true,
+                        post_id: null,
+                        tr_id: 45,
+                        rec_id: 45,
+                    },
+                    45: {
+                    },
+                    trB: {
+                        file_id: 'trFileID',
+                    },
+                },
+            };
+
+            const post = {
+                props: callProps as unknown,
+            } as Post;
+
+            const props = getCallPropsFromPost(post);
+
+            expect(props.recordings).toStrictEqual({
+                recA: {
+                    file_id: '',
+                    post_id: '',
+                },
+                45: {
+                    file_id: '',
+                    post_id: '',
+                },
+                recB: {
+                    file_id: 'recFileID',
+                    post_id: '',
+                },
+            });
+            expect(props.transcriptions).toStrictEqual({
+                trA: {
+                    file_id: '',
+                    post_id: '',
+                },
+                45: {
+                    file_id: '',
+                    post_id: '',
+                },
+                trB: {
+                    file_id: 'trFileID',
+                    post_id: '',
+                },
+            });
+        });
+
         test('full props', () => {
             const callProps = {
                 title: 'call title',
@@ -320,9 +414,9 @@ describe('utils', () => {
             expect(props.title).toBe(post.props.title);
             expect(props.start_at).toBe(post.props.start_at);
             expect(props.end_at).toBe(post.props.end_at);
-            expect(props.recordings).toBe(post.props.recordings);
+            expect(props.recordings).toStrictEqual(post.props.recordings);
             expect(props.recording_files).toBe(post.props.recording_files);
-            expect(props.transcriptions).toBe(post.props.transcriptions);
+            expect(props.transcriptions).toStrictEqual(post.props.transcriptions);
             expect(props.participants).toBe(post.props.participants);
         });
     });
@@ -333,8 +427,8 @@ describe('utils', () => {
 
             const props = getCallRecordingPropsFromPost(post);
 
-            expect(props.call_post_id).toBeUndefined();
-            expect(props.recording_id).toBeUndefined();
+            expect(props.call_post_id).toBe('');
+            expect(props.recording_id).toBe('');
             expect(props.captions.length).toBe(0);
         });
 
@@ -345,8 +439,26 @@ describe('utils', () => {
 
             const props = getCallRecordingPropsFromPost(post);
 
-            expect(props.call_post_id).toBeUndefined();
-            expect(props.recording_id).toBeUndefined();
+            expect(props.call_post_id).toBe('');
+            expect(props.recording_id).toBe('');
+            expect(props.captions.length).toBe(0);
+        });
+
+        test('invalid props', () => {
+            const recProps = {
+                call_post_id: 45,
+                recording_id: {},
+                captions: 'invalid',
+            };
+
+            const post = {
+                props: recProps as unknown,
+            } as Post;
+
+            const props = getCallRecordingPropsFromPost(post);
+
+            expect(props.call_post_id).toBe('');
+            expect(props.recording_id).toBe('');
             expect(props.captions.length).toBe(0);
         });