Skip to content

Add new Google Slides importer plugin - #982

Open
Genyus wants to merge 65 commits into
mainfrom
develop
Open

Add new Google Slides importer plugin#982
Genyus wants to merge 65 commits into
mainfrom
develop

Conversation

@Genyus

@Genyus Genyus commented Sep 10, 2026

Copy link
Copy Markdown
Member

Replace previous local import process with admin screen

Genyus added 30 commits August 21, 2026 03:37
- A6 verified: PhpPresentation parses CBF PPTX with sufficient fidelity
- R1 closed: multi-column geometry detection fully functional
- Key impl notes: shapes in pixels (div9525 for EMU), Drawing\Gd::getContents()
  for image bytes, ZipArchive for hidden-slide detection
- P2.4/P2.7 checklist items updated with concrete implementation guidance
- Plan bumped to v1.3.0
All Phase 0 gates cleared. Phase 1 (plugin scaffold and auth) unblocked.
Plan bumped to v1.4.0.
Boilerplate matches cbf-multisite structure (Genyus/WordPress-Plugin-Boilerplate).

Modules created:
- Bootstrap: cbf-slides-importer.php, Main.php, Install.php, Utils.php
- Crypto.php: AES-256-GCM encrypt/decrypt keyed by CBF_SI_ENCRYPTION_KEY env var
- Assets.php, Admin/{Main,Assets,SettingsPage,ImporterPage}.php
- Api/{Router,AuthController,DriveController,ConfigController,JobController,PreviewController}.php
- Google/{OAuthClient,DriveClient}.php: OAuth2 + Drive file export
- Pptx/{Parser,GeometryDetector,SlideClassifier,BlockRenderer}.php
- Import/{JobRunner,LearnDashImporter}.php

DB tables (dbDelta): cbf_slide_import_configs, cbf_slide_import_jobs
Composer deps: google/apiclient ^2.15, phpoffice/phppresentation ^1.1
All PHP files parse clean (php -l).
.gitignore: add cbf-slides-importer plugin exception
Install.php: change slide_overrides, created_post_ids, result_summary
from LONGTEXT NOT NULL DEFAULT '{}' to LONGTEXT NULL DEFAULT NULL.
MariaDB strict mode forbids default values on TEXT/BLOB columns.

Main.php: add plugin.php include guard before is_plugin_active() so
the check works outside admin context (WP-Cron, REST, WP-CLI).
Also correct REQUIRED_PLUGIN entry-file to learndash-bulk-create.php
(actual file, not learndash-bulk-lessons-or-topics.php).

Verified: clean activation, both tables created, 13 REST routes at
cbf-si/v1 registered, /auth/status returns 401 unauthenticated.
Adds a dev-only 32-char encryption key so the Crypto class works
in the local Lando environment. Must be replaced with a strong
random value in any shared or staging environment.
…t secret

sanitize_textarea_field() runs htmlspecialchars() which encodes double-quotes
to ", making json_decode() fail on perfectly valid JSON. Switch to
wp_unslash()+trim() which strips WP magic quotes without mangling JSON.

Also add structural validation: require "web" or "installed" key with
client_id and client_secret present, and re-encode via wp_json_encode()
before encrypting to normalise whitespace.
Was calling create_auth_url() on a raw GoogleClient — that method does not
exist on GoogleClient and, more critically, it bypassed the OAuthClient
wrapper that stores the state nonce transient. The callback hash_equals()
check would always fail without the transient in place.
The "Connect Google Drive" button was an <a href> pointing directly at
the /auth/begin REST endpoint. A plain browser GET to a REST endpoint
never carries X-WP-Nonce, so WP cookie-auth returns 401.

Fix:
- Add Admin\OAuthBridge: registers admin_post_cbf_si_auth_begin, verifies
  wp_nonce, calls OAuthClient::create_auth_url(), wp_redirect() to Google.
- ImporterPage now renders OAuthBridge::begin_url() (nonce-protected
  admin-post URL) instead of the raw REST URL.
- AuthController::callback() permission_callback changed to __return_true;
  auth checked manually inside (Google redirect carries cookie but never
  X-WP-Nonce — the OAuth state nonce is the CSRF protection).
- Admin\Main::hooks() registers OAuthBridge.

The REST /auth/begin endpoint is kept for the Phase-3 JS SPA which will
send X-WP-Nonce in the request header.
is_user_logged_in() always returns false in the REST API callback because
WP REST cookie-auth requires X-WP-Nonce, which Google's browser redirect
never sends.

Fix: state parameter is now "{user_id}:{nonce}" (set in create_auth_url).
The callback parses user_id from state, loads the user with get_user_by(),
validates capability and nonce, then calls exchange_code() with that user_id.
No session cookie or nonce header needed — the state nonce is the CSRF token.
Admin/Assets.php: restrict enqueue to importer page, register gapi,
fix ajax_url. JS: full Google Picker implementation, job list with
status polling, fix localize global name, wire bindJobActions.
.gitignore: layered negations to track source JS/CSS.
- js: guard DOMContentLoaded with readyState check — in the WP admin,
  74+ synchronous scripts can cause the event to fire before our footer
  script executes; fall through to immediate bootstrap() when DOM is
  already interactive/complete
- js: replace object spread with Object.assign() to satisfy the root
  ESLint parser; add plugin-level .eslintrc.js (ecmaVersion 2020) to
  support async/await and modern browser globals
- php: bump VERSION to 1.0.1 to bust browser asset cache
- db: add 'parsed' to status ENUM in Install.php — missing value caused
  MySQL to store '' for jobs that completed parsing, showing '-' in the UI
- parser: replace Font::isUnderline() (non-existent) with
  getUnderline() !== Font::UNDERLINE_NONE (correct PhpPresentation API)
- Admin/Assets.php: add wp_rest_url to localized params for WP REST access
- Api/JobController.php: trigger_import accepts mode/course_id body params
  and merges them into result_summary.config before scheduling import phase
- assets/js/admin/cbf-slides-importer.js v1.0.2:
  - Add Api.getCourses() fetches sfwd-courses via WP REST with nonce
  - Add Api.triggerImport(id, config) passes mode/course_id to backend
  - Add _loadCourses() caches course list to avoid repeat fetches
  - Replace direct import button with 'Configure and Import' which expands
    an inline config panel below the job row
  - Config panel: mode radio toggle plus course dropdown
  - _doImport reads panel values and passes config to trigger_import
  - Warn if lesson-with-topics selected without a course
  - Panel toggles on/off; closes after import triggered
- cbf-slides-importer.php: bump version to 1.0.2
- plans/cbf-slides-importer-plugin.md: mark Phase 1 and Phase 2 complete,
  P3.1-P3.2 complete; update plan version to 1.6.0
The learndash-bulk plugin exposes its instance as a global variable
$extended_learndash_bulk_create, not via a named function or static
method — fix get_bulk_plugin() to use the global.

run_import_cli() takes a CSV file path; the actual programmatic API is
run_import($content_type, $headers, $rows, $options) — fix
run_import_row() to call run_import() directly:
- Extract post_type as $content_type (not a column)
- Build $headers and $rows from the associative row array
- Pass $img_dir via options.media_dir so ELDBC_Media handles rewrite
- Extract created/updated post IDs from the returned stats arrays

Media rewrite is now handled inside run_import() via ELDBC_Media so
the separate rewrite_post_images() call is no longer needed.
Drive API files.export has a ~10 MB cap; presentations with many slides
or high-res images exceed it with HTTP 403 exportSizeLimitExceeded.

Add try_direct_export() which streams the presentation via the standard
Docs export URL (https://docs.google.com/presentation/d/{id}/export/pptx)
using the user's Bearer token and wp_remote_get() stream mode, writing
directly to disk without loading the full file into memory.

- export_pptx() now tries the API first; on size-limit 403 it falls back
- is_size_limit_error() detects 'exportSizeLimitExceeded' or 'too large'
- should_retry() extracted to reduce try_api_export() cyclomatic complexity
- Last-error default initialised inline to avoid null-coalescing branch
PHP shape objects cannot survive JSON serialisation. The import
phase previously used classified data decoded from the DB, where
all PhpPresentation Shape RichText instances had become empty arrays,
causing every instanceof check to fail and all slide content to render
as empty strings.

Fix: import phase now re-parses the PPTX from the stored pptx_path
and re-classifies using the config stored in result_summary. The
classified key is no longer written to result_summary.

Also adds a post_title field to the config panel (defaulting to the
deck name) so the lesson title can be set before import, rather than
being hardcoded as Imported Lesson.
Two post-import bug fixes:

Image paths (media library not populated)
- BlockRenderer::render_image_block() was emitting src="/filename.png"
  (absolute path, no prefix). ELDBC_Media::rewrite_paths() only matches
  the pattern media/filename so it never uploaded or rewrote these paths.
- Changed to emit src="media/filename.png" when no media_base_url is
  supplied (the import path). ELDBC_Media's resolve_under_media_dir()
  candidate base/without_prefix then resolves to the actual file.
- Switched from esc_url() to esc_attr() for the placeholder src value.

Footer exclusion (copyright text / CBF icon on every slide)
- GeometryDetector: added FOOTER_TOP_RATIO = 0.87 (calibrated against
  Session 07 deck: footer shapes at t>=475 on 540px slides, body at t=96-113).
  Added FOOTER_PLACEHOLDER_TYPES (sldNum, ftr, dt) for named footer shapes.
- build_content_blocks() now accepts slide_height_px and passes it down.
- collect_content_shapes() excludes sldNum/ftr/dt placeholders and shapes
  whose top-edge >= footer cutoff (87% of slide height).
- Parser::extract_images() skips Drawing shapes in the footer zone.
- Parser passes slide_height_px through parse() -> parse_slide() ->
  extract_images() and build_content_blocks().

Bumped plugin version to 1.0.4 to bust browser JS cache.
Google Slides progressive-reveal exports produce several consecutive PPTX
slides with the same title (one per animation step). This caused the same H2
heading to repeat in the rendered output for every build-step slide.

BlockRenderer now tracks the last emitted heading title across the slide loop.
A slide whose title is non-empty and identical to the previous non-empty title
has its heading suppressed — only the first slide in each run of identical
titles emits an H2.

This applies in both render modes:
- lesson-only: prev_title tracked across all body slides.
- lesson-with-topics: prev_title tracked across the whole pass; heading-type
  slides (which become LearnDash Topics) also update the tracker, so the first
  body slide in a topic does not repeat the topic title as a redundant H2.
wp:list-item blocks must not wrap their text in <p> — the extra paragraph
tags add unwanted bottom margins and break the block parser's expectations.

Also added the wp:list-item block comments around each <li>, which Gutenberg
requires for proper round-trip block serialisation.
…dismiss

Two picker UX bugs:

Page jump on open
- picker.setVisible() injects an iframe into the document body and in some
  browsers the viewport scrolls to it. Fixed by saving window.scrollX/Y
  before the picker renders and restoring it synchronously and via
  requestAnimationFrame() after setVisible() returns.

Button stays disabled after dismissal
- _openPicker() only re-enabled the button when a file was selected.
  Dismissing the picker without a selection left the button disabled and
  showing 'Loading picker…' for 30 s (the blunt fallback timeout).
- Picker.open() now accepts an onDismissed callback alongside onSelected.
  _buildAndShow() calls onDismissed when data.action === CANCEL. The 30 s
  timeout fallback is removed — the CANCEL action fires reliably.
- Error path in Picker.open() also calls onDismissed so the button recovers
  if the picker-config fetch fails.
… timing

Button label
- Added onReady callback to Picker.open() / _buildAndShow(). It fires
  immediately after picker.setVisible(true). _openPicker() uses it to
  reset the button text to the normal label while keeping the button
  disabled — so the 'Loading picker...' message disappears as soon as
  the modal is on screen rather than waiting for user interaction.

Scroll position
- Previous fix saved window.scrollX/Y inside _buildAndShow(), which runs
  after gapi.load() completes. gapi.load() injects a hidden iframe on
  first use which causes the viewport jump, so the saved position was
  already wrong by the time it was read.
- scrollX/scrollY are now captured in _openPicker() (synchronously,
  before Api.pickerConfig() or gapi.load() run) and passed through
  Picker.open() → _buildAndShow() so the true pre-open position is
  always available for restoration.
Previous approach called window.scrollTo() synchronously and via
requestAnimationFrame() after picker.setVisible(). This was too early:
the browser's scroll-to-focus on the picker iframe fires asynchronously,
overriding both restores.

Instead, a scroll event listener is registered before setVisible() with
{ once: true } so it fires in direct response to the browser-initiated
scroll and immediately calls window.scrollTo() with the pre-open
coordinates. { once: true } auto-removes the listener on first fire,
ensuring it cannot interfere with any scroll the user makes while the
picker is open. A 1 s setTimeout removes the guard if no scroll event
fires (already at top of page, or subsequent opens where the iframe is
already in the DOM and no focus-scroll occurs).
…urge

Implements the cbf_si_cleanup WP-Cron event (P2.11 / R4 / Failure Isolation).

New Import/Janitor class
- CLEANUP_HOOK = 'cbf_si_cleanup', registered as an hourly WP-Cron event.
- reset_stale_jobs(): queries for jobs in 'downloading', 'parsing', or
  'importing' status whose updated_at is older than 30 minutes. Resets
  each to 'pending' with a human-readable note in error_message so the
  next WP-Cron tick retries the job from scratch. 'parsed' is excluded
  — it is a stable waiting-for-user state, not in-flight processing.
- purge_orphaned_tmp_dirs(): scans cbf-slides-tmp/ for job_N subdirs
  whose mtime is older than 2 hours and deletes them via
  Utils::rmdir_recursive(). Only touches job_* subdirs; the root dir
  and its index.php sentinel are left alone.

Install
- install(): schedules 'cbf_si_cleanup' as an hourly event on activation
  if not already registered (idempotent).
- deactivate(): now also clears the cleanup hook alongside the job
  processor hook, so no orphaned cron events remain after deactivation.

Main
- Registers Janitor::hooks() alongside JobRunner::hooks() so the
  cleanup event handler is active whenever the plugin is loaded.
P2.11: mark complete in plan.

P3.3 — Slide map UI component:
- JobRunner now extracts serialisable slides_meta (index, slide_number,
  title, layout_name, is_hidden, is_cover, slide_type) after classify()
  and stores it in result_summary alongside pptx_path/img_dir.
- New GET /jobs/{id}/slides REST endpoint (JobController::slides()) reads
  slides_meta and merges any stored overrides so the UI pre-populates
  on re-open.
- Admin JS: _openConfigPanel() now fetches slides in parallel with
  courses. A collapsible <details> slide map renders below the config
  form with #, title, layout name, auto-detected type badge, and an
  override <select> per slide (options: auto, Cover, Heading/Topic,
  Content, Hidden).
- Api.getJobSlides(id) added; errors silently fall back to empty list.

P3.5 — Config validation + slide_overrides wiring:
- Title field is now required; _doImport() blocks with alert() and
  focuses the field when empty.
- post_title is always included in the import payload (not optional).
- slide_overrides collected from all [data-slide-override] selects;
  included in POST /jobs/{id}/import body when non-empty.
- New slide_overrides REST arg (type: object) on /jobs/{id}/import.
- save_import_overrides() sanitises each entry (absint key, sanitize_key
  value, allow-list check) and stores as JSON in config.slide_overrides.
- classify_from_summary() already decodes slide_overrides, so overrides
  take effect at import time with no further changes.
Root cause of blank action column (job 12):
- bulk plugin's find_post_for_row() matched existing lesson by title
  ('Introduction to Software Development', post 152) and returned
  status:'skipped' with overwrite:false
- run_import_row() only collected created_entries + updated_entries,
  silently discarding skipped_entries → created_post_ids:[] → blank UI

Fixes:
- run_import_row(): also iterates skipped_entries and appends their IDs
  to the returned post ID list; logs a warning with the skip count
- run_import_row(): now accepts bool $overwrite and passes it to the
  bulk plugin (previously hard-coded false)
- import_lesson_only() / import_lesson_with_topics(): thread $overwrite
  through from import() which reads config['overwrite']
- save_import_overrides(): handle new 'overwrite' boolean REST param
- /jobs/{id}/import: register 'overwrite' as type:boolean REST arg
- Config panel: 'Overwrite existing content' checkbox with description
- _doImport(): reads checkbox, always sends overwrite in importConfig

Data fix: job 12 created_post_ids patched to [152] via direct SQL
so the action column immediately reflects the existing lesson.
Genyus added 30 commits August 26, 2026 17:07
- Read config panel (incl. slide_overrides) before clearing cell innerHTML
  in _doPreview(), so hidden-slide overrides are sent with the POST request
- Add .cbf-si-preview-inner wrapper (max-width: 720px, centred) inside the
  full-width scrollable .cbf-si-preview-content container
- compute SHA-256 config_hash (drive_file_id + mode + course_id +
  slide_overrides) at import-trigger time; store in result_summary
- return 409 when a prior done job for the same deck + config is found;
  include prior_job_id and prior_job_date in response data
- accept force=true on POST /jobs/{id}/import to bypass the check
- JS: triggerImport() intercepts 409 (returns {conflict:true} instead of
  throwing); _showImportConflict() shows inline warning in preview panel
  or confirm dialog when in configure view; force-import action re-runs
  with force flag
- fix: _doImport() falls back to cached preview config when form elements
  are no longer in the DOM (import triggered from preview panel)
- refactor JobController to satisfy PHPCS complexity limits:
  decode_summary(), extract_config(), decode_stored_overrides(),
  apply_request_config(), apply_slide_overrides(), store_config_hash(),
  query_prior_import()
Posts are created in their natural published state. If any errors occur
during the batch, all created post IDs are reverted to draft so students
never see partial content (NR4).

Replaces the temporary wp_insert_post_data filter approach, which risked
interfering with concurrent post-creation processes running in the same
request lifecycle.
Replace window.confirm() with a fixed-position overlay modal that shows:
- a summary of content to be created (lesson count, topic count, course name)
- amber warning notes for missing course selection or overwrite mode

On a 409 conflict the modal stays open; the warning strip is injected and
the confirm button is swapped to "Re-Import anyway" (force=true). The UX
is identical whether the modal is opened from the configure or preview
panel. _showImportConflict() and the force-import event action removed.
Animated progress bar removed — no reliable sub-phase progress metric
is available without backend instrumentation. The existing job list
status badges (downloading/parsing/importing/done/failed) provide
sufficient feedback. Config panel closes immediately on confirm and
_pollJob() keeps the badge current every 3s until a terminal state.
- Remove 'lesson-with-topics' mode entirely; replace with 'topic' mode
  which imports the entire deck as a single sfwd-topic post
- BlockRenderer: remove render_lesson_with_topics(); both modes now use
  render_lesson_only() — HTML is identical, only the LD post type differs
- LearnDashImporter: replace import_lesson_with_topics() with
  import_as_topic(); threads lesson_id through parse_import_params()
  and run_import_mode()
- DB schema: mode column changed VARCHAR(50) (was ENUM); lesson_id
  column added to cbf_slide_import_configs
- API: update enum validation in ConfigController, JobController, and
  PreviewController; add lesson_id param to Config and Job controllers
- UI: mode radios now read 'Lesson' / 'Topic'; Topic mode reveals a
  Lesson dropdown populated via ldlms/v2/sfwd-courses/{id}/steps
  (Shared Course Steps compatible); course change refreshes lessons
- HTML entity decoding added for all WP REST API title.rendered values
  via _decodeHtml() (textarea pattern) before _esc() insertion
Pass lesson/topic/course labels (singular + plural) from PHP to JS via
wp_localize_script using learndash_get_custom_label(), falling back to
the default English strings when LearnDash is unavailable.

Add _label(key) helper to the JS app and replace all hardcoded entity
names throughout the config panel, dropdowns, import modal, and alert.
Add POST /jobs/upload endpoint (multipart/form-data) that accepts a .pptx
file directly, bypassing the Google Drive download step. The file is
saved to the job temp directory immediately and the background cron is
scheduled with phase='parse' to go straight to parsing.

Refactor JobRunner to extract run_parse_and_store() (shared parse +
preview pipeline), add run_parse_phase() for local uploads, and add
dispatch_phase() to route cron payloads cleanly. drive_file_id is stored
as '' for upload jobs so the prior-import conflict check skips them.

Add 'Upload local .pptx file' button and hidden file input to the admin
UI alongside the Drive picker, with Api.uploadJob() and _handleFileUpload()
to handle the upload, notice, and job polling flow.
PDF and DOCX were unintentionally left out of the original requirements. Rather
than add two parallel code paths, the parsing pipeline is refactored around a
format-neutral intermediate representation (Document\Ir) that every source
parser emits, so classification, layout analysis and block rendering are shared.

Document\ParserFactory is now the single place formats are declared: extension,
MIME type, Drive export behaviour and the noun the UI uses for one unit of
content. Adding a fourth format means editing one table.

PDF: a page maps to a slide. PDFs carry glyphs at coordinates and no structure,
so Pdf\TextExtractor rebuilds it — glyphs to lines by baseline, lines to blocks
by proximity, wrapped lines rejoined where the previous line reached the block's
right edge. Pdf\ImageExtractor walks the content stream's matrix stack, because
position is what separates real content from a logo repeated on every page.

DOCX: a heading-delimited section maps to a slide, splitting at the shallowest
heading depth that occurs more than once so a document whose only H1 is its
title splits on H2 instead. Tables and lists are supported; Docx\Numbering reads
word/numbering.xml directly, since PhpWord records which numbering definition a
list item belongs to but not whether it renders as a bullet or a counter.

Drive picker and upload endpoint accept all three formats; Google Slides and
Docs are exported to PPTX and DOCX, and files already stored in a supported
format are downloaded unchanged. Uploads are checked against the format's magic
bytes so a renamed file is rejected before it reaches a parser. The stored job
summary moves from pptx_path to source_path, falling back to the old key so
jobs queued before this change still resolve.

PPTX output is unchanged apart from three fixes the refactor made possible:
empty column wrappers are no longer emitted, numbered bullets render as <ol>,
and paragraphs in a title shape no longer run together. Monospace now marks a
paragraph as code only when the whole paragraph is monospaced, so a code font
used for a few words inside prose stays an inline <code> run.

Adds phpoffice/phpword and smalot/pdfparser. No new server requirements — both
need extensions phppresentation already did, or that ship with PHP by default.
The plugin had no documentation of its own. Covers requirements and setup
(encryption key, Google Cloud project, settings, capability), the import flow,
the REST surface, background jobs and troubleshooting.

The parser sections record why each format is handled the way it is — why the
PDF footer band is dropped, why monospace detection is conditional on the page's
dominant font, why DOCX sectioning descends a heading level — since those are
the decisions most likely to be "fixed" and regressed later. Also documents how
to add a fourth format, and which version metadata has to be kept in step.
The plugin had no tests and the repository had no PHP test framework. Codeception
is installed as a dev dependency of the plugin rather than of the root project,
so the plugin stays self-contained and the root install is unaffected. It was
chosen over bare PHPUnit because the outstanding phases need integration and
functional tiers too, and wp-browser plugs a WordPress-aware module set into the
same runner and configuration.

The Unit suite runs without WordPress: the parsing code touches only a handful
of helpers (escaping, slashes, translation, WP_Error), stubbed in
tests/Support/wordpress-stubs.php. That keeps it at ~0.4s with no database and
draws a clear line — anything needing real WordPress behaviour belongs in the
integration suite rather than a larger stub. Crypto, OAuthClient and the
LearnDash handoff are therefore left uncovered for now.

158 tests, 401 assertions, covering the IR heuristics, the format table,
classification precedence, every rendered block type, all three parsers against
committed fixtures, Word list-numbering resolution and the preview pipeline.
Includes regression guards for the two defects found while adding PDF and DOCX
support: shape objects leaking into the job summary, and a monospaced run inside
prose turning the whole paragraph into a code block.

Fixtures are synthetic and small (~26 KB), rebuilt by tests/_data/build-fixtures.php.
The DOCX and PDF are written as raw bytes because neither writer will produce
what is needed on demand: exact numbering definitions for ordered lists, and
per-glyph positioning with a footer-band image. The PDF draws no space
characters at all, so every space in the parsed output is reconstructed.

CorpusTest parses a directory of real documents and asserts only what must hold
for any input. Real decks are large and not ours to redistribute, so it is
opt-in and everything it reads is gitignored: tests/assets/ by default, or
CBF_SI_FIXTURE_DIR from the environment or the plugin's .env. PHP does not read
.env files itself, and Codeception's `params: - env` only feeds its own config
interpolation, so the helper parses .env via vlucas/phpdotenv — the library
Bedrock already uses at the repository root — and parses rather than populates
so a test run cannot leak settings into the wider process.

The plugin's minimum PHP moves 8.1 -> 8.5 to match the root project and the
Lando appserver. Composer's config.platform applies to dev dependencies as well
as runtime ones, so the old pin was holding the tooling several major versions
back. Declared in the four places the README's version-metadata table lists.

One trap worth knowing: every plugin file ends its ABSPATH guard with `exit`,
and Codeception 5 does not load tests/_bootstrap.php implicitly. Without a
bootstrap defining ABSPATH the first autoloaded class kills the run with no
error and exit code 125. The suite declares its bootstrap explicitly; any new
suite must do the same.

Codeception needs register_argc_argv=On, which the shared Lando php.ini turns
off for the web SAPI, so every entry point overrides it per-invocation rather
than weakening that setting.
Adds a Test Framework section covering the suite layout, the fixture strategy,
how to run the tests, corpus-directory resolution, and the silent-exit trap that
catches any suite missing an ABSPATH bootstrap.

Phase 7 is rewritten around what now exists: P7.0 (framework adoption) and P7.1
(unit suite) are closed, P7.3 is closed with the PHPCS count noted, and P7.2 is
restated as the WordPress-backed suite that will pick up Crypto, OAuthClient,
JobRunner and ConfigController — the four items the unit tier deliberately does
not stub. P7.9 and P7.10 are added for the REST functional suite and for wiring
the tests into CI.

The Validation Plan's unit-test list is replaced with the assertions actually
written, with the four deferred items marked as such rather than quietly
dropped. P6.5 is closed against the README, the feature summary and Done
Criteria now reflect PDF and Word support, and the Existing Tests finding is
corrected: it claimed no PHP test framework existed anywhere, which is no longer
true for this plugin.
google/apiclient 2.19 accepts only Guzzle 6 or 7. Bedrock's root vendor
has shipped Guzzle 8 since 2026-07-30 and its autoloader registers the
GuzzleHttp\ prefix before the copy bundled with this plugin, so every
call through DriveService throws

  LogicException: Could not find supported version of Guzzle

before it reaches the network.

The download and export paths survived this because they fall back to
wp_remote_get with the user's bearer token. The metadata lookup had no
such fallback, so it has been failing outright — unnoticed, because the
single-file flow takes its MIME type from the Drive Picker and only
reaches lookup_mime_type for jobs queued without one.

Route metadata through wp_remote_get as well, mapping Drive's HTTP
statuses onto messages an editor can act on. A 404 is reported as
"deleted, or in a workspace your account cannot reach", since Drive
hides existence from users without access.
Importing a course one file at a time does not scale: each deck needs a
picker selection, a configuration pass and a preview confirmation. A
batch takes one CSV of heading / session_id / type / title / url, plus a
course and an overwrite flag chosen once, and creates every session and
topic in it.

Upload validates without writing anything: the CSV is parsed, each URL
resolved against Drive, each topic's parent session checked against the
selected course, and duplicates flagged. The editor confirms that
pre-flight report before a single post is created. Rows then run one at
a time, each completion queueing the next, and a per-row report is built
as they finish and downloadable as CSV.

Three findings shaped the implementation:

Section headings are ordered by index into the course's lesson list, and
this site has shared course steps enabled — so LearnDash reads that list
from the ld_course_steps tree, not menu_order. The end-of-batch reorder
writes both, reloads the cached steps model first, and refuses to write
a tree missing lessons the batch created; a wrong order can be fixed by
hand, a detached lesson cannot.

Google Forms share the /d/FILE_ID/ URL shape with Slides and Docs, and
are the largest unimportable group in the real material. DriveUrl keys
on the path segment instead, so a Form is rejected with a reason naming
it rather than mistaken for a deck.

Batch jobs carry their config on the job summary rather than in the
configs table, so JobRunner falls back to it when config_id is NULL.
Without that, every batch lesson was created outside its course.

Verified end to end against the real curriculum sheet: 132 rows, 109
resolved with no permission failures, 23 rejected on source type.
The first full-scale batch died on its first row. Parsing is far hungrier
than the file on disk suggests: a 7.2 MB, 42-slide PPTX peaks at 452 MB,
because PhpPresentation holds every slide's object graph and every
embedded image in memory at once. A cron request gets 256 MB.

Two separate problems, and the second is why it looked like a hang.

Jobs now raise their own ceiling through wp_raise_memory_limit(), so the
value stays visible to a site owner through the cbf_si_job_memory_limit
filter rather than being buried in an ini_set(). Only one row runs at a
time, so peak usage is bounded by the largest single document.

run() catches exceptions, but memory exhaustion is not one. The worker
died mid-parse and the job sat in `parsing` with no error against it, so
the batch waited on a row that was never going to report — silent, and
indistinguishable from slow progress. A shutdown handler now inspects
error_get_last() and fails the row with a reason an editor can act on,
which releases the batch. It reserves 512 KB up front and frees it on
entry, because an out-of-memory kill otherwise leaves no room to record
what happened.

A worker killed outright still runs no handler, so the Janitor now
abandons a job after three stale resets instead of cycling it every 30
minutes forever.
*.sql was already ignored, but a compressed dump was not, so a 91 MB
pre-migration backup sitting in a plugin directory was staged by a
directory-wide git add.
Placement was a single pass at batch completion, on the reasoning that
one reorder is cheaper than many. Watching a real run made the cost of
that obvious. Section headings are created before any lesson exists, so
they take indices 0..n; until the pass runs, the builder shows one
heading per lesson with every remaining lesson piled under the last. It
reads as a placement bug, and a batch that is cancelled or abandoned
never runs the pass at all, so the course stays that way.

Rows are sequential, so there is no interleaving for a single pass to
protect against, and the pass is idempotent. Running it after each row
keeps the course correct at every point someone might look at it, and it
now also runs on cancel, for a row that reports after the batch stopped.

Separately, a skipped row gave dangerous advice. The importer matches an
existing post by title across the whole site rather than within the
target course, so two courses cannot both hold an "Introduction to Git" —
four rows in the first 29 hit this. The report said "Enable Overwrite to
update it", which would have rewritten the Full-Stack course's lessons
and moved them into Data Analytics. It now names the owning course and
says to retitle the row, and recommends Overwrite only where the clash
is one the target course already owns.

The site-wide match itself is unchanged; the CLI pipeline relies on it.
Shared course steps are enabled on this site, so a session is not owned
by one course — several bootcamps legitimately teach the same
"Introduction to Git". Treating a title match as a collision was wrong:
four rows in the first real run were skipped, each leaving a gap in the
migrated course for content that already existed and was meant to be
shared.

With shared steps on and Overwrite off, a matched title now adds the
existing session to the course as a shared step and reports the row as
`reused`, naming the courses it already belongs to. set_steps() performs
the attach, which is exactly what the course builder does when a step is
shared. Overwrite still wins where the editor asked for it, and where
shared steps are off the row is skipped with a message that names the
owning course and warns against Overwrite.

tree_is_complete() became attach_missing(). Refusing to write a tree
that was missing a lesson protected content but left the course
unordered, and made reuse impossible, since a shared session is by
definition not in the tree yet. Anything in the sequence that is not a
lesson still abandons the write, because that means a caller has gone
wrong and writing it would attach nonsense to the course.

Verified against the four real clashes: each session now belongs to both
courses, sits under the right heading in CSV order, and the Full-Stack
course it came from is unchanged at 36 lessons.
Feedback from the first full run, which completed 119 rows in about 25
minutes.

A batch finished as "completed with errors" whenever any row was
rejected. But a rejected row is one pre-flight identified and showed to
the editor before they pressed the button — a Google Form, a missing
link — so a batch that did exactly what it said it would was reported as
having gone wrong. Only a failure during the run counts now. The counts
still show the rejections; they just no longer colour the verdict.

The Detail column carried a sentence per row in a table of 119. The
"heading does not exist yet and will be created" notice is gone, since
the panel already lists every heading the import will add, and the reuse
explanation is now one clause. Removing the notice left BatchPlanner
with no use for the course's existing headings, so that read is gone
too.

The jobs table only refreshed once the batch ended, so a 25-minute run
sat there showing its first row as pending and then everything at once.
It now refreshes on the same poll as the batch panel.
A section heading's `order` is not an index into the course's lesson
list. LearnDash builds the list it displays by splicing each heading in
one at a time:

    $lessons = array_keys( $steps['sfwd-lessons'] );
    foreach ( $sections_array as $section ) {
        array_splice( $lessons, (int) $section->order, 0, array( $section ) );
    }

Every splice shifts what follows, so by the time the Nth heading is
inserted the N before it already occupy slots. `order` indexes that
part-built list, and must count the headings as well as the lessons.

Writing bare lesson counts put each heading one position too early per
heading before it, so the last lesson of every section rendered under
the next one. In the first full migration that was 28 of 109 lessons:
Course Onboarding showed 3 of its 4, and Professional Skills collected
the 7 that had drifted off the end of everything above it.

open_sections_up_to() subtracts the same offset, since reading
membership back has to undo exactly what writing it does.

Nothing caught this, and the reason is worth recording. Verification
re-derived the grouping using the same mistaken rule that wrote it, so a
course with 28 misplaced lessons verified as correct twice. section_order()
is now pure and SectionOrderTest checks it against a replica of the
splice, asserting both that the correct rule groups as intended and that
the old one does not.
The jobs table still did not refresh during a batch, and the reason was
not the code. Assets are served with a one-year max-age and versioned by
the VERSION constant, which only moves at release, so the edited script
kept its ?ver=1.1.0 and browsers went on serving the previous file. The
fix had been correct on the server for a day.

Append the file's filemtime to the version, so the URL changes whenever
the file does while the query argument still names the release it came
from. Applies to stylesheets for the same reason, and defers to any
version a caller passes explicitly.
The plugin had two entries in the LearnDash menu. The second, "Slides
Importer", led to the Google credentials screen — an administrator's
screen sitting in a menu mostly used by editors, who could see it and
not open it.

Register that screen without a menu entry and reach it from a tab bar
instead. Both screens now head with "Slides Importer" and carry Import
and Settings tabs, using core's nav-tab-wrapper — the markup themes.php
and Site Health use — so the tabs inherit admin styling and stay correct
through colour-scheme changes rather than carrying CSS of their own.

The bar renders only when the viewer can reach both screens: an editor
with cbf_slides_import but not manage_options sees no tabs, rather than
a single tab or a link that would refuse them.

Registering under LearnDash and then calling remove_submenu_page() is
deliberate. It only takes the item out of $submenu, so the screen stays
registered and routable, and it keeps the capability check that
add_submenu_page() performs. A submenu_file filter keeps Import
Documents highlighted while the settings screen is open, which would
otherwise leave the LearnDash menu with nothing marked current.
Removing the settings link with remove_submenu_page() locked
administrators out of the screen it pointed at.

$submenu is what get_admin_page_parent() searches to decide which menu a
screen belongs to, so taking the entry out left the parent unresolvable.
add_submenu_page() had registered the screen as

    learndash-lms_page_cbf-slides-importer-settings

but with no parent to resolve, user_can_access_admin_page() looked for

    admin_page_cbf-slides-importer-settings

found nothing, and denied access to everyone.

Register it with an empty parent instead, which is how a screen is meant
to be given no menu entry: the hook it registers and the hook the check
looks for are then the same. The capability still gates it, through the
$_wp_submenu_nopriv entry add_submenu_page() records for a user who
lacks it — verified as allowed for an administrator and denied for an
editor holding cbf_slides_import.

A screen with no entry also leaves the sidebar with no menu open, so
parent_file and submenu_file are filtered to keep LearnDash expanded
with Slides Importer highlighted.

The remaining menu item is renamed to "Slides Importer", matching the
heading both screens now share.
…reen

The settings screen loaded but left the sidebar collapsed, with Slides
Importer highlighted only once the menu was opened by hand.

menu-header.php applies the parent_file filter on line 43 and then calls
get_admin_page_parent() on line 55, which sets $parent_file as a side
effect. The screen was registered with an empty parent, so that lookup
found it under $submenu[''] and overwrote the filtered value with '' —
after the filter had already run, and with no hook in between. The
submenu_file filter survived, which is why the item highlighted while
its menu stayed shut.

get_admin_page_parent() only clears $parent_file when it finds a match;
with no match it leaves a value already set. So drop the entry from
$submenu[''] once registration is done. Nothing rendered it — there is
no menu with an empty slug — and removing it is safe here in a way it
was not under a real parent, because the hook name registered comes from
the same unresolvable parent the access check derives its own from, so
the two still agree.

Verified on authenticated requests rather than by inspecting globals.
Both screens render, the LearnDash menu carries wp-menu-open, exactly
one item appears for the plugin and it is marked current, and the right
tab is active on each. An editor holding cbf_slides_import still gets a
403 on the settings screen.
The plugin has no phpcs config of its own, so `composer lint:all` at the
root now checks it against the project standard and found twelve errors.

Nine functions exceeded the complexity ceiling of 6. Each is split along
a seam it already had rather than shuffled to please the sniff:

- SlideClassifier: deciding a user override and matching a layout name
  were two separate questions inside one classifier.
- AuthController: everything the state parameter has to prove — format,
  user, capability, nonce — moves into one function that either returns
  an authorised user ID or ends the request.
- Pptx\Parser: recognising a content image and writing one to disk.
- OAuthClient: refreshing an expired token, including carrying the
  refresh token forward that Google only issues once.
- Main: collecting unmet requirements, separate from acting on them.
- PreviewRenderer, SettingsPage, JobRunner: config resolution, credential
  validation, payload parsing, and preview caching pulled out of the
  functions that were doing several jobs at once.

Two short ternaries become explicit is_array() checks, and the
interpolated-SQL ignore in Janitor moves onto the line it applies to —
phpcs:ignore only covers the following line, and the query it was meant
for started two lines further down.

No behaviour changes. Verified beyond the unit suite, which cannot reach
most of this: the requirements check, credential validation across five
inputs, and a full parse through the refactored pipeline, which produced
7 entries classified 1 cover / 6 body, cached its preview and stored the
summary as before.
The Guzzle 8 problem reached further than the Drive metadata call.
fetchAccessTokenWithRefreshToken() and fetchAccessTokenWithAuthCode()
build their transport through the same client, so both raised

  LogicException: Could not find supported version of Guzzle

which meant an expired access token could never be refreshed and a
re-authorisation could never complete. The plugin worked only for the
hour following each sign-in.

Nothing caught it because nothing had run an hour after signing in. The
119-row migration took 25 minutes; every check before that was made
minutes after authorising. It surfaced while smoke-testing an unrelated
refactor, on the first call made against a genuinely stale token.

Both grants now post to Google's token endpoint over wp_remote_post.
The response is stamped with `created`, which the client sets when it
does this itself and which expiry is measured from — without it every
call would treat the token as expired and mint another. The redirect URI
is read from one place, since Google requires the consent URL and the
code exchange to send byte-identical values. A failed refresh keeps
Google's reason but pairs it with the action that resolves it, rather
than reporting "Bad Request" to an editor.

Verified against a real expired token: refreshed, persisted with its
refresh token carried forward, authenticated a Drive call, and left
alone by the following call rather than refreshed again.
- Fix test stubs loading
- Fix linting
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.

1 participant