Implement subject creation with existing schema

[alistair3149]

For #427

Recording.2026-01-28.192928.mp4

Follow-ups:

• Implement edit summary for subject creation
• Implement back button to go back to the last action

[alistair3149]

I ran into some circular dependency issue when SubjectWithContext extend Subject before Subject was defined.

[malberts]

Where did this happen? And what was the error? It's not immediately obvious why moving PropertyName, which does not reference either of those, would solve this. Especially when this class is still being exported in the original file.

[alistair3149]

When the SubjectWithContext class is being imported, the Subject class is still undefined because it is waiting on other modules to be loaded. Here's the circular dependencies going on:

1. SubjectWithContext
   Imports and extends Subject
2. Subject
   Imports StatementList
3. StatementList
   Imports PropertyName from PropertyDefinition
4. PropertyDefinition
   Imports Neo for createPropertyDefinitionFromJson.
5. Neo
   Imports SubjectDeserializer.
6. SubjectDeserializer
   Imports SubjectWithContext
7. SubjectWithContext
   (Cycle closes here)

There are probably better way other than extracting PropertyName to its own class.
It was done to minimize the change to the core logic and get the subject creation flow working.

Error log from Vitest:

 FAIL  tests/components/SubjectCreator/SubjectCreator.spec.ts [ tests/components/SubjectCreator/SubjectCreator.spec.ts ]
TypeError: Class extends value undefined is not a constructor or null
 ❯ src/domain/SubjectWithContext.ts:7:41
      5| import type { PageIdentifiers } from '@/domain/PageIdentifiers';
      6| 
      7| export class SubjectWithContext extends Subject {
       |                                         ^
      8| 
      9|  public constructor(
 ❯ src/persistence/SubjectDeserializer.ts:4:31

[JeroenDeDauw]

Why is this issue relevant here?

I see:

• SubjectWithContext is not used in this PR
• master branch builds fine

[JeroenDeDauw]

The change does not seem harmful, so let's just ignore the thing for now. We can merge that as-is and spend our time on other things

[alistair3149]

It comes from the test for SubjectCreator. I have dropped it for now and we can revisit it when the component gets more stable.

[alistair3149]

Follow-up: Add $i18n mock to VueTestHelpers

[Copilot]

Pull request overview

Implements the initial “create subject” flow using an existing schema, introducing a dialog-driven UI and wiring it into the app, plus a small domain refactor around PropertyName.

Changes:

• Added SubjectCreatorDialog + SubjectCreator + SchemaLookup components to start subject creation from existing pages and select an existing schema.
• Added Pinia store support for initializing a new subject (initNewSubject) and corresponding component tests.
• Extracted PropertyName into its own domain module and updated imports/usages accordingly.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
resources/ext.neowiki/src/components/SubjectCreator/SubjectCreatorDialog.vue	Hosts the trigger button + creation dialog and transitions into SubjectEditorDialog.
resources/ext.neowiki/src/components/SubjectCreator/SubjectCreator.vue	UI for choosing “existing vs new schema” and emitting a created subject for editing.
resources/ext.neowiki/src/components/SubjectCreator/SchemaLookup.vue	Codex lookup for searching/selecting schemas via SchemaStore.
resources/ext.neowiki/src/stores/SubjectStore.ts	Adds initNewSubject to preload schema + construct an initial Subject.
resources/ext.neowiki/src/domain/Subject.ts	Adds Subject.createNew helper for new-subject construction.
resources/ext.neowiki/src/domain/PropertyName.ts	New dedicated PropertyName value object.
resources/ext.neowiki/src/domain/PropertyDefinition.ts	Removes inline PropertyName and re-exports it from the new module.
resources/ext.neowiki/src/domain/Statement.ts	Updates PropertyName import to new module.
resources/ext.neowiki/src/domain/StatementList.ts	Updates PropertyName import to new module.
resources/ext.neowiki/src/persistence/StatementDeserializer.ts	Updates PropertyName import to new module.
resources/ext.neowiki/tests/VueTestHelpers.ts	Extends MediaWiki mock to support mw.config.get(...).
resources/ext.neowiki/tests/components/SubjectCreator/SubjectCreatorDialog.spec.ts	Tests dialog open/transition-to-editor and save hooks.
resources/ext.neowiki/tests/components/SubjectCreator/SubjectCreator.spec.ts	Tests schema mode toggle and subject initialization on schema selection.
resources/ext.neowiki/tests/components/SubjectCreator/SchemaLookup.spec.ts	Tests schema search wiring and exposed focus method.
i18n/en.json	Adds lookup placeholder message.
extension.json	Registers CdxLookup and loads new i18n key.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[malberts]

Where did this happen? And what was the error? It's not immediately obvious why moving PropertyName, which does not reference either of those, would solve this. Especially when this class is still being exported in the original file.

[malberts]

Need to think about this more. The returned object must be compatible with Subject, since that's being passed into SubjectEditorDialog. Although, it does not seem like the Id is actually being used there.

If NewSubject extends Subject, similar to SubjectWithContext, then we're still passing around a temporary Id, but it would just be hidden here.

[JeroenDeDauw]

Yes, more thinking is needed here.

Currently we generate IDs on the backend.

Some relevant TS:

export interface SubjectRepository extends SubjectLookup {

	createMainSubject(
		pageId: number,
		label: string,
		schemaName: SchemaName,
		statements: StatementList
	): Promise<SubjectId>;

SubjectEditor.vue:

const props = defineProps<{
	schemaStatements: StatementList;
	schemaProperties: PropertyDefinitionList;
}>();

TL;DR:

A Subject is not required in SubjectEditor or in the call to the API that creates a Subject.

[JeroenDeDauw]

Let's call this SchemaSelector or similar. It's different from the Lookup services we have, and the point of the component is to let the user choose a subject. Perhaps SubjectChooser?

[malberts]

I assume the last one was meant to be SchemaChooser.

[alistair3149]

It was picked to align with what Codex called the Lookup component. SchemaChooser sounds fine to me too. What do you think?

[JeroenDeDauw]

The key issue here is using the SubjectEditorDialog.

SubjectCreatorDialog is on the same level as SubjectEditorDialog. They should both use SubjectEditor. This fixes the ID related issues.

Ways in which the two dialogs differ:

• Creation flow has Schema selection step
• Creation flow has required label input. Edit flow currently just shows label
• "Create Subject" vs "Save Subject" button
• call to createMainSubject in SubjectRepository vs call to the SubjectStore
• potentially more stuff later

We figured this out at some point (when we did the refactor that got us SchemaEditor and SchemaEditorDialog), but with the break in the project this unfortunately fell out of my context window and was thus not included on the issue.

[JeroenDeDauw]

I spent some time considering adjacent topic: do we stick to generating IDs on the backend?

I was wondering about that recently as for ECHOLOT this might not be the right move, and our ID format allows distributed generation.

Conclusion: for the UI creation flow we do not change it. What I wrote in my previous comment remains entirely applicable

Reasoning: We will change it later, which will require a simple UI change, which will be not any more complicated then than it is now. Changing it now does not help us much and is a bunch more work over the codebase. We would need to implement ID generator in TS and/or ID generator API endpoint (this seems useful for ECHOLOT. Esp: get 100 new IDs for my batch import UI/tool), and do some refactoring. Later is better.

[malberts]

I spent some time considering adjacent topic: do we stick to generating IDs on the backend?

It would still be nice for new Subject creation API to not require an ID.

get 100 new IDs for my batch import UI/tool

How would we enforce the use of the returned IDs? The local tool could then also just pick its own pattern and increment: sfoo00000000001 -> sfoo00000000100 (or do its own random thing). This increases the risk of a collision, because the timestamp and random components are absent. I don't think we can avoid this, though, so if you decide to use your own, then you will have to handle it on the tool side (e.g., check if ID exists first, or else get all the failed creations, generate again, create again).

When the backend generates the ID, it contains a chronologically correct timestamp component, which is supposed to be sortable (ADR 14). If we allow passing in custom IDs, then the timestamp component can be manipulated. At this point, what is the use of sortability, when the encoded timestamp cannot be guaranteed to be correct at the time of generation? Perhaps there needs to be additional validation. Or perhaps a user actually wants this ability to "backdate" an ID? I think we also wanted to allow custom "human readable" IDs like smypersonalid42. I imagine this is not a strict requirement of the ID format, but then it seems like the timestamp is more for avoiding collisions than actually being useful for sorting. So technically you can "sort" any set of IDs, but the timestamp component should not be assumed to actually carry any meaning. (We can already create chronologically incorrect IDs via direct JSON edit, e.g. demo data, and it does not break anything, but that is not part of the normal workflow.)

Lastly, why would you need a 100 pre-generated IDs? Is the primary use case to create new subjects, with relations to also new subjects, in a single API call? Otherwise, a batch create API could just contain a 100 ID-less subjects.

[JeroenDeDauw]

We don't need to enforce usage of IDs. Every nanosecond an entire ID range is gone, unused forever.

Attackers can create collisions if they can provide their own IDs on creation, at which point all that happens is they make their API request fail. Attackers can also put IDs with incorrect timestamps. Not a huge deal and easy to mostly mitigate by rejecting future IDs and only accepting new IDs up to a certain age old. That still leaves them able to slightly mess with ordering during bulk insert, but so what? They could just insert them in a different order and get the same result.

I think we also wanted to allow custom "human readable" IDs like smypersonalid42.

I do not recall this and do not think there is value in supporting that. It's not not what is decided by ADR 14, which I see no reason to revisit.

[JeroenDeDauw]

Why bulk ID gen: because your bulk Subject import can contain references to other Subjects you are importing. If we generate IDs only on subject creation, then bulk subject import gets complex. This is the main motivator for allowing clients to generate IDs, be it via their own code, or by calling a new API endpoint we provide.

[alistair3149]

SubjectCreatorDialog is on the same level as SubjectEditorDialog. They should both use SubjectEditor. This fixes the ID related issues.

UI-wise, that is an eventual state I want to get to, where we are substituting the content within the dialog instead of opening a new dialog for flows that requires multiple editors:

• SubjectCreator -> SubjectEditor
• SubjectCreator -> SchemaEditor -> SubjectEditor
• SubjectEditor -> SchemaEditor

We can follow up on that in a later PR.

[JeroenDeDauw]

@alistair3149 not sure you fully understood, or if I am not understanding what you are getting at.

Note that SubjectEditorDialog and SubjectEditor are not the same. I'm saying SubjectCreatorDialog should use SubjectEditor but not SubjectEditorDialog. Both the dialogs should use SubjectEditor which has the common behavior, while the dialogs themselves take care of the workflow-specific things and thus encode the differences I listed in yesterdays review.

[alistair3149]

@alistair3149 not sure you fully understood, or if I am not understanding what you are getting at.

Note that SubjectEditorDialog and SubjectEditor are not the same. I'm saying SubjectCreatorDialog should use SubjectEditor but not SubjectEditorDialog. Both the dialogs should use SubjectEditor which has the common behavior, while the dialogs themselves take care of the workflow-specific things and thus encode the differences I listed in yesterdays review.

I understand what you are getting at. Should I follow that up in another PR or should we update this PR?

[JeroenDeDauw]

Let's fix it in a follow up, allowing parallelization of follow ups