Repository navigation
docs(rest-api): fix App-group follow-ups from review (ENG-40977) - #592
pranavkamble-cometchat wants to merge 2 commits into
Conversation
Follow-up to #573 (ENG-40589), addressing @ketanyekale's review: - DELETE /apps/{appId}: replace the empty-"" wrapper + wrong deleteSchema with a direct $ref to deleteAppSchema ({success, scheduledToDeleteAt}), matching the example; response description "List Apps" -> "Delete App". - /apps/{appId}/usage: response description "Updated App" -> "App Usage"; example message "...processed for ." -> "...processed for <App Name>.". - "serverslocated" -> "servers located" typo. mint validates; still 40 paths, no unrelated endpoints touched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Review — docs(rest-api) App-group follow-ups (ENG-40977)Checked against the OAS-source PR, cometchat-team/mgmt-api#927. The two are in parity and this is correct. Verified
Non-blockingCarried over from the OAS repo: the empty- Minor: the checklist still has "I have checked for spelling and grammar errors" and "All links in my changes are valid and working" unticked. Both hold as far as I can tell — just tick them. |
|
@ketanyekale thanks for the review — confirming the docs side. All three fixes mirror mgmt-api cometchat-team/mgmt-api#927 and the
On the two pre-existing items from the mgmt review: |
Re-review (second pass) — same head,
|
ketanyekale
left a comment
There was a problem hiding this comment.
Approving — docs side
Re-checked at 44075458. Nothing outstanding from either of my passes; both items I raised on the second pass were explicitly non-blocking.
Verified independently this time
- Parity with cometchat-team/mgmt-api#927 — normalised comparison of
paths["/apps/{appId}"].deleteandpaths["/apps/{appId}/usage"]between this branch and the mgmt-api PR head: identical apart from this file'soperationId: delete-app./apps/{appId}/usageis identical outright. deleteAppSchemaresolves in this file'scomponents.schemasas{success: boolean, scheduledToDeleteAt: integer}, matching the example.deleteSchemais still referenced by other delete operations, so it correctly stays.- Typo — 0
serverslocatedremaining, 2 correctservers located. - Example placeholder — 0 occurrences of
processed for .left. - No build or nav risk — still 40 paths, spec parses clean, only
management-apis.jsonchanged.delete.mdxbinds asopenapi: delete /apps/{appId}, i.e. by method+path, so no slug, nav or redirect impact. All Mintlify checks green (deployment, link-rot, vale-spellcheck) plusvalidate-branch-name. - The upstream fix reproduces from source — on the mgmt-api side I ran the real generator end-to-end (swagger-php 4.11.1 per
composer.lock,--legacy, repo processor + bootstrap, thenformat_oas_output.php). TheDELETE /apps/{appId}and usage subtrees regenerate exactly as committed there, which is what this file mirrors. So this copy matches generated output, not just a hand edit.
Non-blocking, carried forward
- The empty-
""wrapper still affects 130 other response schemas in this file (list-collaborators,list-extensions, the webhook operations,list-apps'meta). Tracked as ENG-41012 — correct not to widen this PR. - The
delete.mdxsentence about the two-call behaviour (second DELETE on an already-scheduled app hard-deletes and returns onlysuccess) is still worth adding; I confirmed it again againstAppController.phpL1037-1118. Your call, and fine as a follow-up. - ENG-41013 covers the stale
ERR_APP_ARCHIVEDrow inerror-codes.mdx:14. Re-confirmed unreachable in the controller. - One thing to keep in mind for the ENG-41012 sweep:
mgmt-api-commit.shcopiesMgmt-REST-APIs.jsonverbatim over this file whenMINTIFY_REPO_PATHis set, but this file is not a verbatim copy today — 40 paths here vs 47 upstream, plus docs-onlyoperationIds on the App group. A naive sync would overwrite those. Nav binds by method+path so it would not break pages, but the sweep's regen should account for it.
Still worth ticking the two unticked checklist boxes; both hold as far as I can tell.
Approving.
ketanyekale
left a comment
There was a problem hiding this comment.
Requesting changes — retracting my approval, one line is wrong
Apologies for the churn: I approved this earlier today, then re-checked the fetch-usage example against the mgmt-api implementation and found my approval was premature. Everything else I verified still stands; this one line should not ship. Same note on cometchat-team/mgmt-api#927, where the OAS source lives.
The new usage example documents the wrong value
POST /apps/{appId}/usage's 200 example now reads:
Your usage request has been accepted and is being processed for
<App Name>. You will receive an email with the details at<emailID>@example.com
That field is not an app name. In mgmt-api the template is app/Lib/AppResponse/Success.php:197 — ...processed for {{billingCycle}}... — and billingCycle is filled at WrapperController.php:258 from $successMsg, which is assigned in exactly two places in that file plus its initialiser:
| source | value |
|---|---|
WrapperController.php:230 (default) |
current billing cycle (2026-10-01 - 2026-10-31) |
WrapperController.php:216 (usePreviousBillingCycle: true) |
previous billing cycle (2026-09-01 - 2026-09-30) |
WrapperController.php:66, no app subscription |
'' |
An app name never appears there. The dangling processed for . this PR set out to fix was actually a faithful rendering of that third case — it looked broken but was not wrong, whereas the replacement reads cleanly and misinforms. For a PR whose purpose is fixing misleading response docs, that is the worse failure mode.
The preview makes it self-contradictory on the page itself. On fetch-usage the <App Name> example sits directly below a request body whose only property is usePreviousBillingCycle, described as choosing which billing cycle to fetch. Anyone reading top to bottom hits the contradiction immediately.
Requested change
One line in management-apis.json, under paths["/apps/{appId}/usage"].post.responses.200.content["application/json"].example:
- "message": "Your usage request has been accepted and is being processed for <App Name>. You will receive an email with the details at <emailID>@example.com"
+ "message": "Your usage request has been accepted and is being processed for current billing cycle (2026-10-01 - 2026-10-31). You will receive an email with the details at <emailID>@example.com"Any phrasing that conveys a billing-cycle range rather than an app name is fine; the above just matches what the code emits. Keep it in step with the mgmt-api side so the two copies stay in parity.
Everything else: confirmed, with stronger evidence this round
Redocly lint, main vs this branch — this PR fixes exactly 3 problems and introduces 0:
no-invalid-media-type-examples #/paths/~1apps~1{appId}/delete/responses/200/.../example/data/success
no-invalid-media-type-examples #/paths/~1apps~1{appId}/delete/responses/200/.../example/data/scheduledToDeleteAt
no-unused-components #/components/schemas/deleteAppSchema ("never used")
The old {success, message} schema provably contradicted its own example, and deleteAppSchema is provably no longer orphaned.
The delete page renders correctly on the preview — delete shows "Delete App" as the 200 description and { "success": true, "scheduledToDeleteAt": 1631203178 } as the example, with data as a plain object. That is the customer-facing defect from the original review, fixed and verified in the rendered output rather than only in the spec.
Also still true: 40 paths, spec parses clean, pages bind by method+path so no nav/slug/redirect impact, all Mintlify checks green, and parity with mgmt-api#927 holds apart from the docs-only operationId.
Non-blocking, unchanged: the empty-"" wrapper still affects 130 other response schemas here (ENG-41012); the delete.mdx sentence on the two-call delete behaviour is still worth adding; ERR_APP_ARCHIVED in error-codes.mdx:14 is ENG-41013. And the two unticked checklist boxes.
Re-approving as soon as the example line is corrected.
…ement API
Twelve endpoints carried copy-paste response descriptions that disagreed
with the operation they document, so the rendered response panel
announced the wrong action. Each new value matches that operation's own
summary and description:
- GET /apps/{appId}/extensions Enabled/Disabled Extension -> List Extensions
- PUT .../widget/v2/settings Create Chat Widget -> Update Chat Widget
- GET .../widget/v2/settings Create Chat Widget -> List Chat Widgets
- PUT .../gifs-giphy/v1/settings Create Chat Widget -> Update Giphy Settings
- GET .../xss-filter/v1/settings Update XSS filters Settings -> Get XSS filters Settings
- DELETE .../image-moderation/v1/settings Get Image Moderation Settings -> Delete Image Moderation Settings
- GET .../sentiment-analysis/v1/settings Update Sentiment analysis Settings -> Get Sentiment analysis Settings
- PUT .../virus-malware-scanner/v1/settings Create Chat Widget -> Update Virus Malware Scanner Settings
- POST .../push-notification/v1/upload-certificate-p12 Query Certificate -> Upload Certificate
- GET /apps/{appId}/collaborators Created App -> List Collaborators
- DELETE /apps/{appId}/webhooks/{webhookId}/triggers Enabled Triggers -> Remove Triggers
- POST /tenants Map Settings -> Add Tenant
Response descriptions only. operationId, summary and tags are untouched,
so every derived page slug is unchanged and no navigation entry or
redirect is affected. Still 40 paths, no dangling $refs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to #573 (ENG-40589), addressing @ketanyekale's review:
mint validates; still 40 paths, no unrelated endpoints touched.
Description
Related Issue(s)
Type of Change
Checklist
Additional Information
Screenshots (if applicable)