Skip to content

fix: make wiki path prefix configurable based on apiPath#83

Merged
pchuri merged 2 commits intopchuri:mainfrom
saggl:fix/wiki-path-prefix
Mar 23, 2026
Merged

fix: make wiki path prefix configurable based on apiPath#83
pchuri merged 2 commits intopchuri:mainfrom
saggl:fix/wiki-path-prefix

Conversation

@saggl
Copy link
Copy Markdown
Contributor

@saggl saggl commented Mar 19, 2026

Derives the /wiki URL prefix from apiPath instead of hardcoding it, fixing broken links on Server/Data Center instances.

🤖 Generated with Claude Code

URLs no longer hardcode /wiki — they derive the prefix from apiPath,
fixing broken links for Server/Data Center instances without /wiki.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@pchuri
Copy link
Copy Markdown
Owner

pchuri commented Mar 23, 2026

Thanks for the contribution, @saggl! This is a solid fix for Server/Data Center instances that don't use the /wiki prefix. The approach of deriving the prefix from apiPath makes sense.

One concern I noticed: there's a logic duplication between bin/confluence.js and ConfluenceClient that could lead to inconsistent behavior.

bin/confluence.js:11 uses the raw config.apiPath:

const getWebUrlPrefix = (apiPath) => apiPath && apiPath.startsWith('/wiki/') ? '/wiki' : '';

lib/confluence-client.js:38 uses the sanitized this.apiPath:

this.webUrlPrefix = this.apiPath.startsWith('/wiki/') ? '/wiki' : '';

Since sanitizeApiPath() normalizes the leading slash (e.g. wiki/rest/api/wiki/rest/api), the two functions can produce different results for the same input. For example, if a user configures apiPath: 'wiki/rest/api' (missing leading slash):

  • bin/confluence.js'' (raw value doesn't start with /wiki/)
  • ConfluenceClient'/wiki' (sanitized value starts with /wiki/)

Suggestion: Instead of maintaining a separate getWebUrlPrefix function in bin/confluence.js, could we reuse client.webUrlPrefix from the ConfluenceClient instance? That way there's a single source of truth and the sanitization logic is always applied consistently.

The tests look good — nice coverage of Cloud, Server, and default cases. It would also be great to add a test for the edge case above (missing leading slash in apiPath).

Addresses review feedback by removing logic duplication between bin/confluence.js
and ConfluenceClient that could cause inconsistent behavior.

Changes:
- Remove getWebUrlPrefix() and buildPageUrl() functions from bin/confluence.js
- Use client.webUrlPrefix and client.buildUrl() for all URL construction
- Add test for edge case: apiPath without leading slash (e.g., 'wiki/rest/api/')

This ensures consistent URL generation by always using the sanitized apiPath
through ConfluenceClient, preventing discrepancies when users configure apiPath
with or without a leading slash.

Resolves feedback in pchuri#83 (comment)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@saggl
Copy link
Copy Markdown
Contributor Author

saggl commented Mar 23, 2026

Thanks for the thorough review! I've addressed the feedback:

Changes Made

Removed duplicate logic - Deleted getWebUrlPrefix() and buildPageUrl() functions from bin/confluence.js

Single source of truth - All URL construction now uses client.webUrlPrefix and client.buildUrl() from the ConfluenceClient instance

Added edge case test - New test verifies that webUrlPrefix is correctly set to /wiki when apiPath is configured as 'wiki/rest/api/' (missing leading slash)

Benefits

  • Consistent behavior: The sanitized apiPath is always used, preventing the discrepancy you identified
  • Reduced code duplication: 6 lines of duplicate logic removed
  • Better test coverage: Edge case is now explicitly tested

All 158 tests pass ✅

The changes ensure that users get consistent URL output regardless of whether they configure apiPath with or without a leading slash.

@pchuri
Copy link
Copy Markdown
Owner

pchuri commented Mar 23, 2026

Great work on the update, @saggl! All feedback has been addressed cleanly:

  • Duplicate buildPageUrl() / getWebUrlPrefix() removed — single source of truth via client.webUrlPrefix and client.buildUrl()
  • printTree signature updated consistently
  • Edge case test for missing leading slash added

Code looks clean and the approach is solid. LGTM! 👍

@pchuri pchuri merged commit fb58a37 into pchuri:main Mar 23, 2026
6 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 23, 2026
## [1.27.5](v1.27.4...v1.27.5) (2026-03-23)

### Bug Fixes

* make wiki path prefix configurable based on apiPath ([#83](#83)) ([fb58a37](fb58a37)), closes [/github.com//pull/83#issuecomment-4108336611](https://github.com//github.com/pchuri/confluence-cli/pull/83/issues/issuecomment-4108336611)
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 1.27.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants