Skip to content

feat: algolia video troubleshooter backend#8463

Merged
tanflem merged 38 commits intomainfrom
tannerfleming/vmt-189-backend-video-managment-algolia-troubleshooting
Jan 23, 2026
Merged

feat: algolia video troubleshooter backend#8463
tanflem merged 38 commits intomainfrom
tannerfleming/vmt-189-backend-video-managment-algolia-troubleshooting

Conversation

@tanflem
Copy link
Copy Markdown
Contributor

@tanflem tanflem commented Dec 5, 2025

Summary by CodeRabbit

Release Notes

New Features

  • Added ability to verify if videos are properly indexed in search with detailed mismatch reporting
  • Added ability to manually trigger reindexing for videos and their variants
  • Added queries to check video indexing status and report missing or mismatched data
  • Added mutations to update video and variant search indexes

✏️ Tip: You can customize this high-level summary in your review settings.

@linear
Copy link
Copy Markdown

linear Bot commented Dec 5, 2025

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 5, 2025

Walkthrough

This PR introduces GraphQL queries and mutations to validate and synchronize video records with the Algolia search index. The Algolia client infrastructure is refactored from asynchronous with logger dependency to synchronous configuration-based access patterns.

Changes

Cohort / File(s) Summary
GraphQL Schema Definitions
apis/api-gateway/schema.graphql, apis/api-media/schema.graphql
Added three new types (CheckVideoInAlgoliaMismatch, CheckVideoInAlgoliaResult, CheckVideoVariantsInAlgoliaResult), two queries (checkVideoInAlgolia, checkVideoVariantsInAlgolia), and two mutations (updateVideoAlgoliaIndex, updateVideoVariantAlgoliaIndex) to expose Algolia validation and indexing operations.
Algolia Validation & Indexing Implementation
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
New module implementing GraphQL queries to detect field mismatches and missing variants in Algolia, and mutations to trigger index updates; includes deterministic field comparison logic, error handling, and recordUrl construction.
Algolia Client & Configuration Refactoring
apis/api-media/src/lib/algolia/algoliaClient.ts, apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts, apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts
Refactored Algolia client from async with logger dependency to synchronous; introduced getAlgoliaConfig() to centralize environment variable handling; updated dependent modules to use new configuration pattern.
Module Integration
apis/api-media/src/schema/video/index.ts, apis/api-media/src/schema/video/videoAlgolia/index.ts
Barrel file exports to integrate new videoAlgolia module into schema structure.
Test Updates
apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts, apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts, apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
Refactored existing test mocks to use centralized getAlgoliaConfig() pattern; removed environment variable setup; added comprehensive test coverage (568 lines) for Algolia validation queries, index update mutations, and authorization checks.

Sequence Diagram(s)

sequenceDiagram
    participant Client as GraphQL Client
    participant Resolver as checkVideoInAlgolia<br/>Resolver
    participant Prisma as Prisma ORM
    participant Algolia as Algolia Client
    participant Response as GraphQL Response

    Client->>Resolver: checkVideoInAlgolia(videoId)
    Resolver->>Prisma: findVideo(videoId)
    alt Video not found
        Prisma-->>Resolver: null
        Resolver-->>Response: error
    else Video found
        Prisma-->>Resolver: video data
        Resolver->>Algolia: getObject(videoId)
        alt Algolia object not found
            Algolia-->>Resolver: exception
            Resolver-->>Response: { ok: false, error, recordUrl }
        else Algolia object found
            Algolia-->>Resolver: algolia record
            Resolver->>Resolver: Compare fields<br/>(deterministic)
            alt Mismatches detected
                Resolver-->>Response: { ok: false, mismatches[], recordUrl }
            else All fields match
                Resolver-->>Response: { ok: true, mismatches: [], recordUrl }
            end
        end
    end
Loading
sequenceDiagram
    participant Client as GraphQL Client
    participant Resolver as updateVideoAlgoliaIndex<br/>Mutation
    participant Prisma as Prisma ORM
    participant UpdateFn as updateVideoInAlgolia
    participant Algolia as Algolia Client
    participant Response as GraphQL Response

    Client->>Resolver: updateVideoAlgoliaIndex(videoId)
    Resolver->>Prisma: findVideo(videoId)
    alt Video not found
        Prisma-->>Resolver: null
        Resolver-->>Response: error
    else Video found
        Prisma-->>Resolver: video data
        Resolver->>UpdateFn: updateVideoInAlgolia(video)
        UpdateFn->>Algolia: saveObjects([transformed video])
        Algolia-->>UpdateFn: success
        UpdateFn-->>Resolver: success
        Resolver-->>Response: { true }
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • feat: add algolia index updates #6857: Directly exposes and utilizes the updateVideoInAlgolia and updateVideoVariantInAlgolia functions alongside Algolia client/config utilities that are refined in this PR.
  • chore: remove algolia workers #7353: Related Algolia worker exports and import path refactoring in apis/api-media that aligns with the client configuration pattern introduced here.

Suggested reviewers

  • mikeallisonJS
  • csiyang
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: algolia video troubleshooter backend' accurately describes the main change—adding backend GraphQL operations and types for Algolia video index validation and troubleshooting.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch tannerfleming/vmt-189-backend-video-managment-algolia-troubleshooting

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Dec 5, 2025

View your CI Pipeline Execution ↗ for commit 966153a

Command Status Duration Result
nx run player-e2e:e2e ✅ Succeeded 8s View ↗
nx run journeys-admin-e2e:e2e ✅ Succeeded 32s View ↗
nx run short-links-e2e:e2e ✅ Succeeded 3s View ↗
nx run journeys-e2e:e2e ✅ Succeeded 24s View ↗
nx run watch-e2e:e2e ✅ Succeeded 24s View ↗
nx run resources-e2e:e2e ✅ Succeeded 13s View ↗
nx run videos-admin-e2e:e2e ✅ Succeeded 5s View ↗
nx run-many --target=vercel-alias --projects=jo... ✅ Succeeded 2s View ↗
Additional runs (20) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2026-01-22 15:37:26 UTC

@github-actions github-actions Bot had a problem deploying to Preview - short-links December 5, 2025 16:47 Failure
@github-actions github-actions Bot temporarily deployed to Preview - videos-admin December 5, 2025 16:47 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - resources December 5, 2025 16:47 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - journeys-admin December 5, 2025 16:47 Inactive
@tanflem tanflem self-assigned this Dec 5, 2025
@tanflem tanflem requested a review from mikeallisonJS December 5, 2025 16:48
@tanflem tanflem marked this pull request as draft December 5, 2025 16:48
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2025

The latest updates on your projects.

Name Status Preview Updated (UTC)
videos-admin ✅ Ready videos-admin preview Fri Jan 23 04:31:39 NZDT 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2025

The latest updates on your projects.

Name Status Preview Updated (UTC)
watch ✅ Ready watch preview Fri Jan 23 04:31:26 NZDT 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2025

The latest updates on your projects.

Name Status Preview Updated (UTC)
resources ✅ Ready resources preview Fri Jan 23 04:30:58 NZDT 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2025

The latest updates on your projects.

Name Status Preview Updated (UTC)
journeys-admin ✅ Ready journeys-admin preview Fri Jan 23 04:31:50 NZDT 2026

@tanflem tanflem marked this pull request as ready for review December 8, 2025 17:24
@github-actions github-actions Bot temporarily deployed to Preview - journeys-admin December 8, 2025 17:26 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - journeys December 8, 2025 17:26 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - resources December 8, 2025 17:26 Inactive
@github-actions github-actions Bot temporarily deployed to Preview - videos-admin December 30, 2025 18:27 Inactive
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (9)
apis/api-gateway/schema.graphql (5)

516-517: Add descriptions for the new Algolia index mutations.

These mutations lack descriptions explaining their purpose and behavior. GraphQL documentation is essential for API consumers.

📝 Suggested documentation
+ """
+ Triggers a re-index of the video's metadata in Algolia
+ """
  updateVideoAlgoliaIndex(videoId: ID!) : Boolean! @join__field(graph: API_MEDIA) 
+ """
+ Triggers a re-index of all video variants for the specified video in Algolia
+ """
  updateVideoVariantAlgoliaIndex(videoId: ID!) : Boolean! @join__field(graph: API_MEDIA)

928-929: Add descriptions for the new Algolia verification queries.

These queries lack descriptions explaining what they check and what results they return. This makes the API harder to discover and use correctly.

📝 Suggested documentation
+ """
+ Verifies that the video's metadata in Algolia matches the database and returns any mismatches
+ """
  checkVideoInAlgolia(videoId: ID!) : CheckVideoInAlgoliaResult! @join__field(graph: API_MEDIA) 
+ """
+ Verifies that all expected video variants are indexed in Algolia and returns any missing variants
+ """
  checkVideoVariantsInAlgolia(videoId: ID!) : CheckVideoVariantsInAlgoliaResult! @join__field(graph: API_MEDIA)

2640-2644: Add type and field descriptions for CheckVideoInAlgoliaMismatch.

The type and its fields lack descriptions, making it unclear what each field represents.

📝 Suggested documentation
+ """
+ Represents a field mismatch between the database and Algolia index
+ """
  type CheckVideoInAlgoliaMismatch @join__type(graph: API_MEDIA)  {
+   """
+   The name of the field that has a mismatch
+   """
    field: String
+   """
+   The expected value from the database
+   """
    expected: String
+   """
+   The actual value found in Algolia
+   """
    actual: String
  }

2646-2650: Consider making the ok field non-null and add type descriptions.

The ok field is a status indicator that should always have a value. Currently it's nullable (Boolean) but should likely be Boolean!.

🔧 Suggested changes
+ """
+ Result of checking if a video's metadata in Algolia matches the database
+ """
  type CheckVideoInAlgoliaResult @join__type(graph: API_MEDIA)  {
+   """
+   True if the video data in Algolia matches the database, false otherwise
+   """
-   ok: Boolean
+   ok: Boolean!
+   """
+   List of field mismatches found between the database and Algolia
+   """
    mismatches: [CheckVideoInAlgoliaMismatch!]
+   """
+   Direct URL to view the video record in the Algolia dashboard
+   """
    recordUrl: String
  }

2652-2656: Consider making the ok field non-null, add descriptions, and align URL field naming.

Multiple improvements could be made:

  1. The ok field should be non-null (Boolean!) as a status indicator
  2. Missing type and field descriptions
  3. Naming inconsistency: browseUrl here vs recordUrl in CheckVideoInAlgoliaResult - both point to Algolia dashboard
🔧 Suggested changes
+ """
+ Result of checking if all expected video variants are indexed in Algolia
+ """
  type CheckVideoVariantsInAlgoliaResult @join__type(graph: API_MEDIA)  {
+   """
+   True if all expected video variants are present in Algolia, false otherwise
+   """
-   ok: Boolean
+   ok: Boolean!
+   """
+   List of variant identifiers that are missing from the Algolia index
+   """
    missingVariants: [String!]
+   """
+   URL to browse video variants in the Algolia dashboard
+   """
    browseUrl: String
  }

Consider renaming browseUrl to recordUrl for consistency with CheckVideoInAlgoliaResult, unless there's a semantic difference between browsing and viewing records in Algolia.

apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (4)

205-221: Sequential Algolia API calls can be slow for videos with many variants.

Each variant is checked sequentially in a for loop, which could result in significant latency when a video has dozens of variants. Consider using Promise.all or Promise.allSettled to fetch all records in parallel.

🔎 Proposed refactor
-      for (const variant of variants) {
-        try {
-          const record = await client.getObject({
-            indexName: videoVariantsIndex,
-            objectID: variant.id
-          })
-          const objectIdMatches = record.objectID === variant.id
-          const videoIdMatches =
-            record.videoId == null || record.videoId === videoId
-
-          if (!(objectIdMatches && videoIdMatches)) {
-            missingVariants.push(variant.id)
-          }
-        } catch {
-          missingVariants.push(variant.id)
-        }
-      }
+      const results = await Promise.allSettled(
+        variants.map(async (variant) => {
+          const record = await client.getObject({
+            indexName: videoVariantsIndex,
+            objectID: variant.id
+          })
+          const objectIdMatches = record.objectID === variant.id
+          const videoIdMatches =
+            record.videoId == null || record.videoId === videoId
+          
+          return objectIdMatches && videoIdMatches ? null : variant.id
+        })
+      )
+      
+      for (const result of results) {
+        const variantId = variants[results.indexOf(result)].id
+        if (result.status === 'rejected' || result.value != null) {
+          missingVariants.push(result.status === 'rejected' ? variantId : result.value)
+        }
+      }

213-213: Use strict equality for videoId check.

The loose equality check record.videoId == null will match both null and undefined. If this is intentional, consider using record.videoId == null with a comment explaining why. Otherwise, use strict equality record.videoId === null || record.videoId === undefined.

🔎 Proposed refactor
-          const videoIdMatches =
-            record.videoId == null || record.videoId === videoId
+          const videoIdMatches =
+            (record.videoId === null || record.videoId === undefined) || record.videoId === videoId

Or if loose equality is intentional, add a clarifying comment:

           const videoIdMatches =
+            // Allow null or undefined videoId for backwards compatibility
             record.videoId == null || record.videoId === videoId

163-166: Extract duplicate Algolia URL construction to a helper function.

The same URL construction pattern appears three times. Extracting it to a helper improves maintainability and reduces duplication.

🔎 Proposed refactor

Add this helper function near the top of the file (after imports):

function buildAlgoliaRecordUrl(
  appId: string,
  indexName: string,
  query: string
): string | null {
  if (appId === '' || indexName === '') {
    return null
  }
  return `https://www.algolia.com/apps/${appId}/explorer/browse/${indexName}?query=${encodeURIComponent(query)}`
}

Then replace all three occurrences:

         return {
           ok: mismatches.length === 0,
           mismatches,
-          recordUrl:
-            appId !== '' && videosIndex !== ''
-              ? `https://www.algolia.com/apps/${appId}/explorer/browse/${videosIndex}?query=${encodeURIComponent(
-                  videoId
-                )}`
-              : null
+          recordUrl: buildAlgoliaRecordUrl(appId, videosIndex, videoId)
         }

Also applies to: 173-176, 228-231


101-106: Simplify nested ternary for better readability.

The nested ternary for isDownloadable can be made more readable with an early return pattern or explicit if-else logic.

🔎 Proposed refactor
       const primaryVariant = video.variants[0]
       const isVideoContent = primaryVariant?.hls != null
-      const isDownloadable =
-        video.label === 'collection' || video.label === 'series'
-          ? false
-          : (primaryVariant?.downloadable ?? false)
+      
+      let isDownloadable = false
+      if (video.label !== 'collection' && video.label !== 'series') {
+        isDownloadable = primaryVariant?.downloadable ?? false
+      }
+      
       const expected = {
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 02c9f13 and 7e0d1e8.

⛔ Files ignored due to path filters (1)
  • libs/shared/gql/src/__generated__/graphql-env.d.ts is excluded by !**/__generated__/**
📒 Files selected for processing (5)
  • apis/api-gateway/schema.graphql
  • apis/api-media/schema.graphql
  • apis/api-media/src/schema/video/index.ts
  • apis/api-media/src/schema/video/videoAlgolia/index.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apis/api-media/schema.graphql
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.

Files:

  • apis/api-media/src/schema/video/videoAlgolia/index.ts
  • apis/api-media/src/schema/video/index.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

Define a type if possible.

Files:

  • apis/api-media/src/schema/video/videoAlgolia/index.ts
  • apis/api-media/src/schema/video/index.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
🧠 Learnings (10)
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/index.ts
  • apis/api-media/src/schema/video/index.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Use alphabetical order for imports and exports.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/index.ts
📚 Learning: 2025-07-22T18:37:24.555Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: .cursor/rules/base.mdc:0-0
Timestamp: 2025-07-22T18:37:24.555Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Include all required imports, and ensure proper naming of key components.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/index.ts
  • apis/api-media/src/schema/video/index.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Export all components through index.ts files.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/index.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Apollo hooks should use generated types alongside colocated documents. `useVideoChildren` (`apps/resources/src/libs/useVideoChildren/useVideoChildren.ts`) is the reference pattern

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/index.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-08-12T00:53:19.148Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7345
File: apis/api-journeys-modern/src/schema/block/inputs/blocksFilter.ts:3-8
Timestamp: 2025-08-12T00:53:19.148Z
Learning: The JesusFilm/core project has a policy of not updating existing GraphQL field names, even for consistency improvements, to maintain API stability and backward compatibility.

Applied to files:

  • apis/api-gateway/schema.graphql
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.graphql : After editing GraphQL documents, run `pnpm dlx nx run watch:codegen` to regenerate artifacts in `apps/watch/__generated__` and stage those artifacts with changes.

Applied to files:

  • apis/api-gateway/schema.graphql
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact

Applied to files:

  • apis/api-media/src/schema/video/index.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Apollo hooks should use generated types alongside colocated documents. Reference the pattern of `useVideoChildren` (`apps/watch/src/libs/useVideoChildren/useVideoChildren.ts`) for data fetching.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-08-18T17:23:02.876Z
Learnt from: tanflem
Repo: JesusFilm/core PR: 7464
File: apis/api-media/src/workers/processVideoUploads/service/service.ts:300-303
Timestamp: 2025-08-18T17:23:02.876Z
Learning: In the Video GraphQL schema, the slug field is defined as nullable: false with a resolver that returns slug ?? '', meaning it will never be null but could be an empty string if the underlying data is null/undefined.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
🧬 Code graph analysis (1)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (1)
apis/api-media/src/lib/algolia/algoliaClient.ts (1)
  • getAlgoliaClient (4-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: Deploy Preview (videos-admin, 8463/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (short-links, 8463/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (watch, 8463/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (journeys-admin, 8463/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (player, 8463/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (resources, 8463/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (journeys, 8463/merge, pull_request, 22)
  • GitHub Check: test (22, 2/3)
  • GitHub Check: build (22)
  • GitHub Check: test (22, 3/3)
  • GitHub Check: test (22, 1/3)
  • GitHub Check: lint (22)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apis/api-media/src/schema/video/index.ts (1)

4-4: LGTM!

The import follows the existing pattern and is correctly positioned in alphabetical order.

apis/api-media/src/schema/video/videoAlgolia/index.ts (1)

1-1: LGTM!

Standard barrel export pattern for the videoAlgolia module.

Comment thread apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts Outdated
Comment thread apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts Outdated
Comment thread apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
Comment thread apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts Outdated
…-backend-video-managment-algolia-troubleshooting
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apis/api-gateway/schema.graphql (1)

2645-2661: Consider making ok fields non-nullable for clearer API semantics.

The ok: Boolean fields in both result types are nullable, which could lead to ambiguous responses. For a troubleshooting check, a definitive true/false result is typically expected.

Similarly, field in CheckVideoInAlgoliaMismatch being nullable means a mismatch record might not identify which field mismatched.

♻️ Suggested type definitions
 type CheckVideoInAlgoliaMismatch @join__type(graph: API_MEDIA)  {
-  field: String
+  field: String!
   expected: String
   actual: String
 }

 type CheckVideoInAlgoliaResult @join__type(graph: API_MEDIA)  {
-  ok: Boolean
+  ok: Boolean!
   mismatches: [CheckVideoInAlgoliaMismatch!]
   recordUrl: String
 }

 type CheckVideoVariantsInAlgoliaResult @join__type(graph: API_MEDIA)  {
-  ok: Boolean
+  ok: Boolean!
   missingVariants: [String!]
   browseUrl: String
 }

Please verify the intended nullability behavior in the underlying resolver implementation in apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts and update the schema accordingly if the fields should always be present.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7e0d1e8 and 3bd0f0d.

📒 Files selected for processing (1)
  • apis/api-gateway/schema.graphql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: test (22, 2/3)
  • GitHub Check: test (22, 1/3)
  • GitHub Check: test (22, 3/3)
  • GitHub Check: build (22)
  • GitHub Check: lint (22)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apis/api-gateway/schema.graphql (2)

516-517: LGTM!

The new Algolia index update mutations are well-defined with appropriate non-nullable return types and proper graph scoping to API_MEDIA.


928-929: LGTM!

The new Algolia check queries are appropriately defined with non-nullable result types and proper graph scoping.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 8, 2026

Warnings
⚠️ ❗ Big PR (1109 changes)

(change count - 1109): Pull Request size seems relatively large. If Pull Request contains multiple changes, split each into separate PR will helps faster, easier review.

Generated by 🚫 dangerJS against 966153a

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (2)

186-202: Consider concurrency for variant checks with many variants.

Sequential getObject calls could be slow for videos with many language variants. For troubleshooting purposes this may be acceptable, but if performance becomes an issue, consider using Promise.allSettled with concurrency limiting.

♻️ Optional: Concurrent variant checks
const results = await Promise.allSettled(
  variants.map(async (variant) => {
    const record = await client.getObject({
      indexName: algoliaConfig.videoVariantsIndex,
      objectID: variant.id
    })
    const objectIdMatches = record.objectID === variant.id
    const videoIdMatches = record.videoId == null || record.videoId === videoId
    return { id: variant.id, ok: objectIdMatches && videoIdMatches }
  })
)

const missingVariants = results
  .map((result, i) =>
    result.status === 'rejected' || !result.value.ok ? variants[i].id : null
  )
  .filter(Boolean) as string[]

251-256: Consider error handling for partial update failures.

Promise.all will reject on the first error, leaving remaining variants unprocessed. Since updateVideoVariantInAlgolia has internal try/catch, this should be safe, but Promise.allSettled would ensure all variants are attempted even if one fails unexpectedly.

♻️ Optional: Use Promise.allSettled for resilience
-      await Promise.all(
-        variants.map(async (variant) => {
-          await updateVideoVariantInAlgolia(variant.id)
-        })
-      )
+      const results = await Promise.allSettled(
+        variants.map((variant) => updateVideoVariantInAlgolia(variant.id))
+      )
+      const failures = results.filter((r) => r.status === 'rejected')
+      if (failures.length > 0) {
+        throw new GraphQLError(
+          `Failed to update ${failures.length} of ${variants.length} variants`
+        )
+      }
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3bd0f0d and 9988d6b.

⛔ Files ignored due to path filters (1)
  • apps/journeys-admin/__generated__/JourneyVisitorExportToGoogleSheet.ts is excluded by !**/__generated__/**
📒 Files selected for processing (7)
  • apis/api-media/src/lib/algolia/algoliaClient.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.

Files:

  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts
  • apis/api-media/src/lib/algolia/algoliaClient.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

Define a type if possible.

Files:

  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts
  • apis/api-media/src/lib/algolia/algoliaClient.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
🧠 Learnings (22)
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.

Applied to files:

  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
📚 Learning: 2025-03-02T23:04:09.558Z
Learnt from: tataihono
Repo: JesusFilm/core PR: 5390
File: apis/api-media/src/workers/bigQuery/importers/shortLinks/shortLinks.spec.ts:1-1
Timestamp: 2025-03-02T23:04:09.558Z
Learning: Always ignore ESLint warnings (import/no-useless-path-segments) for Prisma client imports using the format `.prisma/api-media-client` instead of `./.prisma/api-media-client`. This is an intentional pattern in the codebase.

Applied to files:

  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts
📚 Learning: 2025-03-02T23:04:53.192Z
Learnt from: tataihono
Repo: JesusFilm/core PR: 5390
File: apis/api-media/src/workers/bigQuery/importers/shortLinks/shortLinks.ts:4-4
Timestamp: 2025-03-02T23:04:53.192Z
Learning: Always ignore ESLint warnings about Prisma client import paths. The import path `.prisma/api-media-client` should not be changed to `./.prisma/api-media-client` despite ESLint flagging it as an error with the rule "import/no-useless-path-segments".

Applied to files:

  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Apollo hooks should use generated types alongside colocated documents. Reference the pattern of `useVideoChildren` (`apps/watch/src/libs/useVideoChildren/useVideoChildren.ts`) for data fetching.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Apollo hooks should use generated types alongside colocated documents. `useVideoChildren` (`apps/resources/src/libs/useVideoChildren/useVideoChildren.ts`) is the reference pattern

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Keep handlers prefixed with `handle*`, type everything explicitly, and lean on generated GraphQL helpers (`ResultOf`, `VariablesOf`).

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-11-11T23:22:02.196Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 8156
File: apis/api-journeys-modern/src/lib/google/googleAuth.ts:0-0
Timestamp: 2025-11-11T23:22:02.196Z
Learning: In apis/api-journeys-modern, use the validated `env` object from `../../env` instead of accessing `process.env` directly for environment variables that are defined in env.ts (e.g., GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, INTEGRATION_ACCESS_KEY_ENCRYPTION_SECRET). This eliminates the need for runtime validation checks since Zod validates them at application startup.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-04-23T18:26:44.096Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 6358
File: apis/api-journeys/db/seeds/playwrightUserAccess.ts:16-53
Timestamp: 2025-04-23T18:26:44.096Z
Learning: For seeding scripts in the JesusFilm/core repository, environment variables should be validated and the code should throw an error if required variables are missing rather than silently continuing with fallbacks or filtering.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : REST-like endpoints should use SWR hooks paired with zod guards for response validation; parse responses before returning them to callers.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-05-22T02:57:46.179Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 6734
File: apps/journeys-admin/src/components/TermsAndConditions/TermsAndConditions.tsx:58-71
Timestamp: 2025-05-22T02:57:46.179Z
Learning: The team prefers to avoid try/catch blocks when working with Apollo Client mutations in favor of Apollo's built-in error handling mechanisms (checking the returned errors property).

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-07-10T22:50:12.252Z
Learnt from: tataihono
Repo: JesusFilm/core PR: 7153
File: apis/api-journeys-modern/src/schema/journey/simple/getSimpleJourney.ts:13-21
Timestamp: 2025-07-10T22:50:12.252Z
Learning: Prisma's `findUnique` method returns `null` when no record is found, rather than throwing an error. The "not found" case should be handled by checking for null, not with try-catch blocks.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-09-29T23:03:36.840Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7629
File: apis/api-journeys-modern/src/schema/event/utils.ts:43-60
Timestamp: 2025-09-29T23:03:36.840Z
Learning: In the JesusFilm/core repository, do not recommend using Prisma's `upsert` operation for `JourneyVisitor` creation in `apis/api-journeys-modern/src/schema/event/utils.ts` as it is not race condition safe for this use case. The current `findUnique` then `create` pattern is the preferred approach.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Wrap component specs with `MockedProvider`, `VideoProvider`, and `WatchProvider` when the unit touches those contexts—`NewVideoContentPage.spec.tsx` shows the expected harness

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Enclose SWR-based hooks in `TestSWRConfig` (`apps/watch/test/TestSWRConfig.tsx`) to isolate cache state between assertions in tests.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
📚 Learning: 2025-04-08T07:57:39.246Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 6209
File: apps/watch/src/components/CollectionsPage/ContainerHeroVideo/ContainerHeroVideo.tsx:60-63
Timestamp: 2025-04-08T07:57:39.246Z
Learning: When adding new method calls to code that uses video.js player (or any external library), the corresponding test mocks must be updated to include those methods. In this specific case, adding `player.src()` to production code required adding a mock implementation of the `src` method in test files to prevent "TypeError: player.src is not a function" errors.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/watch/test`.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Enclose SWR-based hooks in `TestSWRConfig` (`apps/resources/test/TestSWRConfig.tsx`) to isolate cache state between assertions

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Use the shared Jest setup in `apps/watch/setupTests.tsx` which already boots MSW, Next router mock, and has a longer async timeout.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/resources/test`

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : UI flows must sit inside `VideoProvider`, `WatchProvider`, and `PlayerProvider`. Mirror that wiring when composing features and when writing tests.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-11-16T21:30:53.412Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 8309
File: apps/journeys-admin/setupTests.tsx:47-51
Timestamp: 2025-11-16T21:30:53.412Z
Learning: In apps/journeys-admin/setupTests.tsx, the `document.clearImmediate` mock is required for tests involving the mux upload provider, as an underlying library uses clearImmediate for timeout management to prevent race conditions and unnecessary polling calls.

Applied to files:

  • apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts
  • apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts
🧬 Code graph analysis (3)
apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts (1)
apis/api-media/src/lib/algolia/algoliaClient.ts (2)
  • getAlgoliaClient (18-20)
  • algoliaConfig (11-16)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (3)
apis/api-media/src/lib/algolia/algoliaClient.ts (2)
  • getAlgoliaClient (18-20)
  • algoliaConfig (11-16)
apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts (1)
  • updateVideoInAlgolia (8-227)
apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts (1)
  • updateVideoVariantInAlgolia (18-129)
apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts (1)
apis/api-media/src/lib/algolia/algoliaClient.ts (2)
  • getAlgoliaClient (18-20)
  • algoliaConfig (11-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: affected (22)
  • GitHub Check: test (22, 1/3)
  • GitHub Check: build (22)
  • GitHub Check: test (22, 2/3)
  • GitHub Check: test (22, 3/3)
  • GitHub Check: lint (22)
🔇 Additional comments (12)
apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts (1)

5-5: LGTM!

The refactoring to use the centralized algoliaConfig and synchronous getAlgoliaClient() is clean and consistent with the new client architecture. The function correctly uses algoliaConfig.videosIndex for all index references.

Also applies to: 12-12, 213-213, 221-223

apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.spec.ts (1)

7-20: LGTM!

The test mock setup correctly mirrors the new centralized Algolia client structure. The saveObjectsSpy at module scope with the factory mock pattern is clean and allows proper test isolation with jest.clearAllMocks() in beforeEach.

apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts (1)

5-5: LGTM!

The refactoring to use centralized algoliaConfig and synchronous getAlgoliaClient() is consistent with the video update module. The variant-specific behavior (delete on not-found, skip unpublished, remove watch-restricted) is preserved correctly.

Also applies to: 23-23, 45-45, 64-64, 112-112, 121-121

apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.spec.ts (1)

7-22: LGTM!

The mock setup correctly includes both deleteObject and saveObjects spies, matching the production code's usage. Test coverage comprehensively validates all variant update paths including deletion scenarios.

apis/api-media/src/lib/algolia/algoliaClient.ts (2)

3-9: Clean fail-fast pattern for required configuration.

The getRequiredEnv helper correctly throws on missing or empty values, ensuring configuration errors surface immediately at startup rather than at runtime.


11-16: Verify test setup handles eager module evaluation.

algoliaConfig is evaluated at module load time, which means all four environment variables must be set (or the module mocked) before any import. The test files correctly use jest.mock() with factory functions to avoid this, so this is well-handled.

apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts (4)

1-61: LGTM!

The test setup is well-structured with proper mocking of the Algolia client, update functions, and authorization context. The authClient configuration correctly simulates a publisher role for authorization tests.


62-330: Comprehensive query test coverage.

Tests thoroughly validate:

  • Field-level mismatch detection with expected/actual values
  • Container video handling (collection/series forced non-downloadable)
  • Arclight restriction propagation
  • Error paths for missing videos and Algolia failures

332-440: Good coverage for variant checks.

The checkVideoVariantsInAlgolia tests properly validate detection of missing variants and videoId mismatches using sequential mock responses.


443-569: Mutation tests validate update flows and authorization.

Tests confirm that update mutations call the underlying Algolia update functions correctly and enforce publisher authorization.

apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (2)

13-60: LGTM!

The GraphQL type definitions are clean, with proper nullability annotations and field exposure. The mismatch structure captures field-level differences effectively for debugging.


62-167: LGTM on video check implementation.

The checkVideoInAlgolia query correctly:

  • Validates video existence before comparison
  • Mirrors the isDownloadable logic for collections/series
  • Uses JSON.stringify for deep comparison of complex fields like keywords
  • Returns a helpful dashboard URL for manual inspection

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts (2)

31-36: Consider using ES module imports instead of require().

The pattern of using require() to access mocked functions works but mixes CommonJS and ES modules. For consistency with TypeScript conventions, consider importing these at the top level.

♻️ Proposed refactor to use ES module imports

Move the imports to the top of the file and cast them appropriately:

+import { updateVideoInAlgolia } from '../../../lib/algolia/algoliaVideoUpdate'
+import { updateVideoVariantInAlgolia } from '../../../lib/algolia/algoliaVideoVariantUpdate'
+
 import { graphql } from '@core/shared/gql'

 import { getClient } from '../../../../test/client'
 import { prismaMock } from '../../../../test/prismaMock'

 const getObjectSpy = jest.fn()

 // Mock the algolia client helper
 jest.mock('../../../lib/algolia/algoliaClient', () => ({
   getAlgoliaClient: () => ({
     getObject: getObjectSpy
   }),
   algoliaConfig: {
     appId: 'test-app-id',
     apiKey: 'test-api-key',
     videosIndex: 'test-videos-index',
     videoVariantsIndex: 'test-video-variants-index'
   }
 }))

 // Mock the algolia update functions
 jest.mock('../../../lib/algolia/algoliaVideoUpdate', () => ({
   updateVideoInAlgolia: jest.fn()
 }))

 jest.mock('../../../lib/algolia/algoliaVideoVariantUpdate', () => ({
   updateVideoVariantInAlgolia: jest.fn()
 }))

-// Get the mocked functions
-const {
-  updateVideoInAlgolia
-} = require('../../../lib/algolia/algoliaVideoUpdate')
-const {
-  updateVideoVariantInAlgolia
-} = require('../../../lib/algolia/algoliaVideoVariantUpdate')
+// Cast to mocked functions for type safety
+const mockedUpdateVideoInAlgolia = updateVideoInAlgolia as jest.MockedFunction<typeof updateVideoInAlgolia>
+const mockedUpdateVideoVariantInAlgolia = updateVideoVariantInAlgolia as jest.MockedFunction<typeof updateVideoVariantInAlgolia>

Then update references throughout the file to use the typed mocked versions.


50-60: Simplify mock management in test hooks.

Using both jest.clearAllMocks() in beforeEach (line 51) and jest.resetAllMocks() in afterEach (line 59) is redundant. The resetAllMocks() call already clears mock history and resets implementations, making clearAllMocks() unnecessary.

♻️ Proposed simplification
 beforeEach(() => {
-  jest.clearAllMocks()
-
   getObjectSpy.mockReset()
   updateVideoInAlgolia.mockResolvedValue(undefined)
   updateVideoVariantInAlgolia.mockResolvedValue(undefined)
 })

 afterEach(() => {
   jest.resetAllMocks()
 })
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9988d6b and 56b4b1a.

📒 Files selected for processing (1)
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.

Files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

Define a type if possible.

Files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
🧠 Learnings (10)
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Wrap component specs with `MockedProvider`, `VideoProvider`, and `WatchProvider` when the unit touches those contexts—`NewVideoContentPage.spec.tsx` shows the expected harness

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/watch/test`.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Enclose SWR-based hooks in `TestSWRConfig` (`apps/watch/test/TestSWRConfig.tsx`) to isolate cache state between assertions in tests.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-04-08T07:57:39.246Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 6209
File: apps/watch/src/components/CollectionsPage/ContainerHeroVideo/ContainerHeroVideo.tsx:60-63
Timestamp: 2025-04-08T07:57:39.246Z
Learning: When adding new method calls to code that uses video.js player (or any external library), the corresponding test mocks must be updated to include those methods. In this specific case, adding `player.src()` to production code required adding a mock implementation of the `src` method in test files to prevent "TypeError: player.src is not a function" errors.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Enclose SWR-based hooks in `TestSWRConfig` (`apps/resources/test/TestSWRConfig.tsx`) to isolate cache state between assertions

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Use the shared Jest setup in `apps/watch/setupTests.tsx` which already boots MSW, Next router mock, and has a longer async timeout.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/resources/test`

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : UI flows must sit inside `VideoProvider`, `WatchProvider`, and `PlayerProvider`. Mirror that wiring when composing features and when writing tests.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts
🧬 Code graph analysis (1)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts (2)
apis/api-media/src/lib/algolia/algoliaVideoUpdate/algoliaVideoUpdate.ts (1)
  • updateVideoInAlgolia (8-227)
apis/api-media/src/lib/algolia/algoliaVideoVariantUpdate/algoliaVideoVariantUpdate.ts (1)
  • updateVideoVariantInAlgolia (18-129)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: lint (22)
  • GitHub Check: test (22, 1/3)
  • GitHub Check: test (22, 2/3)
  • GitHub Check: build (22)
  • GitHub Check: test (22, 3/3)
🔇 Additional comments (5)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.spec.ts (5)

1-4: LGTM!

Imports are clean and follow project conventions for test utilities.


63-328: LGTM! Comprehensive test coverage for checkVideoInAlgolia.

The test suite thoroughly covers:

  • Happy path with matching data
  • Mismatch detection for multiple fields
  • Special handling for container videos (collections/series)
  • Arclight restrictions
  • Error cases (video not found, Algolia failures)
  • Authorization checks

All test cases are well-structured with appropriate mocks and assertions.


330-438: LGTM! Solid test coverage for checkVideoVariantsInAlgolia.

The test suite effectively validates:

  • Successful variant existence checks
  • Missing variant detection
  • Mismatched videoId identification
  • Publisher authorization requirements

The use of mock chaining (lines 353-355, 383-386) appropriately simulates multiple Algolia lookups.


442-499: LGTM! Complete test coverage for updateVideoAlgoliaIndex mutation.

The test suite properly validates:

  • Successful video updates with correct function calls
  • Error handling when video doesn't exist
  • Prevention of updates without proper authorization

Assertions correctly verify both GraphQL responses and that the underlying updateVideoInAlgolia function is (or isn't) called appropriately.


501-567: LGTM! Thorough test coverage for updateVideoVariantAlgoliaIndex mutation.

The test suite validates:

  • Batch updates of multiple variants (lines 526-529 properly verify all three variants are updated)
  • Error handling when no variants exist
  • Authorization requirements

The assertions effectively verify both successful batch operations and proper error handling.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts:
- Line 107: Replace the hardcoded languageId '529' used in the variant query
with the video's actual primary language by using video.primaryLanguageId (e.g.,
change where: { languageId: '529' } to where: { languageId:
video.primaryLanguageId }); ensure you handle any type mismatches (string vs
number) and that video.primaryLanguageId is defined before using it so the
variant fetch in videoAlgolia correctly returns the video's primary variant.
- Around line 107-122: The access of video.variants[0] (primaryVariant) can be
undefined; update the logic around primaryVariant in the resolver that computes
isVideoContent and isDownloadable to guard against that: retrieve primaryVariant
via video.variants[0] and then either (A) throw a GraphQLError if a primary
variant is required, or (B) treat a missing variant as
non-video/non-downloadable by setting isVideoContent =
Boolean(primaryVariant?.hls) and isDownloadable = primaryVariant ? (video.label
=== 'collection' || video.label === 'series' ? false :
(primaryVariant.downloadable ?? false)) : false; ensure all uses of
primaryVariant use optional chaining or null checks to avoid undefined access.
🧹 Nitpick comments (3)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (3)

186-195: Simplify redundant logging.

The code logs to both logger and console.error when logger is null. Consider simplifying this pattern since the logger typically handles both cases internally.

♻️ Proposed refactor
-        logger?.error(
+        const logMsg = 'Algolia getObject failed while checking video in Algolia'
+        const logContext = { err, ...context }
+        
+        if (logger != null) {
+          logger.error(logContext, logMsg)
+        } else {
+          console.error(
-          { err, ...context },
-          'Algolia getObject failed while checking video in Algolia'
-        )
-
-        if (logger == null) {
-          console.error(
-            `Algolia getObject failed while checking video in Algolia (videoId=${videoId}, appId=${appId}, videosIndex=${videosIndex}): ${errorString}`
+            `${logMsg} (videoId=${videoId}, appId=${appId}, videosIndex=${videosIndex}): ${errorString}`,
+            logContext
           )
         }

223-239: Consider logging errors in the catch block.

The catch block at line 236 silently adds variants to missingVariants without logging the underlying error. This makes it difficult to distinguish between truly missing variants and those that failed due to other errors (network issues, permission problems, etc.).

📝 Suggested enhancement
         } catch {
+          logger?.warn(
+            { variantId: variant.id, videoId },
+            'Failed to fetch variant from Algolia'
+          )
           missingVariants.push(variant.id)
         }

Or capture the error to include in the response:

-        } catch {
+        } catch (err) {
+          logger?.warn(
+            { err, variantId: variant.id, videoId },
+            'Failed to fetch variant from Algolia'
+          )
           missingVariants.push(variant.id)
         }

229-231: Use consistent null checking.

Line 231 uses loose equality (== null) for the first check but strict equality (=== videoId) for the second. While == null correctly checks for both null and undefined, using strict equality throughout is more consistent with modern JavaScript/TypeScript conventions.

♻️ Proposed refactor
           const objectIdMatches = record.objectID === variant.id
           const videoIdMatches =
-            record.videoId == null || record.videoId === videoId
+            (record.videoId === null || record.videoId === undefined) || record.videoId === videoId

Or use optional chaining if appropriate:

           const videoIdMatches =
-            record.videoId == null || record.videoId === videoId
+            !record.videoId || record.videoId === videoId
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 56b4b1a and d4bef18.

📒 Files selected for processing (2)
  • apis/api-media/schema.graphql
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.

Files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

Define a type if possible.

Files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
🧠 Learnings (9)
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Apollo hooks should use generated types alongside colocated documents. Reference the pattern of `useVideoChildren` (`apps/watch/src/libs/useVideoChildren/useVideoChildren.ts`) for data fetching.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-11-11T23:22:02.196Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 8156
File: apis/api-journeys-modern/src/lib/google/googleAuth.ts:0-0
Timestamp: 2025-11-11T23:22:02.196Z
Learning: In apis/api-journeys-modern, use the validated `env` object from `../../env` instead of accessing `process.env` directly for environment variables that are defined in env.ts (e.g., GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, INTEGRATION_ACCESS_KEY_ENCRYPTION_SECRET). This eliminates the need for runtime validation checks since Zod validates them at application startup.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-04-23T18:26:44.096Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 6358
File: apis/api-journeys/db/seeds/playwrightUserAccess.ts:16-53
Timestamp: 2025-04-23T18:26:44.096Z
Learning: For seeding scripts in the JesusFilm/core repository, environment variables should be validated and the code should throw an error if required variables are missing rather than silently continuing with fallbacks or filtering.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : REST-like endpoints should use SWR hooks paired with zod guards for response validation; parse responses before returning them to callers.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-05-22T02:57:46.179Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 6734
File: apps/journeys-admin/src/components/TermsAndConditions/TermsAndConditions.tsx:58-71
Timestamp: 2025-05-22T02:57:46.179Z
Learning: The team prefers to avoid try/catch blocks when working with Apollo Client mutations in favor of Apollo's built-in error handling mechanisms (checking the returned errors property).

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-07-10T22:50:12.252Z
Learnt from: tataihono
Repo: JesusFilm/core PR: 7153
File: apis/api-journeys-modern/src/schema/journey/simple/getSimpleJourney.ts:13-21
Timestamp: 2025-07-10T22:50:12.252Z
Learning: Prisma's `findUnique` method returns `null` when no record is found, rather than throwing an error. The "not found" case should be handled by checking for null, not with try-catch blocks.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-09-29T23:03:36.840Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7629
File: apis/api-journeys-modern/src/schema/event/utils.ts:43-60
Timestamp: 2025-09-29T23:03:36.840Z
Learning: In the JesusFilm/core repository, do not recommend using Prisma's `upsert` operation for `JourneyVisitor` creation in `apis/api-journeys-modern/src/schema/event/utils.ts` as it is not race condition safe for this use case. The current `findUnique` then `create` pattern is the preferred approach.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-08-18T17:23:02.876Z
Learnt from: tanflem
Repo: JesusFilm/core PR: 7464
File: apis/api-media/src/workers/processVideoUploads/service/service.ts:300-303
Timestamp: 2025-08-18T17:23:02.876Z
Learning: In the Video GraphQL schema, the slug field is defined as nullable: false with a resolver that returns slug ?? '', meaning it will never be null but could be an empty string if the underlying data is null/undefined.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: affected (22)
  • GitHub Check: lint (22)
  • GitHub Check: test (22, 3/3)
  • GitHub Check: test (22, 1/3)
  • GitHub Check: test (22, 2/3)
  • GitHub Check: build (22)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (4)

14-28: LGTM!

The error handling utility is well-structured with appropriate fallbacks for different error types.


30-79: LGTM!

The GraphQL type definitions are properly structured and align with the schema changes.


250-270: LGTM!

The mutation properly validates video existence before triggering the Algolia update.


271-330: Well-implemented batch update with comprehensive error handling.

The use of Promise.allSettled ensures all variants are attempted even if some fail, and the error aggregation provides clear feedback about which variants failed. The error message preview limiting to 10 variants is a nice touch for readability.

apis/api-media/schema.graphql (2)

49-66: LGTM!

The new GraphQL types for Algolia validation are well-structured and properly aligned with the TypeScript implementation. The nullability annotations correctly reflect the implementation's behavior.


356-357: LGTM!

The new mutations and queries provide a clean API for Algolia troubleshooting. The required videoId argument and non-null return types are appropriate for these operations.

Also applies to: 749-750

Comment thread apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
Comment thread apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (4)

107-107: Clarify the hardcoded languageId.

The hardcoded '529' for primary variant selection appears intentional but lacks context. Is this the English language ID? Consider adding a comment or extracting to a named constant.

♻️ Suggested refactor
+// Language ID for English (primary variant)
+const PRIMARY_VARIANT_LANGUAGE_ID = '529'
+
 builder.queryFields((t) => ({
   checkVideoInAlgolia: t.withAuth({ isPublisher: true }).field({
     ...
           variants: {
             select: {
               hls: true,
               lengthInMilliseconds: true,
               downloadable: true
             },
-            where: { languageId: '529' },
+            where: { languageId: PRIMARY_VARIANT_LANGUAGE_ID },
             take: 1
           }

223-239: Performance: Parallelize variant checks.

Sequential Algolia API calls could be slow for videos with many variants. The updateVideoVariantAlgoliaIndex mutation uses Promise.allSettled for parallel execution—consider applying the same pattern here.

♻️ Proposed refactor using Promise.allSettled
-      for (const variant of variants) {
-        try {
-          const record = await client.getObject({
-            indexName: algoliaConfig.videoVariantsIndex,
-            objectID: variant.id
-          })
-          const objectIdMatches = record.objectID === variant.id
-          const videoIdMatches =
-            record.videoId == null || record.videoId === videoId
-
-          if (!(objectIdMatches && videoIdMatches)) {
-            missingVariants.push(variant.id)
-          }
-        } catch {
-          missingVariants.push(variant.id)
-        }
-      }
+      const results = await Promise.allSettled(
+        variants.map(async (variant) => {
+          const record = await client.getObject({
+            indexName: algoliaConfig.videoVariantsIndex,
+            objectID: variant.id
+          })
+          const objectIdMatches = record.objectID === variant.id
+          const videoIdMatches =
+            record.videoId == null || record.videoId === videoId
+
+          if (!(objectIdMatches && videoIdMatches)) {
+            return { missing: true, variantId: variant.id }
+          }
+          return { missing: false, variantId: variant.id }
+        })
+      )
+
+      results.forEach((result) => {
+        if (result.status === 'rejected' || 
+            (result.status === 'fulfilled' && result.value.missing)) {
+          const variantId = result.status === 'fulfilled' 
+            ? result.value.variantId 
+            : variants.find(v => v.id)?.id
+          if (variantId != null) {
+            missingVariants.push(variantId)
+          }
+        }
+      })

236-238: Add error logging for debugging.

The silent catch makes it difficult to diagnose why variants are marked as missing. Consider logging errors similar to the updateVideoVariantAlgoliaIndex mutation (lines 309-312).

♻️ Proposed logging addition
-        } catch {
+        } catch (err) {
+          logger?.error(
+            { err, variantId: variant.id, videoId },
+            'Failed to check variant in Algolia'
+          )
           missingVariants.push(variant.id)
         }

267-268: Consider error handling for consistency.

Unlike updateVideoVariantAlgoliaIndex (lines 289-327), this mutation doesn't catch errors from updateVideoInAlgolia. While GraphQL will handle propagation, consider adding a try-catch to enrich errors with context (e.g., videoId) for better debugging.

♻️ Optional error enrichment
+      try {
         await updateVideoInAlgolia(videoId)
-        return true
+      } catch (err) {
+        logger?.error(
+          { err, videoId },
+          'Failed to update video in Algolia'
+        )
+        throw new GraphQLError(
+          `Failed to update video ${videoId} in Algolia: ${getErrorString(err)}`
+        )
+      }
+      return true
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d4bef18 and 4bf216e.

📒 Files selected for processing (1)
  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.

Files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

Define a type if possible.

Files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
🧠 Learnings (10)
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Apollo hooks should use generated types alongside colocated documents. Reference the pattern of `useVideoChildren` (`apps/watch/src/libs/useVideoChildren/useVideoChildren.ts`) for data fetching.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Apollo hooks should use generated types alongside colocated documents. `useVideoChildren` (`apps/resources/src/libs/useVideoChildren/useVideoChildren.ts`) is the reference pattern

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-11-11T23:22:02.196Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 8156
File: apis/api-journeys-modern/src/lib/google/googleAuth.ts:0-0
Timestamp: 2025-11-11T23:22:02.196Z
Learning: In apis/api-journeys-modern, use the validated `env` object from `../../env` instead of accessing `process.env` directly for environment variables that are defined in env.ts (e.g., GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, INTEGRATION_ACCESS_KEY_ENCRYPTION_SECRET). This eliminates the need for runtime validation checks since Zod validates them at application startup.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-04-23T18:26:44.096Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 6358
File: apis/api-journeys/db/seeds/playwrightUserAccess.ts:16-53
Timestamp: 2025-04-23T18:26:44.096Z
Learning: For seeding scripts in the JesusFilm/core repository, environment variables should be validated and the code should throw an error if required variables are missing rather than silently continuing with fallbacks or filtering.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : REST-like endpoints should use SWR hooks paired with zod guards for response validation; parse responses before returning them to callers.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-05-22T02:57:46.179Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 6734
File: apps/journeys-admin/src/components/TermsAndConditions/TermsAndConditions.tsx:58-71
Timestamp: 2025-05-22T02:57:46.179Z
Learning: The team prefers to avoid try/catch blocks when working with Apollo Client mutations in favor of Apollo's built-in error handling mechanisms (checking the returned errors property).

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-07-10T22:50:12.252Z
Learnt from: tataihono
Repo: JesusFilm/core PR: 7153
File: apis/api-journeys-modern/src/schema/journey/simple/getSimpleJourney.ts:13-21
Timestamp: 2025-07-10T22:50:12.252Z
Learning: Prisma's `findUnique` method returns `null` when no record is found, rather than throwing an error. The "not found" case should be handled by checking for null, not with try-catch blocks.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-09-29T23:03:36.840Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7629
File: apis/api-journeys-modern/src/schema/event/utils.ts:43-60
Timestamp: 2025-09-29T23:03:36.840Z
Learning: In the JesusFilm/core repository, do not recommend using Prisma's `upsert` operation for `JourneyVisitor` creation in `apis/api-journeys-modern/src/schema/event/utils.ts` as it is not race condition safe for this use case. The current `findUnique` then `create` pattern is the preferred approach.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
📚 Learning: 2025-08-18T17:23:02.876Z
Learnt from: tanflem
Repo: JesusFilm/core PR: 7464
File: apis/api-media/src/workers/processVideoUploads/service/service.ts:300-303
Timestamp: 2025-08-18T17:23:02.876Z
Learning: In the Video GraphQL schema, the slug field is defined as nullable: false with a resolver that returns slug ?? '', meaning it will never be null but could be an empty string if the underlying data is null/undefined.

Applied to files:

  • apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build (22)
  • GitHub Check: lint (22)
🔇 Additional comments (3)
apis/api-media/src/schema/video/videoAlgolia/videoAlgolia.ts (3)

14-28: LGTM: Comprehensive error conversion.

The fallback chain handles Error objects, strings, JSON serialization, and finally String() conversion effectively.


30-79: LGTM: Well-structured GraphQL types.

The type definitions correctly expose the necessary fields with appropriate nullability constraints.


271-331: LGTM: Excellent error handling and parallel execution.

The use of Promise.allSettled for parallel updates combined with detailed failure reporting (including preview limiting) makes this robust and debuggable.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@apis/api-gateway/schema.graphql`:
- Around line 2645-2661: The gateway GraphQL types CheckVideoInAlgoliaResult and
CheckVideoVariantsInAlgoliaResult declare ok as nullable (ok: Boolean) but the
API_MEDIA resolver exposes it as non-nullable via t.exposeBoolean('ok'); update
both type definitions to ok: Boolean! to match the resolver/TypeScript types
(ensure the ok field is non-nullable in the schema for CheckVideoInAlgoliaResult
and CheckVideoVariantsInAlgoliaResult).

Comment thread apis/api-gateway/schema.graphql
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants