Skip to content

OCD-5460: continued removal of AngularJS - #252

Open
andlar wants to merge 7 commits into
OCD-5430from
OCD-5460
Open

andlar wants to merge 7 commits into
OCD-5430from
OCD-5460

Conversation

@andlar

@andlar andlar commented Sep 9, 2026

Copy link
Copy Markdown
Owner

What

Next slice of removing AngularJS. This branch retires every remaining
getAngularService call outside a single deliberate boundary, drops one Angular
dependency, and finishes the ngStorage-* migration. Routing is deliberately
out of scope
and is the next ticket.

getAngularService went from 15 call sites across 9 files to 2 files: the helper
that defines it, and services/navigation.service.js, which is now the only
place that talks to ui-router.

services/navigation.service.js (new)

Components reached into the router directly, so $state, $location,
$rootScope, $transitions and $stateParams were scattered across eight
files. They are now behind six functions:

Function Replaces Callers
goToState $state.go chpl-link, navigation-top ×2, confirm, login, register-user
reloadState $state.reload navigation-top
getCurrentUrl $location.url() navigation-top
goToUrl $location.url(x) + $digest() compare-display
getRouteParams $stateParams not-found
onRouteChange $transitions.on* chpl-route-loading

ChplLink alone funnels 65 router={...} props across 52 files through one
$state.go, so replacing the router next ticket is a change to this one module
rather than a hunt through components.

Two design notes: goToUrl absorbs the $rootScope.$digest() that a
$location change from a React handler needs, because that is an Angular
implementation detail; and onRouteChange returns a single deregister function
in place of the three chpl-route-loading was tracking.

Uploads through the shared axios client

The four upload components used the Angular Upload service, which meant
hand-building Authorization and API-Key headers and fetching an access token
first. They now post FormData through the shared axios instance, so the
interceptors supply all of it — the token plumbing, the header construction and
the browserInfo selectors all go away, and ng-file-upload is removed
entirely
.

One trap worth knowing about, verified against the installed axios 1.7.7: the
shared instance defaults to Content-Type: application/json, and axios'
transformRequest serialises a FormData body to JSON when that header is set,
so uploads would have silently posted JSON. The request interceptor now clears
the header for FormData and lets the browser set multipart/form-data with
its own boundary.

Two response shapes also differed and are corrected: the uploaded filename comes
from the local file rather than response.config.data.file.name, which axios
does not populate, and error details are read from error.response.data instead
of error.data.

ngStorage-* retired

  • BrowserWrapper and BrowserContext deleted. Nothing consumed the
    context — no file imported it or called addToCompared, addToViewed,
    getPreviouslyCompared or getPreviouslyViewed. The live path was always
    Redux.
  • That deletion also fixes a persistence bug. useStorage writes its state
    back on mount, and BrowserWrapper was an ancestor of the widget that
    dispatched, so React ran the child effect first: the reducer wrote the new
    list, then BrowserWrapper wrote back the list it had read during its own
    render, dropping the entry just added. Redux kept it in memory, so only the
    persisted copy was wrong — the most recently viewed or compared listing was
    lost on reload.
  • browserInfo reducers are pure again. The one-time carry-over moved into
    store.js alongside the existing legacy-key migration. Note that
    preloadedState replaces a slice rather than merging into its initialState,
    so the migration spreads the slice defaults — otherwise api and apiKey
    would come back undefined. For the same reason api/apiKey are now
    overlaid from the current build over whatever chplState holds; neither has a
    reducer, so a persisted copy would pin a returning user to configuration from
    an earlier deploy.

Complaint criteria sorting (fix!)

complaint-edit was the last consumer of the Angular utilService, via its
sortCertActual comparator. That comparator's sort table is stale — it is
missing 170.315 (b)(10), (d)(12), (d)(13), (g)(10), (g)(31) through
(g)(33), (j)(20) and (j)(21) — and anything absent from the table is given
Number.MAX_VALUE, so those criteria were all collapsing to the end of the
dropdown in input order instead of sorting into sequence:

BEFORE                        AFTER
 1. 170.315 (a)(1)             1. 170.315 (a)(1)
 2. 170.315 (b)(9)             2. 170.315 (b)(9)
 3. 170.315 (c)(1)             3. 170.315 (b)(10)
 4. 170.315 (j)(21)  <- stray  4. 170.315 (c)(1)
 5. 170.315 (b)(10) <- stray   5. 170.315 (d)(12)
 6. 170.315 (g)(31) <- stray   6. 170.315 (g)(10)
 7. 170.315 (g)(10) <- stray   7. 170.315 (g)(31)
 8. 170.315 (d)(12) <- stray   8. 170.315 (j)(21)

criteria.service's sortCriteria already has the current table and is already
used by surveillance.service, so it is used here too. The array is also copied
before sorting: criteriaData comes from the react-query cache and sort
mutates in place.

CLAUDE.md

Documents the branch-name commit tag requirement. Analysing 600 commits from
production showed the guidance needed to match reality: 392 of 409 non-merge
commits are a subject line plus the tag with no body, so that is the documented
shape. Also records the subject prefixes actually in use (including the
non-standard ui and <type>-flag families), that ! marks a user-visible
change for release notes rather than a breaking change, that release: commits
are exempt from the tag, and the three ways the prepare-commit-msg hook fails
to fire.

Base branch

This stacks on OCD-5430, which is not yet in staging. Point the PR at
OCD-5430 or it will show all 36 commits instead of these 7.

Not in this PR

  • Routing. 13 *.state.js, the remaining angular.module() calls,
    reactToAngularComponent and navigation.service.js itself. Next ticket.
  • The eight ui-view shell components (*.component.js), the four .html
    templates and src/app/templates.js — deleting these would also remove a
    webpack entry. Sized and ready, but it is routing scaffolding, so it belongs
    with the routing ticket.
  • Dead Angular dependencies. angular-zxcvbn has zero references, and
    ngCytoscape plus the vendored lib/ngCytoscape.min.js are superseded by
    react-cytoscapejs. Several others look dead but each needs verifying.
  • chplReportsProducts is referenced by reports.state.js:21 and
    registered nowhere, so /reports/products renders nothing. Found while
    inventorying; needs its own ticket to decide whether the route is dead or
    broken.

Verification

  • ESLint clean on every changed file, no new warnings
  • yarn build compiles
  • The store migration and the FormData content-type behaviour were both verified
    against the real modules with throwaway harnesses, including negative controls

🤖 Generated with Claude Code

andlar and others added 7 commits September 9, 2026 12:37
BrowserContext had no consumers: nothing imported it or called addToCompared,
addToViewed, getPreviouslyCompared or getPreviouslyViewed. The live path is
Redux — the compare and viewed widgets dispatch into browserInfo, and the search
listings page reads it with useSelector.

Deleting it also removes a second writer of the ngStorage-previouslyCompared and
ngStorage-previouslyViewed keys. useStorage writes its state back on mount, and
because BrowserWrapper was an ancestor of the widget that dispatched, React ran
the widget's effect first: the reducer wrote the new list, then BrowserWrapper
wrote back the list it had read during its own render, dropping the entry just
added. Redux kept it in memory, so only the persisted copy was wrong — the most
recently viewed or compared listing was lost on reload.

[#OCD-5460]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The slice read ngStorage-previouslyCompared and ngStorage-previouslyViewed in
initialState and wrote them back from its reducers, which made the reducers
impure and duplicated what chplState already persists. Those keys were only ever
a cross-bundle sync hack, and the shared store retired the need for it.

Move the one-time carry-over into store.js alongside the existing legacy-key
migration, and add both keys to the sweep. Note that preloadedState replaces a
slice rather than merging into its initialState, so the migration has to spread
the slice defaults - otherwise api and apiKey would come back undefined.

For the same reason, overlay api and apiKey from the current build over whatever
chplState holds: neither has a reducer, so a persisted copy would pin a
returning user to configuration from an earlier deploy.

[#OCD-5460]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
[#OCD-5460]

The tag is mandatory in every commit message and must appear exactly once, so
the doc now names the three mechanisms that can supply it - a
prepare-commit-msg hook, these instructions when an agent drafts the message,
or typing it by hand - and says to establish which one is live before relying
on it. It also records the two ways the hook can fail to fire, and where to put
the tag when you are the one writing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…file-upload

The four upload components used the AngularJS Upload service, which meant
building Authorization and API-Key headers by hand and fetching an access token
first. Posting FormData through the shared axios instance lets the interceptors
supply all of that, so the token plumbing, the header construction and the
browserInfo selectors all go away, and ng-file-upload can be dropped entirely.

The instance defaults to Content-Type: application/json, and axios serialises a
FormData body to JSON when that header is set, so the request interceptor now
clears it for FormData and lets the browser set multipart/form-data with its own
boundary.

Two response shapes differed and are corrected: the uploaded filename now comes
from the local file rather than response.config.data.file.name, which axios does
not populate, and error details are read from error.response.data instead of
error.data.

[#OCD-5460]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Components reached into ui-router directly via getAngularService, so $state,
$location, $rootScope, $transitions and $stateParams were scattered across eight
files. Concentrate them in services/navigation.service.js and have the call
sites depend on that instead. getAngularService now appears in exactly one
place, so replacing ui-router later is a change to this module rather than a
hunt across components.

goToUrl absorbs the $rootScope.$digest() that a $location change from a React
handler needs, and onRouteChange returns one deregister function in place of the
three that chpl-route-loading was tracking. No behaviour changes.

[#OCD-5460]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
complaint-edit was the last consumer of the AngularJS utilService, using its
sortCertActual comparator. That comparator's sort table is stale - it is missing
170.315 (b)(10), (d)(12), (d)(13), (g)(10), (g)(31) through (g)(33), (j)(20) and
(j)(21) - and anything absent from the table is given Number.MAX_VALUE, so those
criteria were all collapsing to the end of the dropdown in input order rather
than sorting into sequence.

criteria.service's sortCriteria already has the current table and is already
used by surveillance.service, so use it here too. Also copy the array before
sorting: criteriaData comes from the react-query cache and sort mutates in
place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

[#OCD-5460]
Analysing 600 commits from production showed the documented shape was wrong:
392 of 409 non-merge commits are a subject line plus the tag with no body, so
the example now shows that form and treats a body as the exception. Records the
subject prefixes in use, including the non-standard ui and <type>-flag families,
notes that ! marks a user-visible change for release notes rather than a
breaking change, and exempts release commits from the tag requirement. Adds
commits made through the GitHub web UI to the list of ways the hook does not
fire, which accounts for untagged commits in the history.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

[#OCD-5460]
@andlar
andlar requested review from kekey1 and tmy1313 September 9, 2026 20:32
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.

2 participants