Conversation
📝 WalkthroughWalkthroughThis PR updates dependencies to reference dev-dev branches and revises the OpenAPI contract across multiple endpoints. The openapi.json changes introduce a new GET /api/v2/subscribe-pages list endpoint with cursor pagination, restructure POST /api/v2/subscribe-pages request/response bodies to include a data array and support validation errors, convert /api/v2/subscribe-pages/{id}/data from PUT to DELETE, add a new GET /api/v2/lists/{listId}/public endpoint, refactor GET /api/v2/bounces pagination schema, and define new SubscribePageData and updated SubscribePage schemas. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openapi.json (1)
4907-4933:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep this endpoint authenticated or strip the owner payload.
Line 4911 makes
php-auth-pwoptional, but Lines 4932-4933 still returnSubscribePage, and that schema includesowner -> Administrator. That effectively documents unauthenticated access to admin email/login/privilege data. Either keep auth required here or return a reduced public schema instead.🤖 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 `@openapi.json` around lines 4907 - 4933, The OpenAPI entry currently marks the header "php-auth-pw" as optional while returning the full SubscribePage (which contains owner -> Administrator), exposing admin data; either make the header required or change the response to a public schema without owner. Fix by updating the endpoint that uses path param "id": either set the "php-auth-pw" header object's "required" to true, or replace the response schema "$ref": "`#/components/schemas/SubscribePage`" with a new or existing public variant (e.g., SubscribePagePublic) that omits the owner/Administrator fields and ensure the components/schemas reflect that change.
🤖 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`:
- Around line 50-54: composer.json currently pins phplist/core and
tatevikgr/rest-api-client to the floating alias "dev-dev", which makes installs
non-reproducible; update both entries to an immutable version (a specific tagged
version or a commit reference like "v1.2.3" or "dev-branch#commit-hash") instead
of "dev-dev", then run composer update to regenerate composer.lock and commit
the updated composer.json and the generated composer.lock so installs use exact
resolved package hashes; look for the package keys "phplist/core" and
"tatevikgr/rest-api-client" when making this change.
In `@openapi.json`:
- Around line 4745-4752: The OpenAPI parameter "after_id" documents the cursor
as "starting from 0" but its schema sets "default" and "minimum" to 1; update
the schema to match the description by changing the "default" and "minimum"
values for the "after_id" parameter to 0 so generated clients and validators
accept 0 as a valid starting cursor (or alternatively change the description to
"starting from 1" if you intend the cursor to start at 1).
- Around line 4843-4857: The OpenAPI contract allows the "data" property to be
nullable in write/input schemas but defines SubscribePage.data (and the shared
success response schema used by create/update) as a non-null array, causing a
mismatch; choose one canonical behavior and make schemas consistent: either mark
SubscribePage.data and all response schemas that reference it as "nullable":
true to accept null responses, or change the write/input schemas (the
occurrences at lines noted) to disallow null and require an empty array instead;
update the shared schema(s) (SubscribePage and the success response schema
references) so that all read and write definitions for "data" use the same
nullable setting.
---
Outside diff comments:
In `@openapi.json`:
- Around line 4907-4933: The OpenAPI entry currently marks the header
"php-auth-pw" as optional while returning the full SubscribePage (which contains
owner -> Administrator), exposing admin data; either make the header required or
change the response to a public schema without owner. Fix by updating the
endpoint that uses path param "id": either set the "php-auth-pw" header object's
"required" to true, or replace the response schema "$ref":
"`#/components/schemas/SubscribePage`" with a new or existing public variant
(e.g., SubscribePagePublic) that omits the owner/Administrator fields and ensure
the components/schemas reflect that change.
🪄 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
Run ID: a6d6e538-334f-4921-a0e2-8036874f1d90
📒 Files selected for processing (2)
composer.jsonopenapi.json
| "phplist/core": "dev-dev", | ||
| "symfony/twig-bundle": "^6.4", | ||
| "symfony/webpack-encore-bundle": "^2.2", | ||
| "symfony/security-bundle": "^6.4", | ||
| "tatevikgr/rest-api-client": "dev-main" | ||
| "tatevikgr/rest-api-client": "dev-dev" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check composer.json constraints =="
python - <<'PY'
import json
with open("composer.json") as f:
data = json.load(f)
req = data.get("require", {})
for name in ("phplist/core", "tatevikgr/rest-api-client"):
print(f"{name}: {req.get(name)}")
PY
echo
echo "== Verify remote branches exist =="
git ls-remote --heads https://github.com/phpList/core.git dev dev-dev main || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev dev-dev main || true
echo
echo "== Verify lockfile pins commits for these packages (if lockfile exists) =="
if [ -f composer.lock ]; then
python - <<'PY'
import json
with open("composer.lock") as f:
lock = json.load(f)
pkgs = {p["name"]: p for p in lock.get("packages", [])}
for name in ("phplist/core", "tatevikgr/rest-api-client"):
p = pkgs.get(name)
if not p:
print(f"{name}: NOT FOUND in composer.lock")
continue
print(f"{name}: version={p.get('version')} reference={p.get('source',{}).get('reference')}")
PY
else
echo "composer.lock not found"
fiRepository: phpList/web-frontend
Length of output: 521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.json constraints =="
python - <<'PY'
import json
with open("composer.json") as f:
data = json.load(f)
req = data.get("require", {})
for name in ("phplist/core", "tatevikgr/rest-api-client"):
print(f"{name}: {req.get(name)}")
PY
echo
echo "== Check composer.lock exists (reproducibility) =="
if [ -f composer.lock ]; then
echo "composer.lock: present"
else
echo "composer.lock: MISSING"
fi
echo
echo "== Verify upstream 'dev' branches exist (dev-dev maps to branch 'dev') =="
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || trueRepository: phpList/web-frontend
Length of output: 415
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.json constraints =="
python - <<'PY'
import json
with open("composer.json") as f:
data = json.load(f)
req = data.get("require", {})
for name in ("phplist/core", "tatevikgr/rest-api-client"):
print(f"{name}: {req.get(name)}")
PY
echo
echo "== Check composer.lock exists (reproducibility) =="
if [ -f composer.lock ]; then
echo "composer.lock: present"
else
echo "composer.lock: MISSING"
fi
echo
echo "== Verify upstream 'dev' branches exist (dev-dev maps to branch 'dev') =="
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || trueRepository: phpList/web-frontend
Length of output: 415
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.lock reproducibility check =="
test -f composer.lock && echo "composer.lock: present" || echo "composer.lock: MISSING"
echo
echo "== upstream branch check (dev) for floating dev-dev constraints =="
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || trueRepository: phpList/web-frontend
Length of output: 309
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.lock reproducibility check =="
test -f composer.lock && echo "composer.lock: present" || echo "composer.lock: MISSING"
echo
echo "== upstream branch check (dev) for floating dev-dev constraints =="
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || trueRepository: phpList/web-frontend
Length of output: 309
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.lock reproducibility check =="
test -f composer.lock && echo "composer.lock: present" || echo "composer.lock: MISSING"
echo
echo "== upstream branch check (dev) for floating dev-dev constraints =="
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || trueRepository: phpList/web-frontend
Length of output: 309
🏁 Script executed:
#!/bin/bash
set -euo pipefail
test -f composer.lock && echo "composer.lock: present" || echo "composer.lock: MISSING"
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || trueRepository: phpList/web-frontend
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "composer.lock:"
test -f composer.lock && echo "present" || echo "MISSING"
echo
echo "upstream dev branches:"
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || trueRepository: phpList/web-frontend
Length of output: 223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "composer.lock:"
test -f composer.lock && echo "present" || echo "MISSING"
echo
echo "upstream dev branches:"
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || true
echo
echo "composer.json constraints:"
python - <<'PY'
import json
with open("composer.json") as f:
data = json.load(f)
req = data.get("require", {})
for name in ("phplist/core", "tatevikgr/rest-api-client"):
print(f"{name}: {req.get(name)}")
PYRepository: phpList/web-frontend
Length of output: 308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "composer.lock:"
test -f composer.lock && echo "present" || echo "MISSING"
echo
echo "composer.json constraints:"
python - <<'PY'
import json
with open("composer.json") as f:
data = json.load(f)
req = data.get("require", {})
for name in ("phplist/core", "tatevikgr/rest-api-client"):
print(f"{name}: {req.get(name)}")
PY
echo
echo "upstream dev branches (what dev-dev tracks):"
git ls-remote --heads https://github.com/phpList/core.git dev || true
git ls-remote --heads https://github.com/tatevikgr/phplist-api-client.git dev || trueRepository: phpList/web-frontend
Length of output: 330
Pin phplist/core and tatevikgr/rest-api-client to immutable refs (and commit composer.lock)
composer.json sets both phplist/core and tatevikgr/rest-api-client to dev-dev, which tracks the upstream moving dev branches. With composer.lock missing, installs/deploys won’t be reproducible and can drift.
- Replace
dev-devwith a tag/commit reference (immutable). - Generate and commit
composer.lockso Composer pins exact commit hashes.
🤖 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` around lines 50 - 54, composer.json currently pins
phplist/core and tatevikgr/rest-api-client to the floating alias "dev-dev",
which makes installs non-reproducible; update both entries to an immutable
version (a specific tagged version or a commit reference like "v1.2.3" or
"dev-branch#commit-hash") instead of "dev-dev", then run composer update to
regenerate composer.lock and commit the updated composer.json and the generated
composer.lock so installs use exact resolved package hashes; look for the
package keys "phplist/core" and "tatevikgr/rest-api-client" when making this
change.
| "name": "after_id", | ||
| "in": "query", | ||
| "description": "Last id (starting from 0)", | ||
| "required": false, | ||
| "schema": { | ||
| "type": "integer" | ||
| "type": "integer", | ||
| "default": 1, | ||
| "minimum": 1 |
There was a problem hiding this comment.
Align the after_id docs with the actual constraints.
Line 4747 says the cursor starts from 0, but Lines 4750-4752 set default: 1 and minimum: 1. That makes the documented starting cursor invalid for generated clients and validators.
🤖 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 `@openapi.json` around lines 4745 - 4752, The OpenAPI parameter "after_id"
documents the cursor as "starting from 0" but its schema sets "default" and
"minimum" to 1; update the schema to match the description by changing the
"default" and "minimum" values for the "after_id" parameter to 0 so generated
clients and validators accept 0 as a valid starting cursor (or alternatively
change the description to "starting from 1" if you intend the cursor to start at
1).
| "data": { | ||
| "type": "array", | ||
| "items": { | ||
| "properties": { | ||
| "key": { | ||
| "type": "string" | ||
| }, | ||
| "value": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "type": "object" | ||
| }, | ||
| "nullable": true | ||
| } |
There was a problem hiding this comment.
The data field is nullable on write but non-null on read.
Lines 4843-4857 and 5000-5014 allow data: null, while Lines 8563-8568 define SubscribePage.data as a plain array and all success responses reuse that schema. A successful create/update can therefore violate its own documented response contract unless the backend always normalizes null to [].
💡 One simple contract fix
"data": {
"type": "array",
"items": {
"$ref": "`#/components/schemas/SubscribePageData`"
- }
+ },
+ "nullable": true
}Also applies to: 5000-5014, 8563-8568
🤖 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 `@openapi.json` around lines 4843 - 4857, The OpenAPI contract allows the
"data" property to be nullable in write/input schemas but defines
SubscribePage.data (and the shared success response schema used by
create/update) as a non-null array, causing a mismatch; choose one canonical
behavior and make schemas consistent: either mark SubscribePage.data and all
response schemas that reference it as "nullable": true to accept null responses,
or change the write/input schemas (the occurrences at lines noted) to disallow
null and require an empty array instead; update the shared schema(s)
(SubscribePage and the success response schema references) so that all read and
write definitions for "data" use the same nullable setting.
Summary by CodeRabbit
New Features
Updates
Chores
Summary
Provide a general description of the code changes in your pull request …
were there any bugs you had fixed? If so, mention them. If these bugs have open
GitHub issues, be sure to tag them here as well, to keep the conversation
linked together.
Unit test
Are your changes covered with unit tests, and do they not break anything?
You can run the existing unit tests using this command:
Code style
Have you checked that you code is well-documented and follows the PSR-2 coding
style?
You can check for this using this command:
Other Information
If there's anything else that's important and relevant to your pull
request, mention that information here. This could include benchmarks,
or other information.
If you are updating any of the CHANGELOG files or are asked to update the
CHANGELOG files by reviewers, please add the CHANGELOG entry at the top of the file.
Thanks for contributing to phpList!