Skip to content

fix(postman): fall back to English (id=1) when _getdefault returns invalid id (#35780)#35817

Open
dsolistorres wants to merge 1 commit into
mainfrom
issue-35780-postman-language-fallback
Open

fix(postman): fall back to English (id=1) when _getdefault returns invalid id (#35780)#35817
dsolistorres wants to merge 1 commit into
mainfrom
issue-35780-postman-language-fallback

Conversation

@dsolistorres
Copy link
Copy Markdown
Member

Summary

Follow-up to PR #35795. The prerequest script we added there accepts any non-falsy value from GET /api/v2/languages/_getdefault and stores it as defaultLanguageId. In some test environments the endpoint returns the LANG__404 sentinel with id=-1, which our guard treated as valid. Test bodies then sent \"languageId\": -1 and the server FK-failed on contentlet insert.

Evidence from run 26312707306 (PR #35771)

GET http://localhost:8080/api/v2/languages/_getdefault [200 OK, 925B, 28ms]
'defaultLanguageId = -1'
...
ERROR:  insert or update on table "contentlet" violates foreign key constraint "fk_contentlet_lang"
Detail: Key (language_id)=(-1) is not present in table "language".

The failing test (Page API / Test DotAsset as titleImage / Create DotAsset image) had defaultLanguageId = -1 substituted into its body. Server's _getdefault returned the sentinel, our script blindly trusted it.

Fix

Tighten the existing guard and add a fallback to English (id=1) — always present in dotCMS starter data, matches the same approach as #35813:

-const id = body && body.entity && body.entity.id;
-if (!id) {
-    throw new Error('Could not resolve default language: entity.id missing in response');
+let id = body && body.entity && body.entity.id;
+if (!id || id <= 0) {
+    // _getdefault can return the LANG__404 sentinel (id=-1) in fresh
+    // test environments. Fall back to English (id=1), which is always
+    // present in dotCMS starter data. See issue #35780.
+    console.log('Default language returned invalid id (' + id + '), falling back to English (id=1)');
+    id = 1;
 }

Network errors and non-200 responses still throw (no silent swallowing) — only invalid-id payloads fall back.

Scope

Test plan

  • JSON validity verified for both files.
  • Diff is surgical: 14 inserts / 6 deletes total.
  • CI verifies — PR Test / Postman Tests - GraphQL and Workflow should pass even when _getdefault returns the sentinel.

🤖 Generated with Claude Code

…valid id (#35780)

PR #35795 added a prerequest script to Workflow_Resource_Tests and GraphQLTests
that calls GET /api/v2/languages/_getdefault to resolve a real language id. The
script's existing guard accepted ANY non-falsy value — but in some test
environments _getdefault returns the LANG__404 sentinel with id=-1. The script
then stored -1 in defaultLanguageId, the test body sent "languageId": -1, and
the server FK-failed on contentlet insert (Key (language_id)=(-1) is not
present in table "language").

Observed in run 26312707306:

  GET /api/v2/languages/_getdefault [200 OK]
  defaultLanguageId = -1
  ...
  ERROR: insert or update on table "contentlet" violates foreign key
  constraint "fk_contentlet_lang"

This tightens the guard (id <= 0 also triggers the fallback) and falls back
to English (id=1) — which is always present in dotCMS starter data. Network /
auth errors still throw, only invalid-id responses fall back.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 22, 2026

Claude finished @dsolistorres's task in 1m 18s —— View job


Review

  • Read changed files
  • Verify diff matches the description
  • Check for edge cases / risks
  • Post review

Verdict: looks correct and minimal. The guard change (!id || id <= 0) catches the -1 LANG__404 sentinel, the fallback to id=1 is justified by the inline comment, and the original throws for network/HTTP errors are preserved so non-payload failures still surface. JS coercion makes id <= 0 work whether the server returns -1 as a number or "-1" as a string. Both files were kept in sync.

Worth flagging (non-blocking)

  1. Script duplication is growing. This prerequest script now lives in GraphQLTests.json:14860 and Workflow_Resource_Tests.json:19833, and the PR mentions a third copy is coming via fix(postman): pass explicit language in AI Search Related by identifier (#35780) #35816. Three identical 30-line embedded JS blocks in Postman JSON is a future drift hazard — the next bug fix has to touch all three, and there is no compiler to catch a missed file. Not something to fix in this PR, but worth a tracking issue: either move this to a Postman collection-level pre-request (parent folder), or generate the JSON from a shared snippet at build time.

  2. id = 1 is an environment assumption, not a fact. The comment says English is "always present in dotCMS starter data," which is true for fresh installs, but a test environment that has deleted or reseeded languages without id=1 would still FK-fail with exactly the same symptom — just less obviously, because the fallback masks the misconfig. Acceptable trade-off for tests, but if anyone hits this again the script's console.log is the only breadcrumb. Consider also asserting via pm.test so the run fails loudly rather than silently picking 1, e.g.:

    console.warn('Default language returned invalid id (' + id + '), falling back to English (id=1) — server may be misconfigured');

    console.warn over console.log would make this stand out in Newman output.

  3. Type of id is not normalized. entity.id comes back as a JSON number, but if it ever returned a numeric string the env var stored would be a string. That has not been a problem historically, just noting that id = Number(id) (or parseInt(id, 10)) before storing would make the contract explicit. Low priority.

  4. No coverage for the sentinel path itself. There's no test that asserts the fallback is exercised when _getdefault returns id=-1 — only the in-the-wild CI failure validates it. If the sentinel value ever changes (e.g. becomes 0 or null), the guard might miss it again. A Postman test in LanguageResourceTests.json that calls _getdefault on a fresh env and asserts the script's behavior would be defensive. Probably overkill for now.

Nits

  • The block is now 14/6 instead of 7/3 because the comment is included in both copies. Fine.
  • let id is needed (reassigned) — correct change from const.
    · branch issue-35780-postman-language-fallback

dsolistorres added a commit that referenced this pull request May 22, 2026
…valid id

Same fallback shipping for Workflow_Resource_Tests + GraphQLTests in #35817.
Applied here so AI is consistent before this PR merges.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant