Publish slides from the CLI - #419
Conversation
6ae8e9a to
a347150
Compare
a347150 to
6c435c8
Compare
Add QiitaApi#postSlide / #patchSlide (description typed as string, since the API rejects a present-but-null description with 400) and SlideFileSystemRepo#publishSlide, which posts or patches a slide, then writes the uuid back to the local frontmatter before refreshing the mirror — the uuid has to land first or the sync can't tell which local file the returned slide belongs to and creates a second one. validatePublishSlide reports the frontmatter type check, the value validation and the "older than the remote" check one tier at a time, as validatePublishItem does for articles, and `qiita publish` resolves a basename against both stores and validates articles and slides together. Also fills in the publish/pull help text left empty by #414: with the experimental slide feature on, they say they handle slides too, and the new --slide line drops the "how to enable it" note that no longer applies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6c435c8 to
d842612
Compare
|
|
||
| const { items: targetItems, slides: targetSlides } = args["--all"] | ||
| ? await loadAllPublishTargets({ fileSystemRepo, slideFileSystemRepo }) | ||
| : await resolveTargetsByBasenames(args._, { |
There was a problem hiding this comment.
resolveTargetsByBasenames は、記事とスライドの両方で basename が被るものがあったときにエラーになり、リネームを求めますが、loadAllPublishTargets ではエラーにならないという不整合があります。
basename が被っているときに、どっちを指定したかったのかが明確でないので、エラーを出しているのだと思いますが、loadAllPublishTargets から見た時に不正な構造ではないなら、リネームを求めるのは微妙に思えます。
引数などで、更新したいものがスライドなのか記事なのかを指定できるようにするか、loadAllPublishTargets の方でもエラーを出して一貫性を保つようにすると良さそうです
There was a problem hiding this comment.
ご指摘ありがとうございます。今回はコードを変えずに、別 PR で対応させてください。
単独指定で重複をエラーにしているのは、記事とスライドのどちらを指定したのかが曖昧になるからです。--all は両方とも投稿するので曖昧さがなく、エラーにしていません。とはいえ、--all から見て不正な構造ではないのにリネームを求めるのは一貫していない、というご指摘はその通りだと思います。
そこで、記事・スライドを明示して指定できる publish コマンドを別 PR で用意する予定です。既存の new --slide に揃えて publish --slide <basename> のようにするか、qiita slide publish <basename> のような体系にするかを含めて検討します。
🤖 Generated by Claude Code
There was a problem hiding this comment.
引数などで、更新したいものがスライドなのか記事なのかを指定できるようにするか、
基本的にこれの方針なのですが、渡し方は他のコマンドを通しても一貫したものにしたい (Qiita CLI で preview / publish だけではなく gh CLI などのように API ラッパーとして拡張していく可能性が考えられ、その際に直感的なコマンド体系にしたい) ので、どのような渡し方にするかはこの場で決めないことにしました。
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The 400/403/404 messages told users to check their article files even when a slide caused the error. Switch the wording on experimentalSlideFeatureEnabled, as the help text does, and keep the messages unchanged while the feature is disabled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Articles already honor ignorePublish, but a slide treated the key as a Marp directive and posted it as part of the slide markdown. Reserve the key, keep the local value across a sync since Qiita does not store it, and exclude such slides from the publish targets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tomoasleep
left a comment
tomoasleep
left a comment
There was a problem hiding this comment.
🤖 AI Review 🤖
問題はありません。
その他確認した項目
何を確認したか
- PR: #419
- diff: main...publish-pull-remote-slide
- CI state: success（6/6 checks pass）
- 既存レビュー: レビュアー P-SiZK による行コメント7件。いずれも本日 tomoasleep がコミット付きで返信・対応済み。行に紐づかないトップレベルコメントは0件。
見たが問題にしなかった観点
7件の行コメントすべてで、返信の説明内容と実際のコミット差分が一致していることを確認した。
- エラーメッセージ・ignorePublish 対応・冗長コメント削除・help テストの対称化・template literal 化・async/await 統一の各コメントは、いずれも対応コミットの diff が返信内容と一致していた。
- basename 重複時の挙動が単独指定と
--allで非対称という指摘は、今回はコードを変更せず別 issue に切り出す判断になっている。切り出し先の issue には指摘内容が正確に引用され、設計の選択肢と完了条件まで書かれており、実害もリネームを求められる程度の UX 上の不便に留まるため、この判断は妥当と考える。 - basename 衝突検出のみ
publish.tsに残し repository / validator に slide を漏らさない設計、正常系・異常系・フラグ on/off 両方をカバーするテスト、feature flag 無効時は現行 main と完全に同一という分岐についても、新たな問題は見当たらなかった。
tomoasleep に確認するべき点
特になし。
補足
CI は 6/6 pass、conflict なし（MERGEABLE）。branch protection によりレビュー承認待ちで mergeStateStatus は BLOCKED（P-SiZK の approve 待ち）。
head: 4b0e56a
2 participants
What
slides/配下の markdown をqiita publishで投稿・更新できるようにする。QiitaApiにpostSlide/patchSlideを追加SlideFileSystemRepo#publishSlideが投稿・更新後に uuid をローカル frontmatter へ書き戻し、.remoteミラーを更新validatePublishSlideを追加し、frontmatter の型チェック・値のバリデーション・「リモートより古い」チェックを、validatePublishItemと同じ形で1段ずつ報告qiita publish <basename>/qiita publish --allがスライドも対象になるignorePublish: trueでqiita publish --allの対象から外せるようにする。ignorePublishは Marp ディレクティブとして扱わず、投稿本文に含めないすべて
experimentalSlideFeatureEnabledが有効なときだけ動く。無効なら現行 main と完全に同じ挙動。Base の #418(pull/preview側の同期・ミラー機構)に依存する:
publishSlideは.remoteミラーの更新に、validatePublishSlideの「リモートより古い」判定はisOlderThanRemoteに、それぞれ #418 の実装を使っている。動作確認
自動テスト
yarn lint/yarn build/yarn test(24 suites, 213 tests)がいずれも成功。Issue で名指しされている「過去に round 1 が PR から外れた原因の2バグ」の回帰テストは以下に入れてある。
src/lib/slide-file-system-repo.test.ts:publishSlide()がdescription: nullを""に正規化して送ること / 投稿本文に Marp ディレクティブ(---\nmarp: true\n---)が含まれること / 書き込み順序(uuid 書き戻し → ミラー)で uuid 名の重複ファイルが作られないこと / 投稿直後にミラーと「差分なし」と判定されることsrc/commands/publish.test.ts: 新規 POST / 既存 PATCH / 記事とスライドの混在 / basename 衝突 /--allの対象 /isOlderThanRemote±--force/ フラグ off のときスライド関連が一切呼ばれないことsrc/commands/help.test.ts: フラグ on/off で publish / pull /new --slideの説明が切り替わることsrc/lib/slide-file-system-repo.test.ts:ignorePublish: trueのスライドが publish 対象から外れること / 投稿本文にignorePublishが含まれないこと / 同期してもローカルのignorePublishがミラーとローカルの両方で保たれることsrc/lib/error-handler.test.ts: フラグ on/off で 400 / 403 / 404 の文言が切り替わることdev 環境での手動確認
ローカルの increments/Qiita 開発サーバー(
QIITA_DOMAIN=qiita.localhost、開発用 fixture の public トークン)に対して実施した。ビルド済み CLI(node dist/main.js)を使っている。qiita new --slide dev443-deckslides/dev443-deck.mdがmarp: true/theme: default付きで生成theme: gaia/paginate: trueに書き換えてqiita publish dev443-deckPosted (slide): dev443-deck -> 35d69ce4f26ca19aac1e。frontmatter にid/updated_atが書き戻り、marp/theme/paginateは残存。slides/.remote/35d69ce4f26ca19aac1e.mdが作られ、ローカルはdev443-deck.mdのまま(uuid 名の重複ファイルは作られない)"markdown": "---\nmarp: true\ntheme: gaia\npaginate: true\n---\n# ..."。Marp ディレクティブがそのまま保存されている(以前の実装でテーマが落ちていたバグの回帰確認)qiita publish --allNothing to publish。実サーバー相手でも投稿直後に「差分あり」にならない(往復正規化の回帰確認)qiita publish dev443-deckUpdated (slide): ...で PATCHdescription: nullにしてqiita publish dev443-deckdescription_markdownは""(以前の実装で 400 になっていたバグの回帰確認)qiita pullの状態でqiita publish dev443-deckdev443-deck: 内容がQiita上のスライドより古い可能性がありますで中断qiita publish dev443-deck --forceqiita publish --all→ 1 件だけ編集してqiita publish --allNothing to publish、後者は編集した 1 件だけUpdated (slide)qiita helpqiita pull記事が見つかりませんでした/Qiita上で記事が削除されていないかご確認ください(現行 main と同じ)。on は記事、スライドが見つかりませんでした/Qiita上で記事、スライドが削除されていないかご確認くださいレビュー対応で入れた
ignorePublish対応は、dev 環境の Rails が起動できず手動確認できていない(自動テストのみで確認)。400 / 403 の文言も 404 と同じ切り替え処理なので、自動テストで確認している。qiita previewでの表示 と Qiita 本体での表示theme: gaiaとpaginate: trueが両方で同じように効いていることを Playwright で確認した(toMarkdown()を投稿本文に使う判断の実証)。qiita preview(ローカル)Why
フラグが有効でも Qiita 側のスライド API が有効になっていないユーザーだと、
qiita publishが起動時点でQiitaNotFoundErrorになりうる。これまではスライドの個別ページを開いたときだけ 404 で、記事のプレビュー自体は動いていたので、そこは挙動が変わる。opt-in の実験的フラグなのでsyncSlidesFromQiitaを try/catch で握り潰すのは避けたが、判断が分かれるところなので明示しておく。How
投稿本文は Marp ディレクティブを含めて送る
Qiita 側は渡された markdown をそのまま保存してレンダリングするので、
rawBody(Marp ディレクティブを除いた本文だけ)を送るとtheme:などが落ちて、preview で見えていたものと投稿結果が食い違う。toMarkdown()(#418 でtoPreviewMarkdown()からリネーム済み)を投稿にも使うようにした。descriptionは必ず String で送るQiita 側は「キーが存在するのに String でない」と 400 を返す。
QiitaApi#postSlide/#patchSlideのシグネチャでdescription: stringを要求して、null が渡らないことを型で担保する形にした。キーごと省略する案もあったが、PATCH ではリモートの値が保持されてローカルと恒久的に食い違い、「差分あり」が消えなくなるのでやめた(ローカルを source of truth にする)。
POST 後は先に frontmatter へ uuid を書き戻す
saveSlideはリモートの uuid でローカルファイルを引くので、書き戻す前にミラー更新をすると「対応するローカルファイルが無い」と判断されてslides/<uuid>.mdが新規に作られてしまう。記事のupdateItemUuid→saveItemと同じ順序にしてある。この不変条件はSlideFileSystemRepo#publishSlideの中に閉じているので、呼び出し側から壊せない。投稿シーケンスは #415 と同じ置き場所に揃える
#415 が item 側に作った形(
FileSystemRepo#publishItem/loadPublishTargets()/validatePublishItem)にそのまま対応させ、slide 側もSlideFileSystemRepo#publishSlide/loadPublishTargets()/validatePublishSlideに置いた。basename の衝突検出だけは repository に下げず
publish.tsに残した。 item と slide の両方を知る必要があるが、これは「CLI 引数の名前空間解決」であってドメインロジックではないため。結果として repository / validator 層に slide は漏れていない。QiitaSlide#publishedはid !== nullのまま記事は
.remote/の有無で判定しているが、スライドで同じにすると、フラグ off・オフライン・API 未有効の状態で投稿済みスライドが全部「未投稿」に見える。既存の表示挙動を変えないほうがリスクが低いのでそのままにした。Refs
確認中に見つけた既存の挙動(この PR のスコープ外)
description:の行を キーごと削除 したスライドは、qiita publishがdescriptionは文字列で入力してくださいで止まる。SlideFileContent.readがdata.description(キーが無ければundefined)をそのまま渡すのに対し、checkSlideFrontmatterTypeはnullかstringしか許さないため。description: nullと明示的に書いた場合は通る。これは #409 で入った組み合わせによるもので、この PR では両方とも触っていない。qiita previewでも同じエラーになる既存挙動なので、直すなら別 issue にしたい。🤖 Generated with Claude Code