Skip to content

Commit 70cc1a7

Browse files
JeroenDeDauwclaude
andcommitted
Make the deep-link fragment the bare Subject ID
Per review, the Data tab URL fragment is the bare Subject id (`#sEpfwJLAtndCccC`), like Wikibase's `#P123`, not the row's DOM id. The fragment never needed to equal a DOM id — the mount handler does the expand/scroll/highlight itself and the highlight is class-based — so fragment and DOM id decouple. The row DOM id stays `ext-neowiki-subject-row-{id}` as an internal rendering detail. - Dereference endpoint (`data-tab`): the Location fragment is now `#{subjectId}`. - Data tab: `applyHash` reads the fragment through a bare-id parser (`subjectIdFromHash`); manual expand writes `#{subjectId}` via `history.replaceState`. - Removed `SubjectRowAnchor` and its literal-pin test: with the fragment being the id itself there is no prefix scheme to keep in lockstep across languages. The handler appends `'#' . $subjectId`; a handler test asserts the Location ends with the bare id, and a vitest asserts the parser accepts exactly subject-id-shaped fragments (and rejects the old long form — no legacy acceptance). - The dereference tests pin a `page` baseline in setUp so the Accept-negotiation tests are independent of any ambient $wgNeoWikiSubjectDereferenceTarget; the now-redundant explicit page-target test is dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4a85db9 commit 70cc1a7

8 files changed

Lines changed: 57 additions & 112 deletions

File tree

docs/rdf/rdf-export.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ TriG wins when both RDF types are acceptable; the RDF redirects use the native p
108108
on a page the caller may not read returns one indistinguishable `404`; a malformed id `400`.
109109

110110
The HTML target is the Subject's hosting page by default, or that page's Data tab (`?action=subjects`) opened on the
111-
Subject's row when `$wgNeoWikiSubjectDereferenceTarget` is `data-tab`.
111+
Subject's row (`#{subjectId}`) when `$wgNeoWikiSubjectDereferenceTarget` is `data-tab`.
112112

113113
The negotiator is always reachable at the REST path, which needs no server configuration:
114114

resources/ext.neowiki/src/components/SubjectsManager/SubjectsManagerPage.vue

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ import { useSubjectStore } from '@/stores/SubjectStore.ts';
467467
import { useSchemaStore } from '@/stores/SchemaStore.ts';
468468
import { useSubjectPermissions } from '@/composables/useSubjectPermissions.ts';
469469
import { useSubjectDrag } from '@/composables/useSubjectDrag.ts';
470-
import { subjectRowDomId, subjectIdFromRowDomId } from '@/presentation/subjectRowDomId.ts';
470+
import { subjectRowDomId, subjectIdFromHash } from '@/presentation/subjectRowDomId.ts';
471471
import { Subject } from '@/domain/Subject';
472472
import { Schema } from '@/domain/Schema';
473473
import { SubjectId } from '@/domain/SubjectId';
@@ -663,10 +663,11 @@ function toggleExpanded( id: string ): void {
663663
next.delete( id );
664664
} else {
665665
next.add( id );
666-
// Make the address bar a shareable deep link to the row the user just opened. replaceState (not a
667-
// location.hash assignment) adds no history entry and fires no hashchange, so it does not
668-
// re-trigger applyHash. Collapsing deliberately leaves the fragment in place.
669-
history.replaceState( null, '', '#' + subjectRowDomId( id ) );
666+
// Make the address bar a shareable deep link to the row the user just opened. The fragment is the
667+
// bare Subject id (like Wikibase's `#P123`); replaceState (not a location.hash assignment) adds no
668+
// history entry and fires no hashchange, so it does not re-trigger applyHash. Collapsing
669+
// deliberately leaves the fragment in place.
670+
history.replaceState( null, '', '#' + id );
670671
}
671672
expandedIds.value = next;
672673
}
@@ -867,7 +868,7 @@ async function executeDelete( comment: string ): Promise<void> {
867868
}
868869
869870
function applyHash(): void {
870-
const id = subjectIdFromRowDomId( window.location.hash.slice( 1 ) );
871+
const id = subjectIdFromHash( window.location.hash.slice( 1 ) );
871872
if ( id === null ) {
872873
return;
873874
}
Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,17 @@
11
import { SubjectId } from '@/domain/SubjectId';
22

3-
// The DOM id — and, identically, the URL fragment — of a Subject's row on the Data tab
4-
// (?action=subjects). The subject-IRI dereference endpoint mints `#<domId>` into its Location when the
5-
// wiki targets the Data tab, and the Data tab reads the fragment back to expand, scroll to, and
6-
// highlight the row.
7-
//
8-
// Single source of truth for this scheme on the TypeScript side. Its PHP counterpart is SubjectRowAnchor
9-
// in src/Presentation/SubjectRowAnchor.php; the two must stay in lockstep. Each side has a test asserting
10-
// the same literal example, so a prefix change on one side alone breaks a test.
3+
// The DOM id of a Subject's row on the Data tab (?action=subjects). Internal rendering detail — HTML
4+
// hygiene and in-page scroll targeting — not a public contract. A row's public deep-link anchor is the
5+
// bare Subject id in the URL fragment (see subjectIdFromHash), which the mount handler resolves to a row
6+
// itself, so the fragment and this DOM id are decoupled.
117
const ROW_ID_PREFIX = 'ext-neowiki-subject-row-';
128

139
export function subjectRowDomId( subjectId: string ): string {
1410
return ROW_ID_PREFIX + subjectId;
1511
}
1612

17-
// The Subject id carried by a row DOM id (or the equivalent URL fragment), or null when the string is
18-
// not one of ours: it lacks the prefix, or the part after the prefix is not a valid Subject id. Callers
19-
// treat null as "nothing to do" (e.g. an unrelated fragment leaves the Data tab untouched).
13+
// The Subject id encoded in a row DOM id, or null when the string is not one of ours. Reads the Subject
14+
// off a dragged row element, whose id subjectRowDomId built.
2015
export function subjectIdFromRowDomId( domId: string ): string | null {
2116
if ( !domId.startsWith( ROW_ID_PREFIX ) ) {
2217
return null;
@@ -26,3 +21,11 @@ export function subjectIdFromRowDomId( domId: string ): string | null {
2621

2722
return SubjectId.isValid( subjectId ) ? subjectId : null;
2823
}
24+
25+
// The Subject id carried by a Data tab URL fragment, or null when the fragment is not a Subject id (an
26+
// unrelated anchor, or junk — either leaves the Data tab untouched). The dereference endpoint's data-tab
27+
// Location and manual row expansion both write a bare Subject id as the fragment, like Wikibase's
28+
// `#P123`; this reads it back. Deliberately the Subject id, not the row DOM id.
29+
export function subjectIdFromHash( hash: string ): string | null {
30+
return SubjectId.isValid( hash ) ? hash : null;
31+
}
Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,50 @@
11
import { describe, it, expect } from 'vitest';
2-
import { subjectRowDomId, subjectIdFromRowDomId } from '@/presentation/subjectRowDomId';
2+
import { subjectRowDomId, subjectIdFromRowDomId, subjectIdFromHash } from '@/presentation/subjectRowDomId';
33

44
const VALID_ID = 's12345abcdefghj';
55

6-
describe( 'subjectRowDomId', () => {
6+
// The internal row DOM id (rendering + reading the Subject off a dragged row), not a public contract.
7+
describe( 'subjectRowDomId / subjectIdFromRowDomId', () => {
78

8-
// The same literal example is asserted by the PHP counterpart
9-
// (tests/phpunit/Presentation/SubjectRowAnchorTest.php). Keep them identical so a prefix change on
10-
// either side breaks a test.
11-
it( 'prefixes the subject id', () => {
12-
expect( subjectRowDomId( VALID_ID ) ).toBe( 'ext-neowiki-subject-row-s12345abcdefghj' );
13-
} );
14-
15-
} );
16-
17-
describe( 'subjectIdFromRowDomId', () => {
18-
19-
it( 'round-trips a row DOM id back to its subject id', () => {
9+
it( 'round-trips a subject id through its row DOM id', () => {
2010
expect( subjectIdFromRowDomId( subjectRowDomId( VALID_ID ) ) ).toBe( VALID_ID );
2111
} );
2212

23-
it( 'returns null for a bare subject id without the row prefix', () => {
13+
it( 'returns null for a string without the row prefix', () => {
2414
expect( subjectIdFromRowDomId( VALID_ID ) ).toBeNull();
2515
} );
2616

2717
it( 'returns null when the prefix is present but the body is not a valid subject id', () => {
2818
expect( subjectIdFromRowDomId( subjectRowDomId( 'not-a-valid-id' ) ) ).toBeNull();
2919
} );
3020

31-
it( 'returns null for the prefix with an empty body', () => {
32-
expect( subjectIdFromRowDomId( 'ext-neowiki-subject-row-' ) ).toBeNull();
33-
} );
34-
35-
it( 'returns null for an unrelated fragment', () => {
36-
expect( subjectIdFromRowDomId( 'section-heading' ) ).toBeNull();
37-
} );
38-
3921
it( 'returns null for a same-length prefix that is not ours', () => {
4022
// 'ext-neowiki-subject-ROW-' is 24 characters like the real prefix but differs in case, so a
4123
// naive slice without a prefix check would wrongly extract a valid id from it.
4224
expect( subjectIdFromRowDomId( 'ext-neowiki-subject-ROW-' + VALID_ID ) ).toBeNull();
4325
} );
4426

45-
it( 'returns null for an empty string', () => {
46-
expect( subjectIdFromRowDomId( '' ) ).toBeNull();
27+
} );
28+
29+
// The public deep-link contract: a Data tab URL fragment is a bare Subject id (like Wikibase's #P123).
30+
describe( 'subjectIdFromHash', () => {
31+
32+
it( 'accepts a bare subject-id-shaped fragment', () => {
33+
expect( subjectIdFromHash( VALID_ID ) ).toBe( VALID_ID );
34+
} );
35+
36+
it( 'returns null for a fragment that is not a subject id', () => {
37+
expect( subjectIdFromHash( 'section-heading' ) ).toBeNull();
38+
} );
39+
40+
it( 'returns null for an empty fragment', () => {
41+
expect( subjectIdFromHash( '' ) ).toBeNull();
42+
} );
43+
44+
it( 'does not accept the internal row DOM id form as a fragment', () => {
45+
// The old scheme put the row DOM id in the fragment; the contract is now the bare Subject id, and
46+
// the long form must not be silently accepted.
47+
expect( subjectIdFromHash( subjectRowDomId( VALID_ID ) ) ).toBeNull();
4748
} );
4849

4950
} );

src/EntryPoints/REST/ResolveSubjectIriApi.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use ProfessionalWiki\NeoWiki\EntryPoints\Actions\SubjectsAction;
1515
use ProfessionalWiki\NeoWiki\NeoWikiExtension;
1616
use ProfessionalWiki\NeoWiki\Presentation\SubjectDereferenceTarget;
17-
use ProfessionalWiki\NeoWiki\Presentation\SubjectRowAnchor;
1817
use Wikimedia\ParamValidator\ParamValidator;
1918

2019
/**
@@ -104,9 +103,8 @@ private function hostingPageRedirect( string $subjectId, PageIdentifiers $hostin
104103
private function hostingPageUrl( Title $title, string $subjectId ): string {
105104
if ( $this->dereferenceTarget() === SubjectDereferenceTarget::DataTab ) {
106105
// The Data tab reads this fragment on mount to expand, scroll to, and highlight the row. The
107-
// fragment is the row's DOM id; {@see SubjectRowAnchor} is the PHP source of truth for that scheme.
108-
return $title->getCanonicalURL( [ 'action' => SubjectsAction::ACTION_NAME ] )
109-
. '#' . SubjectRowAnchor::domId( $subjectId );
106+
// fragment is the bare Subject id (like Wikibase's `#P123`), not the row's internal DOM id.
107+
return $title->getCanonicalURL( [ 'action' => SubjectsAction::ACTION_NAME ] ) . '#' . $subjectId;
110108
}
111109

112110
return $title->getCanonicalURL();

src/Presentation/SubjectRowAnchor.php

Lines changed: 0 additions & 26 deletions
This file was deleted.

tests/phpunit/EntryPoints/REST/ResolveSubjectIriApiTest.php

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
use ProfessionalWiki\NeoWiki\Domain\Schema\SchemaName;
1414
use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectLabel;
1515
use ProfessionalWiki\NeoWiki\EntryPoints\REST\ResolveSubjectIriApi;
16-
use ProfessionalWiki\NeoWiki\Presentation\SubjectRowAnchor;
1716
use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject;
1817
use ProfessionalWiki\NeoWiki\Tests\NeoWikiIntegrationTestCase;
1918
use ProfessionalWiki\NeoWiki\Tests\NeoWikiMockAuthorityTrait;
@@ -35,6 +34,11 @@ class ResolveSubjectIriApiTest extends NeoWikiIntegrationTestCase {
3534
public function setUp(): void {
3635
$this->setUpNeo4j();
3736

37+
// Pin the default dereference target so the Accept-negotiation tests are independent of any
38+
// ambient $wgNeoWikiSubjectDereferenceTarget (e.g. a dev LocalSettings override). Tests that
39+
// exercise a specific target override this.
40+
$this->overrideConfigValue( 'NeoWikiSubjectDereferenceTarget', 'page' );
41+
3842
$this->createSchema( self::SCHEMA );
3943

4044
$this->pageId = $this->createPageWithSubjects(
@@ -104,15 +108,6 @@ public function testAbsentAcceptRedirectsToTheHostingPage(): void {
104108
$this->assertHostingPageLocation( $response );
105109
}
106110

107-
public function testPageDereferenceTargetRedirectsToThePlainHostingPage(): void {
108-
$this->overrideConfigValue( 'NeoWikiSubjectDereferenceTarget', 'page' );
109-
110-
$response = $this->deref( headers: [ 'Accept' => 'text/html' ] );
111-
112-
$this->assertSame( 303, $response->getStatusCode() );
113-
$this->assertHostingPageLocation( $response );
114-
}
115-
116111
public function testDataTabDereferenceTargetRedirectsToTheSubjectsRow(): void {
117112
$this->overrideConfigValue( 'NeoWikiSubjectDereferenceTarget', 'data-tab' );
118113

@@ -129,9 +124,9 @@ public function testDataTabDereferenceTargetRedirectsToTheSubjectsRow(): void {
129124
);
130125
$this->assertStringContainsString( 'action=subjects', $location, 'The redirect opens the Data tab.' );
131126
$this->assertStringEndsWith(
132-
'#' . SubjectRowAnchor::domId( self::SUBJECT_ID ),
127+
'#' . self::SUBJECT_ID,
133128
$location,
134-
'The fragment is the Subject row anchor the Data tab expands and highlights.'
129+
'The fragment is the bare Subject id the Data tab expands and highlights.'
135130
);
136131
}
137132

tests/phpunit/Presentation/SubjectRowAnchorTest.php

Lines changed: 0 additions & 27 deletions
This file was deleted.

0 commit comments

Comments
 (0)