Conversation
* Update .gitignore and composer.json for environment configuration * fix test * use dev --------- Co-authored-by: Tatevik <tatevikg1@gmail.com>
📝 WalkthroughWalkthroughThe changes configure PHPUnit and integration tests to load the project environment through Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.gitignore (1)
18-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep a safe environment template available.
This change ignores both
.envand.env.dist, whiletests/bootstrap.phploads.env. A clean checkout then depends on the Composer script creating every required variable.If
.env.distis the project template, keep it tracked and ignore only local environment files. Otherwise, add an equivalent committed test configuration or document the generator.🤖 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 @.gitignore around lines 18 - 19, Update the .gitignore entries so .env.dist remains tracked as the committed environment template while only local .env files are ignored; ensure tests/bootstrap.php can load the template from a clean checkout without relying on Composer-generated variables.tests/bootstrap.php (1)
5-10: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDeclare
symfony/dotenvexplicitly.This bootstrap directly instantiates
Symfony\Component\Dotenv\Dotenv, butcomposer.jsondoes not declare that package. If the resolved core branch does not provide it transitively,method_exists()skips the environment load and integration tests run without the intended variables.Add the dependency to
require-devand fail fast when the test environment cannot load.Proposed dependency change
"require-dev": { + "symfony/dotenv": "^6.4", "phpunit/phpunit": "^10.0",🤖 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 `@tests/bootstrap.php` around lines 5 - 10, Add symfony/dotenv explicitly to composer.json under require-dev, then update the tests/bootstrap.php Dotenv initialization to fail fast when the class or required bootEnv capability is unavailable instead of silently skipping environment loading.
🤖 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 `@composer.json`:
- Line 45: Update the phplist/core dependency in composer.json from the
unresolvable dev-dev revision to a resolvable released version containing the
required script handler, or configure the correct VCS repository and lockfile
entry if dev-dev is required.
In `@src/Messaging/Controller/TemplateController.php`:
- Around line 125-160: Update the OpenAPI metadata for createTemplates to
reference CreateTemplateRequest instead of UpdateTemplateRequest, document the
successful response as a single Template object rather than an array, and add a
409 response for conflicts raised by UniqueTemplateTitleValidator.
---
Nitpick comments:
In @.gitignore:
- Around line 18-19: Update the .gitignore entries so .env.dist remains tracked
as the committed environment template while only local .env files are ignored;
ensure tests/bootstrap.php can load the template from a clean checkout without
relying on Composer-generated variables.
In `@tests/bootstrap.php`:
- Around line 5-10: Add symfony/dotenv explicitly to composer.json under
require-dev, then update the tests/bootstrap.php Dotenv initialization to fail
fast when the class or required bootEnv capability is unavailable instead of
silently skipping environment loading.
🪄 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: 1cbe1c08-ee72-4546-9cc0-338a02e84426
📒 Files selected for processing (8)
.github/workflows/ci.yml.gitignorecomposer.jsonphpunit.xml.distsrc/Messaging/Controller/TemplateController.phptests/Integration/Common/Routing/RoutingTest.phptests/Integration/Composer/ScriptsTest.phptests/bootstrap.php
| "require": { | ||
| "php": "^8.1", | ||
| "phplist/core": "dev-main", | ||
| "phplist/core": "dev-dev", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
versions="$(
curl -fsSL https://repo.packagist.org/p2/phplist/core.json |
jq -r '.packages["phplist/core"][].version'
)"
printf '%s\n' "$versions"
grep -Fxq 'dev-dev' <<<"$versions"Repository: phpList/rest-api
Length of output: 336
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== composer relevant sections =="
python3 - <<'PY'
import json
with open("composer.json") as f:
data=json.load f
for key in ("require","config","scripts"):
if key in data:
print(key)
print(json.dumps(data[key], indent=2, sort_keys=True))
PY
echo "== lock file phplist/core =="
python3 - <<'PY'
from pathlib import Path
p=Path("composer.lock")
if p.exists():
text=p.read_text()
print("composer.lock_exists=true")
for needle in ("phplist/core","createDotenvConfiguration","source","dist"):
print(f"{needle}=", text.count(needle))
else:
print("composer.lock_exists=false")
PY
echo "== lock branch/version strings =="
rg -n '"phplist/name"|"phplist/core"|phplist/core|dev-dev|dev-main|createDotenvConfiguration' composer.lock composer.json || trueRepository: phpList/rest-api
Length of output: 282
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== composer relevant sections =="
python3 - <<'PY'
import json
from pathlib import Path
data=json.loads(Path("composer.json").read_text())
for key in ("require","config","scripts"):
if key in data:
print(key)
print(json.dumps(data[key], indent=2, sort_keys=True))
PY
echo "== composer.lock presence =="
python3 - <<'PY'
from pathlib import Path
p=Path("composer.lock")
print("composer.lock_exists=", p.exists())
if p.exists():
text=p.read_text()
for needle in ("phplist/core","createDotenvConfiguration","source","dist"):
print(f"{needle}_count=", text.count(needle))
PY
echo "== lock/json branch/version strings =="
rg -n '"phplist/name"|"phplist/core"|phplist/core|dev-dev|dev-main|createDotenvConfiguration' composer.lock composer.json /tmp 2>/dev/null | head -80 || trueRepository: phpList/rest-api
Length of output: 3432
Use a resolvable phplist/core revision.
dev-dev is not published for phplist/core, and there is no VCS repository or lockfile entry to resolve it. Revert to a released version that contains the required script handler, publish dev-dev, or add the correct repository entry.
🤖 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 `@composer.json` at line 45, Update the phplist/core dependency in
composer.json from the unresolvable dev-dev revision to a resolvable released
version containing the required script handler, or configure the correct VCS
repository and lockfile entry if dev-dev is required.
Source: MCP tools
| requestBody: new OA\RequestBody( | ||
| description: 'Pass session credentials', | ||
| required: true, | ||
| content: new OA\MediaType( | ||
| mediaType: 'multipart/form-data', | ||
| schema: new OA\Schema(ref: '#/components/schemas/UpdateTemplateRequest') | ||
| ) | ||
| ), | ||
| tags: ['templates'], | ||
| parameters: [ | ||
| new OA\Parameter( | ||
| name: 'php-auth-pw', | ||
| description: 'Session key obtained from login', | ||
| in: 'header', | ||
| required: true, | ||
| schema: new OA\Schema(type: 'string') | ||
| ), | ||
| ], | ||
| responses: [ | ||
| new OA\Response( | ||
| response: 201, | ||
| description: 'Success', | ||
| content: new OA\JsonContent( | ||
| type: 'array', | ||
| items: new OA\Items(ref: '#/components/schemas/Template') | ||
| ) | ||
| ), | ||
| new OA\Response( | ||
| response: 403, | ||
| description: 'Failure', | ||
| content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse') | ||
| ), | ||
| new OA\Response( | ||
| response: 422, | ||
| description: 'Failure', | ||
| content: new OA\JsonContent(ref: '#/components/schemas/ValidationErrorResponse') |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct the OpenAPI contract for createTemplates.
Line 130 documents UpdateTemplateRequest, but line 169 validates CreateTemplateRequest.
Lines 147-150 document an array, but line 174 returns one normalized Template.
Document the 409 response because UniqueTemplateTitleValidator throws ConflictHttpException for an existing title.
Proposed fix
- schema: new OA\Schema(ref: '`#/components/schemas/UpdateTemplateRequest`')
+ schema: new OA\Schema(ref: '`#/components/schemas/CreateTemplateRequest`')
...
- content: new OA\JsonContent(
- type: 'array',
- items: new OA\Items(ref: '`#/components/schemas/Template`')
- )
+ content: new OA\JsonContent(ref: '`#/components/schemas/Template`')
...
+ new OA\Response(
+ response: 409,
+ description: 'Failure',
+ content: new OA\JsonContent(ref: '`#/components/schemas/AlreadyExistsResponse`')
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| requestBody: new OA\RequestBody( | |
| description: 'Pass session credentials', | |
| required: true, | |
| content: new OA\MediaType( | |
| mediaType: 'multipart/form-data', | |
| schema: new OA\Schema(ref: '#/components/schemas/UpdateTemplateRequest') | |
| ) | |
| ), | |
| tags: ['templates'], | |
| parameters: [ | |
| new OA\Parameter( | |
| name: 'php-auth-pw', | |
| description: 'Session key obtained from login', | |
| in: 'header', | |
| required: true, | |
| schema: new OA\Schema(type: 'string') | |
| ), | |
| ], | |
| responses: [ | |
| new OA\Response( | |
| response: 201, | |
| description: 'Success', | |
| content: new OA\JsonContent( | |
| type: 'array', | |
| items: new OA\Items(ref: '#/components/schemas/Template') | |
| ) | |
| ), | |
| new OA\Response( | |
| response: 403, | |
| description: 'Failure', | |
| content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse') | |
| ), | |
| new OA\Response( | |
| response: 422, | |
| description: 'Failure', | |
| content: new OA\JsonContent(ref: '#/components/schemas/ValidationErrorResponse') | |
| requestBody: new OA\RequestBody( | |
| description: 'Pass session credentials', | |
| required: true, | |
| content: new OA\MediaType( | |
| mediaType: 'multipart/form-data', | |
| schema: new OA\Schema(ref: '`#/components/schemas/CreateTemplateRequest`') | |
| ) | |
| ), | |
| tags: ['templates'], | |
| parameters: [ | |
| new OA\Parameter( | |
| name: 'php-auth-pw', | |
| description: 'Session key obtained from login', | |
| in: 'header', | |
| required: true, | |
| schema: new OA\Schema(type: 'string') | |
| ), | |
| ], | |
| responses: [ | |
| new OA\Response( | |
| response: 201, | |
| description: 'Success', | |
| content: new OA\JsonContent(ref: '`#/components/schemas/Template`') | |
| ), | |
| new OA\Response( | |
| response: 403, | |
| description: 'Failure', | |
| content: new OA\JsonContent(ref: '`#/components/schemas/UnauthorizedResponse`') | |
| ), | |
| new OA\Response( | |
| response: 409, | |
| description: 'Failure', | |
| content: new OA\JsonContent(ref: '`#/components/schemas/AlreadyExistsResponse`') | |
| ), | |
| new OA\Response( | |
| response: 422, | |
| description: 'Failure', | |
| content: new OA\JsonContent(ref: '`#/components/schemas/ValidationErrorResponse`') |
🤖 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 `@src/Messaging/Controller/TemplateController.php` around lines 125 - 160,
Update the OpenAPI metadata for createTemplates to reference
CreateTemplateRequest instead of UpdateTemplateRequest, document the successful
response as a single Template object rather than an array, and add a 409
response for conflicts raised by UniqueTemplateTitleValidator.
2 participants
Summary by CodeRabbit
Bug Fixes
Chores
Thanks for contributing to phpList!