CLARIN-DSpace v9/Restore the FE-22 negative menu assertions

[milanmajchrak]

What

Adds src/app/app.menus.spec.ts (5 tests, no production code) to close a coverage gap left by the FE-22 port of #1326, merged this morning as b0c48c578e (PR #1508).

Why

The source commit fecfa07e5d asserted the scoped-search entry in three places in dso-edit-menu.resolver.spec.ts: it is present for a Community, present for a Collection, and — in the Item describe block, under should not return Community/Collection-specific entries —

const searchEntry = menu.find(entry => entry.id === 'search-dso');
expect(searchEntry).toBeFalsy();

On v9 the resolver does not exist, so that spec had no target and the port replaced it with comcol-search.menu.spec.ts. That spec covers the provider well, but it only ever calls getSectionsForContext(dso) directly. Two things it cannot see:

1. the negative case — the provider is registered onRoute(COMMUNITY_PAGE, COLLECTION_PAGE), so it must never resolve a section on an item page. Nothing asserted that;
2. the registration itself — nothing referenced app.menus.ts at all. Deleting the ComColSearchMenuProvider.onRoute(...) block left the whole existing suite green.

Neither gap is visible to an it()-count check: the port has 2 provider tests where the source had 2 resolver hunks, so it still reads 3 >= 3. The live probe in AC-FE-22-4 does cover the behaviour, but a refactor of app.menus.ts can silently re-break it between deploys.

Changes

One new file, src/app/app.menus.spec.ts. app.menus.ts is unchanged (git diff --stat is a single added file, 166 insertions).

buildMenuStructure() emits one resolved MENU_PROVIDER entry per registered provider, and its useFactory is what stamps menuID, parentID and activePaths onto the instance. The spec finds the entry whose deps[0] is ComColSearchMenuProvider and runs that factory, so it asserts against the real registration rather than a restatement of it.

• registration (3 tests): the entry exists; menuID === MenuID.DSO_EDIT; parentID is set, i.e. it is nested under DsoOptionMenuProvider and renders inside the options dropdown; activePaths contains COMMUNITY_PAGE and COLLECTION_PAGE and not ITEM_PAGE.
• resolved sections (2 tests): the configured provider is fed to a real MenuProviderService with a spy MenuService, and resolveRouteMenus() is driven with a community route and with an item route. On a community route exactly one section with model.link === '/search' is added, scoped to the community uuid. On an item route, none is.

The item-page test carries a second, always-active provider whose only job is to emit a marker section. resolveRouteMenus builds combineLatest over the providers that survive the route filter; with the search provider filtered out and nothing else in the list, that combineLatest never emits, addSection is never called, and a bare expect(addSection).not.toHaveBeenCalled() would pass for the wrong reason — it would also pass if the pipeline threw. The marker assertion makes the test fail in that case instead. The two behavioural tests are wrapped in fakeAsync for the same reason.

Testing

$ npm run test:headless -- --include='src/app/app.menus.spec.ts' --code-coverage=false
  MENUS - scoped-search entry (ComColSearchMenuProvider)
    registration in app.menus.ts
      should register ComColSearchMenuProvider in the DSO edit menu
      should register it as a sub-provider so it renders inside the options dropdown
      should activate it on community and collection pages but not on item pages
    sections resolved for a route
      should add the scoped-search section on a community page
      should not add the scoped-search section on an item page
TOTAL: 5 SUCCESS

Each test proven load-bearing by breaking what it guards:

Control A — delete the ComColSearchMenuProvider.onRoute(...) block from app.menus.ts (the regression "nothing tests the registration" describes):

TOTAL: 5 FAILED, 0 SUCCESS
  should register ComColSearchMenuProvider in the DSO edit menu   Error: Expected undefined to be truthy.
  (+ the other four, TypeError: Cannot read properties of undefined (reading 'useFactory'))

Control B — add MenuRoute.ITEM_PAGE to the onRoute(...) list, which is exactly the regression the source's dropped assertion guarded against:

TOTAL: 2 FAILED, 3 SUCCESS
  should activate it on community and collection pages but not on item pages FAILED
      Error: Expected [ 'community-page', 'collection-page', 'item-page' ] not to contain 'item-page'.
  should not add the scoped-search section on an item page FAILED
      Error: Expected $.length = 1 to equal 0.

Precisely the two negative tests go red; the three registration tests stay green.

Control C — remove the always-active control provider from the service's provider list, to show the item-page test does not pass vacuously:

TOTAL: 1 FAILED, 4 SUCCESS
  should not add the scoped-search section on an item page FAILED
      Error: Expected spy MenuService.addSection to have been called with: ...

Regression over the FE-22 suite and the rest of the menu specs:

$ npm run test:headless -- --include='src/app/app.menus.spec.ts' \
    --include='src/app/shared/menu/providers/comcol-search.menu.spec.ts' \
    --include='src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.spec.ts' \
    --include='src/app/shared/menu/*.spec.ts' --code-coverage=false
TOTAL: 97 SUCCESS

$ npm run lint:nobuild -- --quiet
All files pass linting.

$ NODE_OPTIONS=--max-old-space-size=4096 npm run build:prod
EXIT=0        # 0 errors

Follow-up to #1508 (card FE-22, source dtq-dev PR #1326).

🤖 Generated with Claude Code

[milanmajchrak]

Scope check — tests only

$ git diff --stat origin/dtq-dev-9-base HEAD
 src/app/app.menus.spec.ts | 166 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 166 insertions(+)

app.menus.ts is untouched. No production change was needed: buildMenuStructure() already exports what the test needs — MENUS is a Provider[] whose resolved MENU_PROVIDER entries carry deps: [providerType] and a useFactory that stamps menuID / parentID / activePaths onto the instance (menu.structure.ts:88-118).

Why this is the right seam

The route gate the negative test exercises is the real one, MenuProviderService.resolveRouteMenus:

// menu-provider.service.ts:158-160
if (!provider.shouldPersistOnRouteChange && isNotEmpty(provider.activePaths)) {
  provider.activePaths.forEach((path: MenuRoute) => {
    if (route.data.menuRoute === path) { shouldUpdate = true; }
  });
}

The spec drives that method rather than re-implementing the predicate, so the test cannot drift away from the production logic.

The gap this closes, at git level

fecfa07e5d touched dso-edit-menu.resolver.spec.ts with three hunks — Community, Collection, and this one in the Item describe block:

@@ -378,6 +398,9 @@
       it('should not return Community/Collection-specific entries', (done) => {
         const result = resolver.getDsoMenus(testObject, route, state);
         combineLatest(result).pipe(map(flatten)).subscribe((menu) => {
+          const searchEntry = menu.find(entry => entry.id === 'search-dso');
+          expect(searchEntry).toBeFalsy();
+
           const subscribeEntry = menu.find(entry => entry.id === 'subscribe');

On v9 the resolver does not exist, so #1508 replaced the whole spec with comcol-search.menu.spec.ts. That spec only calls getSectionsForContext(dso) directly, so it can see neither the negative case nor the registration. There was no app.menus.spec.ts on b0c48c578e — nothing in the repo referenced MENUS at all.

Neither gap shows up in an it()-count check: 2 provider tests against 2 source resolver hunks still reads 3 >= 3.

Full negative-control output

A — delete the ComColSearchMenuProvider.onRoute(...) block from app.menus.ts:

TOTAL: 5 FAILED, 0 SUCCESS
  should register ComColSearchMenuProvider in the DSO edit menu
      Error: Expected undefined to be truthy.
  should register it as a sub-provider so it renders inside the options dropdown
      TypeError: Cannot read properties of undefined (reading 'useFactory')
  should activate it on community and collection pages but not on item pages
      TypeError: Cannot read properties of undefined (reading 'useFactory')
  should add the scoped-search section on a community page
      TypeError: Cannot read properties of undefined (reading 'resolveRouteMenus')
  should not add the scoped-search section on an item page
      TypeError: Cannot read properties of undefined (reading 'resolveRouteMenus')

B — add MenuRoute.ITEM_PAGE to onRoute(...) (the regression the source's dropped assertion guarded against):

TOTAL: 2 FAILED, 3 SUCCESS
  registration in app.menus.ts should activate it on community and collection pages but not on item pages FAILED
      Error: Expected [ 'community-page', 'collection-page', 'item-page' ] not to contain 'item-page'.
  sections resolved for a route should not add the scoped-search section on an item page FAILED
      Error: Expected $.length = 1 to equal 0.

The controls discriminate — exactly the two negative tests go red while the three registration tests stay green.

C — remove the always-active control provider from the service's provider list (vacuity check):

TOTAL: 1 FAILED, 4 SUCCESS
  sections resolved for a route should not add the scoped-search section on an item page FAILED
      Error: Expected spy MenuService.addSection to have been called with: ...

resolveRouteMenus builds combineLatest over the providers surviving the route filter; with the search provider filtered out and nothing else in the list, combineLatest([]) never emits and addSection is never called. A bare expect(addSection).not.toHaveBeenCalled() would then pass for the wrong reason — and would also pass if the pipeline threw. The marker assertion is what makes the test fail in that case instead; both behavioural tests are additionally wrapped in fakeAsync.

All three controls were reverted; git status is clean apart from the new spec, and the suite is back to TOTAL: 5 SUCCESS.