feat(cfl): add space key display to page view and search#46
Merged
Conversation
- page view: Show space key alongside space ID (e.g., "Space: DEV (ID: 9999)") - page view JSON: Enrich output with spaceKey field via GetSpace API call - search: Extract space key from displayUrl and show in SPACE KEY column - Graceful fallback if space lookup fails Closes #42
Contributor
Author
Test Coverage AssessmentSummaryOverall: Tests are adequate for this PR. The changes are well-covered with tests that address the critical paths and edge cases. Changes Analyzed1.
2.
Test Coverage Analysis
|
| Code Path | Covered | Test(s) |
|---|---|---|
| Space key lookup succeeds | Yes | TestRunView_WithSpaceKey |
| Space lookup fails gracefully | Yes | TestRunView_SpaceLookupFails_Graceful |
enrichPageWithSpaceKey helper |
Yes | TestEnrichPageWithSpaceKey |
JSON output with spaceKey |
Partial | Relies on existing TestRunView_JSONOutput + new tests |
No spaceId on page |
No | (edge case, but gracefully handles via empty check) |
search/search.go changes (18 additions):
| Code Path | Covered | Test(s) |
|---|---|---|
extractSpaceKey regex parsing |
Yes | TestExtractSpaceKey (7 cases) |
| Empty URL fallback | Yes | TestExtractSpaceKey/empty_URL |
No /spaces/ in URL |
Yes | TestExtractSpaceKey/no_spaces_in_URL |
| Integration with search results | Yes | TestRunSearch_DisplaysSpaceKey |
| Various URL formats | Yes | Standard, wiki-prefixed, full domain, with numbers, blogpost |
Observations
Strengths:
- The
extractSpaceKeyfunction has excellent table-driven test coverage with 7 distinct URL patterns - Graceful degradation is tested (space lookup failure)
- Both API call sequencing is verified (GetPage then GetSpace)
- Tests follow the project's established patterns (httptest servers, testify assertions)
Minor gaps (acceptable for this PR):
TestRunView_WithSpaceKeydoesn't explicitly verify the output contains "Space: DEV (ID: 98765)" - it only checks the API calls were made. The output is implicitly trusted via the view renderer.TestRunSearch_DisplaysSpaceKeyhas a comment stating output should contain "DEV" but doesn't assert on stdout content. This is a minor documentation/assertion gap.- No test for page with empty
spaceId(though the code handles it correctly with theif page.SpaceID != ""guard).
These gaps are acceptable because:
- The output formatting is handled by the trusted
viewpackage - The logic paths are covered; it's just the final string output that isn't asserted
- Empty
spaceIdcase is implicitly covered by existing tests that don't set it
Verdict
The test coverage is pragmatic and appropriate for this feature. The critical paths are covered:
- API call sequencing
- Error handling/graceful fallback
- URL parsing edge cases
- Helper function behavior
No obvious gaps that a reviewer would flag. The tests demonstrate the code works correctly without being overly pedantic about asserting every output string.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
page view: Display space key alongside space ID (e.g.,Space: DEV (ID: 9999))page view -o json: Enrich JSON output withspaceKeyfield via secondaryGetSpaceAPI callsearch: AddSPACE KEYcolumn extracted fromresultGlobalContainer.displayUrlChanges
internal/cmd/page/view.go: Add space lookup and enriched JSON outputinternal/cmd/search/search.go: AddextractSpaceKey()helper and update column headerTest plan
enrichPageWithSpaceKeyhelperextractSpaceKeywith various URL patternsCloses #42