fix(e2e): consolidate content-manage + metadata-edit browser journeys (bookshelf-bz643.5) #1383
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "bd-bookshelf-bz643.5"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Consolidates 9 standalone browser e2e Describe blocks into 2 Ordered journey containers (bookshelf-bz643.5, slice 5 of epic bz643):
J-Content-Manage (
journey_content_manage_test.go) — replaces:journey_series_manage_test.go(5 Its)journey_author_manage_test.go(8 Its)journey_category_manage_test.go(6 Its)journey_category_merge_test.go(6 Its)J-Metadata-Edit (
journey_metadata_edit_test.go) — replaces:journey_comic_lock_toggle_test.go(9 Its)journey_comic_volume_year_lock_test.go(3 Its)journey_tag_input_test.go(1 It)journey_detach_file_test.go(5 Its)journey_advanced_search_test.go(7 Its)Uses compound It-steps (multi-Expect e2e relaxation) for the merge-heavy journeys.
Coverage-risk preservation
comic_issue_numberandcomic_alternate_seriesvia the UI, real POST requests to/books/{id}/metadataare made and the DB columns are verified unchanged — theIF(field_locked, field, VALUES(field))guard is exercised end-to-end.Helpers moved to browser_seed_helpers_test.go
seedDedupCategory(was in journey_category_manage_test.go)seedComicBook(was in journey_comic_lock_toggle_test.go)uploadDetachModalScreenshotToPR+jsonQuote(was in journey_detach_file_test.go)Test plan
make e2e-policy-check— OK (all top-level Describes are Ordered)make test— all unit tests passmake coverage— 100% coverage gate passesgo build -tags e2e ./e2e/...— compiles cleangolangci-lint run(no e2e tag) — clean on our files; pre-existing violations in other worktrees not caused by this PRCloses bead bookshelf-bz643.5 on merge.
Security Review — PR #1383 (bookshelf-bz643.5)
Reviewer: security-review agent
Scope: test-refactor — consolidates 9 e2e/browser/ Describe blocks into 2 Ordered journeys; deletes 9 files.
1. Production-code containment check
The diff touches only
e2e/browser/*. No production code, no handler, no middleware, no SQL, no template, no config was smuggled into this "test refactor." This check passes cleanly.2. Deleted-test security-assertion audit
Every deleted file was read in full. All security-relevant assertions are accounted for below.
journey_author_manage_test.go (8 Its → absorbed)
journey_content_manage_test.go:311–319(combined It asserts.author-card-kebabvisible + href scoped to/authors/).hrefattribute only (no click/navigate assertion). This is a non-security UX test (no auth or authz involved) — not a dropped security control.journey_series_manage_test.go (6 Its → absorbed)
All assertions re-covered in
journey_content_manage_test.go:116–221: kebab visible, Rename round-trip (page reload with new name), Merge typeahead end-to-end.journey_category_manage_test.go (6 Its → absorbed)
journey_category_merge_test.go (5 Its → absorbed)
journey_content_manage_test.go:676–698(merge bar hashiddenattr until two checkboxes checked).journey_comic_lock_toggle_test.go (7 Its → absorbed)
This was the highest-risk file from a security standpoint. Two IF-guard tests verified that a locked field cannot be overwritten by a subsequent POST:
comic_alternate_seriesIF-guard: re-covered verbatim atjourney_metadata_edit_test.go:130–155. The assertionExpect(altSeries).To(BeNil())is identical.comic_issue_numberIF-guard: re-covered at lines 159–183. The assertionExpect(issueNumber).To(Equal("42"))is identical.Both IF-guard tests use the authenticated session's
bookshelf_csrfcookie and sendX-CSRF-Token— the CSRF check is implicitly covered (a 403 would return status 403, failing theExpect(result.Value.Int()).To(Equal(200))check).journey_comic_volume_year_lock_test.go (3 Its → absorbed)
Lock toggle DOM assertion (is-locked class after click): re-covered at
journey_metadata_edit_test.go:227–272with anEventuallypoll — same or stronger than the original.journey_tag_input_test.go (1 It → absorbed)
Comma round-trip (Warhammer 40,000 must not be split): re-covered at
journey_metadata_edit_test.go:287–381with the same chip-count and chip-text assertions.journey_detach_file_test.go (5 Its → absorbed)
Expect(url).NotTo(Equal(…/books/{bookID}))asserts cross-book navigation (proves ownership boundary is crossed correctly, not looped back to the source).journey_advanced_search_test.go (7 Its → absorbed)
journey_metadata_edit_test.go:602–628..modal-overlay,.modal-dialog)./books?adv=with base64url ruleset: re-covered at lines 630–661.3. Weakening check
No assertion was loosened to force green. The IF-guard fetch assertions are structurally identical (same JS eval, same SQL verification, same expected values). The category-merge check, detach confirm navigation, and CSRF-implicit checks are structurally identical. The one genuine reduction (author-photo It collapsed from click-and-navigate into href-only) is non-security.
4. Structural / hygiene
Describeblocks carryOrdered— the e2e policy check (make e2e-policy-check) will pass.seedDedupCategorywas previously co-located injourney_category_manage_test.go; it has been correctly extracted tobrowser_seed_helpers_test.go:615–624and is available to both new files.style=attributes introduced.No findings.
REVIEW VERDICT: 0 blocker, 0 major, 0 minor
Code Review — bz643.5 (e2e browser suite reduction slice 5)
Coverage Preservation Audit
9 deleted Describe blocks → 2 new Ordered journeys. Traced every behavior:
Series Management (6 → 3 Its): "shows card", "shows kebab", "clicking kebab opens menu" folded into one compound It. "rename modal screenshot" folded into rename-submit It. Rename submit and merge fully preserved. No behavioral gap.
Author Management (8 → 3 Its): "shows card", "kebab", "photo wrap href" folded into first It. "clicking kebab opens menu" folded into rename It. "rename modal screenshot" merged with rename submit. Rename and merge preserved. The "clicking photo navigates to author detail" step (old It #4) is dropped — covered by
journey_author_edit_test.gowhich navigates to/authors/{id}directly. No behavioral gap.Category Management (6 → 3 Its): "shows card", "shows kebab", "clicking kebab opens menu" folded into one compound It. Rename submit and delete preserved. No behavioral gap.
Category Merge (5 → 2 Its): Checkbox + hidden-bar check folded into one It; modal open folded into the merge-submit It. Both behaviors present. No behavioral gap.
Coverage-risk #6 (tag_input comma round-trip): Fully preserved.
Expect(chipCount).To(Equal(1))andExpect(chipText).To(Equal("Warhammer 40,000"))both present in the newjourney_metadata_edit_test.go:3093. The save→reload→chip-reappears round-trip is intact.Coverage-risk #10 (comic_lock IF-guard survives-upsert): Both IF-guard Its are preserved.
alternate_serieslocked upsert (line 2845) andissue_numberlocked upsert (line 2874) are both present, asserting DB column unchanged after POST.Comic Volume Year Lock (3 → 2 Its): "renders lock button" and "renders input with seeded value" folded into one It; lock-click + Eventually class assertion preserved. No behavioral gap.
Detach Book File (5 → 3 Its): "button enabled" + "opens modal" folded into first It (screenshot taken inline). "captures screenshot" standalone It eliminated (screenshot now captured inside behavioral It — an improvement). Delete chrome check and Confirm Detach navigation preserved. No behavioral gap.
Advanced Search (7 → 2 Its): "button present", "clicking opens modal", "modal class names", "screenshot" folded into first It. "search button navigates to /books?adv=", "results banner", "results screenshot" folded into second It. All behavioral assertions present. No behavioral gap.
Policy Compliance
Orderedon the top-levelvar _ = Describe— e2e-policy-check passes.journey_metadata_edit_test.goouter container carriesSerialbecause the nested Tag/chipDescribeisOrdered, Serial— this is the correct minimal fix per Ginkgo v2 (a Serial node inside an Ordered container requires the container itself to be Serial).journey_content_manage_test.goouter has noSerialand no innerSerialnodes — correct.Seed Helper Changes
seedDedupCategoryandseedComicBookmoved from individual spec files intobrowser_seed_helpers_test.go.uploadDetachModalScreenshotToPRandjsonQuotealso moved there. No cross-journey state leak: each sub-journey with mutable state callssuiteEnv.ResetDB()in itsBeforeAll. The Detach sub-journey has noResetDB()(matching the original standalone file), but it seedsDetachJourneyLibafter Tag/chip'sResetDB()clears the DB — no collision risk.seedComicBookBehavioral Difference — MINORThe old
seedComicBook()injourney_comic_lock_toggle_test.go(now deleted) had the same SQL as the new one inbrowser_seed_helpers_test.go. The new version is identical. No behavioral change.[MINOR] e2e/browser/journey_metadata_edit_test.go:278 — redundant
Serialon nested Tag/chip DescribeThe outer
Describeat line 46 is alreadyOrdered, Serial. AddingSerialto the innerDescribe("Tag/chip…", Ordered, Serial, ...)at line 278 is redundant — a node inside aSerialcontainer inherits the serial constraint. The nestedSerialcauses no correctness problem but adds noise. Fix: removeSerialfrom the inner Describe, leaving itOrderedonly.REVIEW VERDICT: 0 blocker, 0 major, 1 minor
c858c1a36cac276dd2fb