[Due for payment 2025-08-20] [$250] Add CI check for old video player in help site

[rafecolton]

Problem

When a PR is merged that adds an outdated video URL to the help site (example) then that page doesn't match the rest of the site

Originally reported by @rushatgabhane here. cc @strepanier03

@roryabraham you might also be interested in this one.

Why is this important?

Our help site must be uniform in aspects like video player for it to appear professional and polished.

Solution

Using the wrong type of video link is prone to human error, so let's add a CI check for this. The file .github/workflows/deployExpensifyHelp.yml may be a good candidate. However, it's unclear how the concurrency: param would affect this - if a bad commit is pushed while a build is in progress, will it still get checked after or will it get ignored? If the latter, then a separate check ay be more appropriate.

As part of the solution, please use docs/articles/travel/getting-started/Expensify-Travel-Walkthrough.md for testing and update the video player there as part of the PR

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021950276909108016415
• Upwork Job ID: 1950276909108016415
• Last Price Increase: 2025-07-29

Issue Owner

Current Issue Owner: @muttmuure

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @hungvu193 (External)

[melvin-bot]

Triggered auto assignment to @muttmuure (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[PiyushChandra17]

🚨 Edited by proposal-police: This proposal was edited at 2025-07-24 15:09:07 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Add CI check for old video player in help site

What is the root cause of that problem?

Improvement

What changes do you think we should make in order to solve the problem?

It already mentioned in OP. We should add a CI check for .github/workflows/deployExpensifyHelp.yml. Currently we are using outdated video player like iframe

App/docs/articles/travel/getting-started/Expensify-Travel-Walkthrough.md

Line 11 in 9b98e97

<iframe src="https://drive.google.com/file/d/125j1iFdce9VP6VkDZ609TrfaaSMzdc8M/preview" allowfullscreen width="560" height="315" allow="autoplay"></iframe>

Perhaps we should use some modern video component. Lets say <VideoPlayer src="https://drive.google.com/file/d/125j1iFdce9VP6VkDZ609TrfaaSMzdc8M/preview".

• We should add a CI Check for .github/workflows/deployExpensifyHelp.yml, create a new workflow

name: Check for Old Video Players

on:
  pull_request:
    paths:
      - 'docs/**.md'
    branches-ignore:
      - staging
      - production

jobs:
  scan-video-links:
    name: Ensure modern video player is used
    runs-on: ubuntu-latest

    steps:
      - name: Checkout repo
        uses: actions/checkout@v4

      - name: Scan for <iframe> YouTube embeds
        run: |
          echo "🔍 Scanning for old-style YouTube <iframe> embeds..."
          BAD_FILES=$(grep -rl '<iframe[^>]*youtube\.com/embed' docs/ || true)

          if [ ! -z "$BAD_FILES" ]; then
            echo "🚨 Found outdated video embed(s):"
            echo "$BAD_FILES"
            echo
            echo "❌ Please use the modern <VideoPlayer src=\"...\" /> component instead of <iframe>."
            exit 1
          else
            echo "✅ No legacy embeds found."
          fi

• Update the expensify-walkthrough MD

Replace the old <iframe>

App/docs/articles/travel/getting-started/Expensify-Travel-Walkthrough.md

Line 11 in 9b98e97

<iframe src="https://drive.google.com/file/d/125j1iFdce9VP6VkDZ609TrfaaSMzdc8M/preview" allowfullscreen width="560" height="315" allow="autoplay"></iframe>

with the new

<VideoPlayer src="https://drive.google.com/file/d/125j1iFdce9VP6VkDZ609TrfaaSMzdc8M/preview" />

• Concurrency concern: If the concurrency: param is set in our existing deployExpensifyHelp.yml, it could cancel previous runs, it’s better to run it in a separate workflow. This avoids being blocked or skipped by concurrency settings.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

n/A

What alternative solutions did you explore? (Optional)

n/A

Result

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[github-actions]

⚠️ @PiyushChandra17 Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

[rafecolton]

@MacroLens are you interested in this one?

[MacroLens]

I'll take a look.

…

-------- Original Message --------

On 7/24/25 10:46 AM, Rafe Colton wrote: rafecolton left a comment [(Expensify/App#67072)](#67072 (comment)) ***@***.***(https://github.com/MacroLens) are you interested in this one? — Reply to this email directly, [view it on GitHub](#67072 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ANANN7U7HVF3IZSZK6XUS4T3KDWWFAVCNFSM6AAAAACCJAQF5GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCMJTG43DAMBXHA). You are receiving this because you were mentioned.Message ID: ***@***.***>

[MacroLens]

@rafecolton some clarifying questions to make this issue clearer.

1. I am assuming the issue is that the video shared is not responsive to screen size? And the intent for the issue is a change to the CI to detect this?
2. On the help pages I didn't see any other uses of iframe for embedding the videos, or any videos 😅. If you could point me to a good use case that may help. You mention the "wrong type of video link," do you mean that videos should instead be using docs/_includes/video.html as their container or something else?

In response to the issues about concurrent runs. From the github docs:

This means that there can be at most one running and one pending job in a concurrency group at any time. When a concurrent job or workflow is queued, if another job or workflow using the same concurrency group in the repository is in progress, the queued job or workflow will be pending. Any existing pending job or workflow in the same concurrency group, if it exists, will be canceled and the new queued job or workflow will take its place.

That is if you have an in-progress (currently running), and you put a new job up it will be pending. If a third job were to be added it would replace the currently pending job.
If you have job A running, then you add job B while still running, A is in progress and B is pending. If you add C while A is running, C replaces B for the pending slot.

To me this does not seem like an issue if you are ok with checking all the docs for this issue on deploy run vs. only checking the files changed.

[rafecolton]

Ah my bad, I forgot to link to this: https://github.com/Expensify/App/tree/main/docs#add-videos. That's the format that should be used

RE concurrency - I think it's fine if some intermediate commits are not checked as long as all changes in the PR are checked in the latest commit and the check shows red if it fails.