Skip to content

Refactor: env#385

Open
TatevikGr wants to merge 1 commit into
mainfrom
env
Open

Refactor: env#385
TatevikGr wants to merge 1 commit into
mainfrom
env

Conversation

@TatevikGr

@TatevikGr TatevikGr commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Configuration
    • Updated API endpoint configuration to use a single base URL with the /api/v2 path.
    • Consolidated API base URL settings under the application configuration.
    • Removed the separate REST API base URL environment setting.

Thanks for contributing to phpList!

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

API URL configuration now uses API_BASE_URL as the shared source, derives the REST API URL with /api/v2, renames the plain API parameter to app.api_base_url, and removes the separate REST_API_BASE_URL default.

Changes

API URL configuration

Layer / File(s) Summary
Shared API base URL parameters
config/parameters.yml.dist
app.api_base_url is sourced from API_BASE_URL, while app.rest_api_base_url appends /api/v2; the separate REST_API_BASE_URL default is removed.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is too vague and generic to clearly describe the config parameter and base URL refactor. Rename it to something specific like "Refactor API base URL environment parameters".
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch env

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@config/parameters.yml.dist`:
- Around line 37-39: Normalize API_BASE_URL to remove any trailing slash before
appending /api/v2 in app.rest_api_base_url, while preserving app.api_base_url
and downstream URL values without duplicate slashes. Ensure the default
configuration and deployment-provided values follow the same no-trailing-slash
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a9189fb3-cd53-4927-a570-613d605161a1

📥 Commits

Reviewing files that changed from the base of the PR and between c5efbd8 and 3ef0af5.

📒 Files selected for processing (1)
  • config/parameters.yml.dist

Comment on lines +37 to 39
app.rest_api_base_url: '%%env(API_BASE_URL)%%/api/v2'
app.api_base_url: '%%env(API_BASE_URL)%%'
env(API_BASE_URL): 'http://api.phplist.local/'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Normalize the API base URL before appending /api/v2.

The default API_BASE_URL ends with /, so this composition produces http://api.phplist.local//api/v2. Any deployment value with a trailing slash has the same problem, affecting downstream URLs and HTTP stamp values. Either remove the slash from the default and enforce a no-trailing-slash contract, or normalize the value before concatenation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@config/parameters.yml.dist` around lines 37 - 39, Normalize API_BASE_URL to
remove any trailing slash before appending /api/v2 in app.rest_api_base_url,
while preserving app.api_base_url and downstream URL values without duplicate
slashes. Ensure the default configuration and deployment-provided values follow
the same no-trailing-slash behavior.

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