Stevens / Ivan Issue Report

PR #842 — комментарии и разбор

PR: https://bitbucket.org/stevens_edu/stevens_main_nextjs/pull-requests/842 Задача: IX-SIT60 — Add schema for youtube Branch: IX-SIT60-youtube-schema → master Статус на момент разбора: OPEN (всё ещё не замержен на 2026-04-16) Создан: 2026-03-31 Files changed: 3, Commits: 2

Таймлайн PR

  • 2026-03-31 (вт): Иван открыл PR
  • 2026-03-31 (вт): Michael — комментарий на youtube-embed.js: оставлен console.log('Parsed video ID:', videoId) (debug-строка)
  • 2026-03-31 (вт): Michael — попытка merge в vercel-stage сломалась: Cannot query field "publishedAt" on type "Video" (новое поле есть только в develop Contentful, не в master). Просьба прописать pre-merge tasks в описание PR.
  • 2026-03-31 (вт): Michael — VideoObject генерируется даже без uploadDate, что даёт невалидный VideoObject (по словам Michelle Strauss). Просьба не выводить schema в этом случае.
  • 2026-04-01 (ср): Иван — "I'll add a condition so that the schema isn't added if there's no upload date. Thank you!"
  • 2026-04-01 (ср): Иван отредактировал description; запушил коммит c184bc7 (UPDATED)
  • 2026-04-01 (ср): Vercel bot — deploy ready
  • 2026-04-03 (пт): Michael — "EDIT: Nevermind, question removed. I just realized the missing publishedDate explains my concern."
  • 2026-04-03 (пт): Michael — новый кейс: ~700 существующих Video entries не синхронизированы (Published At = "Not synced yet"), UX путаный. Предложение: pre-merge step с автозаполнением.
  • 2026-04-06 (пн): Иван — предлагает one-time backfill migration script.
  • 2026-04-10 (пт): Michael — согласен; просит положить скрипт в codebase Stevens CMS Tools.
  • 2026-04-10 (пт): Michael отредактировал description.
  • 2026-04-16 (чт): Иван — "Instructions have been added to the end of the PR description"
  • 2026-04-16 (чт): Иван отредактировал description.
  • 2026-04-29 (ср), 22:28: письмо Alexis Watson про development workflow (см. главный README) — PR всё ещё OPEN.

Comment #1 — оставлен debug console.log в коде ⚠️

  • Скриншот: screenshots/01_michael_debug-console-log.png

  • Файл: components/youtube-embed/youtube-embed.js

  • Автор: Michael Forbes

  • Дата: 2026-03-31 (на сам день открытия PR)

  • Цитата (комментарий на строку):

    console.log('Parsed video ID:', videoId)

    This looks like a debugging line we wouldn't want merged into master.

  • Суть: в PR попала отладочная строка console.log, которую Иван забыл убрать перед отправкой на ревью. Michael сразу его поймал.

  • Обоснованно? (да/частично/нет): Да, на 100%. Это банальная ошибка качества — debug-код в production-PR. Michael сам же зарезолвил тред.

  • Почему важно: прямой пример того, о чём пишет Alexis в письме — "internal validation pass prior to sending things over". Это ровно то, что должен ловить self-review / lint / pre-commit. Иван этого не сделал перед отправкой.

  • Действие: комментарий помечен OUTDATED — значит, Иван убрал console.log в следующем коммите. Поправил.

Comment #2 — сломанная сборка из-за missing pre-merge tasks 🚨

  • Скриншот: screenshots/02_michael_build-failed-pre-merge-tasks.png

  • Автор: Michael Forbes

  • Дата: 2026-03-31

  • Цитата:

    I tried merging this branch into vercel-stage (to do a build that combines this new code with the master Contentful environment) but the build failed due to Error while querying backend: Cannot query field "publishedAt" on type "Video".

    It makes sense: this code expects a new field which has only been added to the content model in the develop Contentful environment so far. When other steps beyond just clicking the PR's Merge button are required to deploy a pull request, please mention the steps in the PR description. For example, in this case it looks like we have a new field, and that field's configuration is defined by the Stevens CMS Tools app, so the deployment instructions could say something like...

    Pre-merge tasks:

    1. Deploy the Stevens CMS Tools app to the master environment (separate PR for that app must be approved first).
    2. Go into Contentful > Apps > Merge > Target=master Source=develop > select the Video row > merge.

  • Суть: PR не мог быть замержен как есть — для деплоя требовались доп. шаги в Contentful (наличие нового поля publishedAt в master environment) и отдельный PR для Stevens CMS Tools. Этих инструкций в описании PR не было — Michael узнал об этом, только когда у него упал build.

  • Обоснованно? (да/частично/нет): Да, абсолютно.

  • 🚨 Это дословно повторяет претензию Alexis из письма (29 апреля):

    we'd kindly ask... that we continue to ensure pull requests that require post-deployment tasks are always accompanied by such.

    То есть этот PR — один из конкретных кейсов, на который ссылается Alexis. Здесь PR требовал pre-deployment tasks (Contentful merge + отдельный PR для CMS Tools), и они не были описаны. Michael узнал об этом только через сломавшийся build.

  • Реакция Ивана: в итоге description был обновлён (см. скрин 08), pre-merge tasks прописаны.

  • Действие: исправлено в description, но гэп процесса налицо — это не должен был ловить Michael.

Comment #3 — невалидный VideoObject (schema без обязательного uploadDate) ⚠️

  • Скриншот: screenshots/03_michael_invalid-videoobject-no-uploaddate.png

  • Автор: Michael Forbes (с упоминанием Michelle Strauss)

  • Дата: 2026-03-31

  • Цитата:

    I noticed that a few Video entries in the develop environment now have the "Published at" field populated, for example the video on the home page: we see the VideoObject including "uploadDate": "2025-01-21T21:43:40.000Z" with this code (and no VideoObject without this code) – very nice.

    And for Video entries that don't have the "Published at" field populated yet, for example the video on this article: we see the VideoObject excluding uploadDate with this code (and no VideoObject without this code) – it's cool to see it gracefully skip that property, but I think Michelle Strauss mentioned that this would be an invalid VideoObject. If that's true, we should not be outputting a VideoObject in this case, right?

  • Суть: Schema VideoObject требует поле uploadDate (по schema.org для рич-сниппетов в Google). Реализация Ивана выводила VideoObject и тогда, когда uploadDate отсутствует — то есть формально невалидный schema.org объект. SEO-эксперт Michelle Strauss это подтвердила.

  • Обоснованно? (да/частично/нет): Да. Это функциональная ошибка — фича выдаёт невалидную schema, которая не сработает для Google rich results.

  • Реакция Ивана (2026-04-01):

    I'll add a condition so that the schema isn't added if there's no upload date. Thank you!

    Поправил на следующий день.

  • Почему важно: опять же, это пример из категории "deviate from spec" — фича декларативно работает, но фактически генерирует мусор (невалидную schema). Если бы Иван знал требования schema.org или сверился с Michelle до старта — этого можно было избежать.

Comment #4 — Vercel bot (служебное)

  • Скриншот: screenshots/04_vercel-deploy-and-update.png
  • Автор: Vercel for Bitbucket (от имени Michael)
  • Дата: 2026-04-01
  • Не претензия. Deploy preview успешно собрался после исправлений.
  • Иван запушил коммит c184bc7.

Comment #5 — Michael снимает свой вопрос (positive)

  • Скриншот: screenshots/05_michael_nevermind-publishedDate.png

  • Автор: Michael Forbes

  • Дата: 2026-04-03

  • Цитата:

    EDIT: Nevermind, question removed. I just realized the missing publishedDate explains my concern.

  • Не претензия. Michael сам разобрался в каком-то своём вопросе и снял его. Свидетельствует о вдумчивом ревью.

Comment #6 — UX-проблема: ~700 Video entries в "Not synced yet" 🟡

  • Скриншот: screenshots/06_michael_700-entries-not-synced.png

  • Автор: Michael Forbes

  • Дата: 2026-04-03

  • Цитата:

    @Ivan.Kotovsky When I go to a Video entry like this, the Published At field says "Not synced yet — add a video URL to trigger automatic sync." This is confusing to the Contentful user managing the content, because they shouldn't "add a video URL" if there's already one.

    I tried clicking "Change status" > "Publish with References" > "Publish" but this did not populate the field. Then I tried adding and removing a random character to the existing video URL ("Youtube Link" field) and that populated the field, at which point I also need to click "Publish changes". If that's what authors need to do, the "Not synced yet" message should say that. But can we offer something better, so that users won't have to go into nearly 700 existing Video entries and do that? Like maybe we can add a pre-merge step where we somehow automate populating the "Published at" field for all existing Video entries?

  • Суть: реализованный механизм автосинка Published At срабатывает только когда контентщик "трогает" поле YouTube Link. На существующих ~700 entries поле просто пустое + сообщение "Not synced yet — add a video URL to trigger automatic sync" — вводит в заблуждение (URL уже есть). Заставлять контент-команду вручную пройти 700 записей — нереалистично.

  • Обоснованно? (да/частично/нет): Да. Это не баг кода, а gap в продуктовом мышлении: фича запланирована без миграционной стратегии для исторических данных + сообщение в UI неточное.

  • Реакция Ивана (2026-04-06):

    For the existing video entries missing data — we'd suggest running a one-time backfill migration script. The current sync logic already handles new and updated entries correctly, so this would just fill in the gaps for historical data. We'd back up the environment first and test everything on dev before proceeding.

  • Michael (2026-04-10):

    Perfect. After backfill, we would see the "Not synced yet — add a video URL to trigger automatic sync" message only when creating new Video entries, so it will make sense at that point. I'll add this as another pre-merge task. Would you be able to place that script into the codebase for Stevens CMS Tools, since it will probably share a decent amount of code that's already there?

  • Иван (2026-04-16):

    Instructions have been added to the end of the PR description

  • Что важно:

    • Решение конструктивное (backfill script), договорились.
    • Но до прямого вопроса Michael ни Иван, ни мы не подумали про существующие 700 записей и про UX "Not synced yet" сообщения.
    • Это снова возвращает к теме internal validation pass перед отправкой PR.

Финальное состояние PR

  • Скриншот описания: screenshots/08_pr-description-with-pre-merge-tasks.png
  • Branch: IX-SIT60-youtube-schema → master, OPEN на момент 2026-04-16
  • Created: 2026-03-31, Last updated: 2026-04-16 → PR висит уже 16+ дней
  • Files changed: 3, Commits: 2

Description (после доработок):

Summary: Added schema with uploadDate for youtube component Changes:

  • Added schema for youtube component
  • Add publishedAt for featuredVideo and videoWithCaption components

Pre-merge tasks:

  1. Deploy the Stevens CMS Tools app to the master environment (separate PR for that app must be approved first).
  2. Go into Contentful > Apps > Merge > Target=master Source=develop > select the Video row > merge.
  3. Run the backfill script as discussed here.

Using the Backfill Script: The video date backfill script was tested in the develop environment. During testing:

  • the script completed successfully
  • entries with empty publishedAt values were updated
  • already-filled entries were skipped
  • draft entries remained draft

For the full run procedure, backup steps, and command sequence, follow the instructions in: VIDEO_DATE_BACKFILL.md