Skip to content

test(glm5): review #36275 — zai.glm-5#40

Closed
wezell wants to merge 1 commit into
p36275-glm5-basefrom
p36275-glm5-head
Closed

test(glm5): review #36275 — zai.glm-5#40
wezell wants to merge 1 commit into
p36275-glm5-basefrom
p36275-glm5-head

Conversation

@wezell

@wezell wezell commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Agentic reviewer @v3.3.0-beta.18 on a NEW dotCMS/core change (dotCMS#36275 — publishing API: include scheduled bundles). Model: zai.glm-5. Reviewed diff = only dotCMS#36275 (11 files). Throwaway.

…tCMS#36275)

### Include scheduled bundles in GET /api/v1/publishing (resolves
dotCMS#36267)

###  Problem

/api/v1/publishing only reads publishing_queue_audit. Bundles pushed
with a future publish date have no audit row until the cron picks them
up, so they were invisible to the API (the legacy JSP Queue tab showed
them by reading publishing_queue directly).

###  Change

Surface those bundles through the existing endpoints as a new SCHEDULED
status — no new endpoint.

- PublishAuditStatus.Status: add SCHEDULED (negative sentinel code,
never persisted).
- PublisherAPI: add countScheduledBundleIds / getScheduledBundleIds —
queue rows with publish_date IS NOT NULL and no audit row (excludes
drafts); asset count from queue rows,
  env count via correlated subquery.
- GET /api/v1/publishing: unions the scheduled tier with the audit tier
as a contiguous block; pagination total and slicing stay exact.
?status=SCHEDULED filters to that tier;
  mixed filters work.
- GET /api/v1/publishing/{id}: returns a synthesized SCHEDULED detail
instead of 404.
- DELETE /api/v1/publishing/purge?status=SCHEDULED: rejected
(audit-based purge can't match it; points to per-bundle DELETE).
- Views: add nullable scheduledPublishDate (future run time) so
createDate keeps its meaning.
  - Regenerated openapi.yaml.

###  Tests

Integration tests in PublishingResourceIntegrationTest using the real
push path: visibility/status, status filter (single + mixed), pagination
total, detail-not-404, delete-cancels, transition to
WAITING_FOR_PUBLISHING once an audit row appears, and a
legacy-getQueueBundleIds-vs-API parity check.

---------

Co-authored-by: Will Ezell <will@dotcms.com>
Co-authored-by: Humberto Morera <31667212+hmoreras@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown

🤖 dotBot Review (Bedrock)

Reviewed 11 file(s); 8 candidate(s) → 2 confirmed, 6 uncertain (unverified, kept for review).

Confirmed findings

  • 🟡 Medium dotCMS/src/main/java/com/dotmarketing/util/NumberUtil.java:79 — NumberFormatException not caught for malformed string input
    Integer.parseInt(value.toString()) throws NumberFormatException for non-numeric strings, which propagates up uncaught. Callers may not expect this unchecked exception from a utility method that returns a default for null.
  • 🟡 Medium dotcms-integration/src/test/java/com/dotcms/rest/api/v1/publishing/PublishingResourceIntegrationTest.java:842 — Potential test isolation issue with countScheduledBundleIds
    The countScheduledBundleIds(null) call may include scheduled bundles from other tests running in parallel, making the assertion flaky. The test creates one scheduled bundle but counts all scheduled bundles in the system.

🔎 Uncertain (could not confirm or disprove — review manually)

  • 🟡 Medium dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingResource.java:267 — Potential inconsistency if scheduledTotal changes between calls
    scheduledTotal is computed once and used for both count and offset calculation. If data changes between the two calls, the offset math could be wrong or items could be duplicated/missed.
  • 🟡 Medium dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingResource.java:273 — Missing null check on getScheduledBundleIds result
    getScheduledBundleIds could return null instead of empty list, causing NPE on .stream() call.
  • 🟡 Medium dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingResource.java:386 — Missing permission check for scheduled bundle detail
    When a bundle is SCHEDULED, the code returns scheduledDetail without the permission check that would normally happen for audit-based bundles.
  • 🟡 Medium dotcms-integration/src/test/java/com/dotcms/rest/api/v1/publishing/PublishingResourceIntegrationTest.java:768 — Test may be flaky due to shared state
    The test uses createEnvironmentWithPermission which may fail if environment name 'sched-env' already exists from another test run, causing test isolation issues.
  • 🔵 Low dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingResource.java:285 — getPublishAuditStatus may return null items
    getPublishAuditStatus could return null elements in the list, which would cause NPE in the stream map.
  • 🔵 Low dotcms-integration/src/test/java/com/dotcms/rest/api/v1/publishing/PublishingResourceIntegrationTest.java:927 — Potential race condition in scheduled bundle transition test
    Between inserting the audit status and querying the endpoint, a real cron job could theoretically pick up the bundle, though unlikely in test environment.

zai.glm-5 · Run: #28257821964 · tokens: in: 24629 · out: 811 · total: 25440 · calls: 11 · est. ~$0.017

@wezell wezell closed this Jun 26, 2026
@wezell wezell deleted the p36275-glm5-head branch June 26, 2026 19:07
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