Prototype: Notion to Cortext importer#275
Conversation
- ImportPane: drop the .then in importCollection. runImport's onProgress already fires on the terminal tick with status === 'done', so the progress handler covers the same state transition the .then was applying. Only the .catch is load-bearing. - notionImport: stop exporting startImport / tickImport. They're only called by runImport in the same module; runImport is the one public entry point. JSDocs shrunk to one-line internal comments to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- New Cortext\Notion\Client wraps wp_remote_request: get/post/paginate,
mirrors upstream status codes back as WP_Error, casts empty POST
bodies to {} so Notion accepts them.
- New Cortext\Rest\NotionController owns three routes:
GET /cortext/v1/notion/collections (replaces the generic proxy)
POST /cortext/v1/notion/import/start
POST /cortext/v1/notion/import/{id}/tick
/collections returns { id, title } per data source; the client never
sees Notion's raw payload anymore.
- Drop NotionImportController (generic /notion/proxy) and
NotionImportJobsController; their plumbing constants and Notion HTTP
helpers folded into Client.
- notionImport.js shrinks from ~250 to ~65 lines: no notion() wrapper,
paginate, slugify, fetchAllDataSources, transformField/Schema, or
rate-limit retry. Three apiFetch wrappers + runImport.
- Dead fields/slug/parent_database_id removed from the wire format
(confirmed unused by the UI after the panel removal); the JS↔PHP
schema-mapping duplication that justified transformField goes away.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- New POST /cortext/v1/notion/import/{job_id}/finish drops the
cortext_notion_import_{job_id} option once the client has observed
the terminal state. Returns 404 if already gone (idempotent from
the client's POV) and 409 if the job is still running, so a
misfired finish can't discard mid-flight cursor state.
- runImport calls finishImport best-effort after the tick loop exits;
errors are swallowed because they don't change the import outcome.
The failure path still leaks: a thrown tick (or a tab close) never
runs finishImport, so its option lingers. A bulk-purge sweep over
old cortext_notion_import_* records is the natural complement, and
is tracked in the PR's To-do.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Detect ticks where processed count fails to advance while the server still reports has_more, and abort after MAX_STALLED_TICKS (2) consecutive stalls. One stalled page can happen legitimately (e.g. a Notion view returning zero results on a page); two in a row is the runaway-loop signal. The thrown error propagates through the existing .catch in importCollection (ImportPane), so the card surfaces it. The job record is not finished on the failure path; the bulk-purge follow-up will cover it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e9b1f8f to
d3a2b9e
Compare
- Trim some more. - Mix in the PR description. - Unwrap lines.
priethor
left a comment
There was a problem hiding this comment.
I don't love that NotionImporter.php handles posts directly in the database; it shouldn't know the details of the data structure.
This highlights one of the pain points I was trying to address with the document unification, but it also reveals a missing gap: we don't have a full PHP API that unifies creation.
I think it's ok to merge since #280 is ready, too, and it will break these insertions anyway. I'll add a Documents::save() and Fields::Save() methods to both decouple the importer and ease the merge with the new data layer.
| $status = (int) wp_remote_retrieve_response_code( $response ); | ||
| $json = json_decode( wp_remote_retrieve_body( $response ), true ); | ||
|
|
||
| if ( $status < 200 || $status >= 300 ) { |
There was a problem hiding this comment.
What happens if we hit limit rates?
There was a problem hiding this comment.
Good point. Pushed fbe7e11, though it only covers rows and not collections themselves. But I think this is fine for now: are there really that many collections in a connection that we see ourselves hitting 429 in extractCollections?
There was a problem hiding this comment.
Note that the FIXMEs in that commit are removed by the subsequent commit, but I made a point to keep them so that anyone could see it in action easily:
cortext-import-retry-after.mov
Yeah, I get that. I started out the whole thing by decoupling as much as possible, essentially defining an intermediate data representation between the Notion API and Cortext's underlying primitives. But, through iteration and as the flow became more concrete, that got more in the way, and it became natural to let the code stay closer to the metal.
Good to know, that sounds like it could be a better abstraction to rely on. |
Other than |
With the type for the changelog automation, it should be fine; areas are optional nice-to-haves. Let's add them as we see fit, in this case, I don't think it makes sense in the collection area, as it touches content in general; it would make more sense in an import(export?)-related area if we needed. |
cortext-importer.mp4
Summary
Part of #87.
Adds a new Import screen which, given a Notion API key ("connection token", in Notion parlance), will list all Notion databases accessible to that key. The user can then choose which collections to import to Cortext.
Implementation highlights
The client stores the Notion token in
localStorage, but all request to the Notion API are handled by the server, mediated byCortext\Notion\Client. This mediator understands Notion's response propertieshas_moreandnext_cursorin order to make serial requests until all paginated results have been fetched.A new REST controller,
Cortext\Rest\NotionController, can be divided in two tasks:Discovering available databases in Notion (
/notion/collections), which consists of querying the Notion API repeatedly fordata_sourceobjects.Importing a database by fetching pages of a database in batches, creating Cortext rows with each new fetched batch. This is achieved using three endpoints (
notion/import/start,notion/import/tick,notion/import/finish), explained below.Batching architecture
To minimise the complexities of server-side async jobs, imports are orchestrated by the client.
Once an import starts, the client calls the
startendpoint. It will then keep calling thetickendpoint until the server signals that there are no more rows to import, at which point the client will call thefinishendpoint.Throughout this process, the client doesn't see any actual Notion data. It's merely acting as a scheduler for the server, instructing it to process the import the next batch of rows. In turn, each
tickresponse contains a count of rows processed so far, which we can feed into the UI to show progress. All this is encapsulated inrunImport.Meanwhile, the server relies on the three endpoints to 1) create, 2) update, then 3) delete a WordPress option —
cortext_notion_import_{$uuid}— that represents the ongoing import job. This option contains all the necessary data for each new tick to query the Notion API with the right cursor data.Import fidelity
The actual fidelity (parity?) of the imported data needs to be tested with real data before we can assess how good it is. Some new Notion-related metadata is attached to the newly created entries, so that we can follow up later. For example,
cortext_notion_data_source_idshould be helpful once we try to import relations. SeeCortext\Notion\Importer.Missing user flows
It is cumbersome to create a Connection in Notion and manage with databases are visibile through it. So we should explore ways to make this more obvious to novice users. It could be as prosaic as providing some screenshots and linking to the Notion documentatin.
Test plan