Skip to content

Fix plural forms structure in POEditor scripts #9342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DawoudIO wants to merge 3 commits into
base: master
Choose a base branch
from
+59 −1
Open

Fix plural forms structure in POEditor scripts #9342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
891b910
Fix plural forms structure in POEditor scripts
DawoudIO Aug 1, 2026
b2b0bde
Fix failing payment submission tests - add missing CHECK method selec…
DawoudIO Aug 1, 2026
156b32c
Fix: allow empty strings in restructurePluralForms() null check
DawoudIO Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cypress/e2e/ui/finance/finance.payment-submission.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ describe("Finance Payment Submission - Issue #7257 Regression Test", () => {
// Provide a family (required by the new pledge editor form)
cy.get("#FamilyID").invoke("val", "1");

cy.get("#Method").select("CHECK");
cy.get(".fund-select").first().select(1);
cy.get("#CheckNo").type(checkNumber); // Fill check number first

Expand Down Expand Up @@ -155,6 +156,7 @@ describe("Finance Payment Submission - Issue #7257 Regression Test", () => {
// Provide a family (required by the new pledge editor form)
cy.get("#FamilyID").invoke("val", "1");

cy.get("#Method").select("CHECK");
cy.get("#CheckNo").type(checkNumber); // Fill check number first

// Fill payment form with multiple funds
Expand Down
28 changes: 28 additions & 0 deletions locale/scripts/poeditor-downloader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,39 @@ async function fetchUntranslatedTerms(poEditorLocale) {
});
}

/**
* Restructure POEditor export from malformed to correct format for upload.
* POEditor returns: { "one": { term1: "...", term2: "..." }, "other": { ... } }
* We need: { "term1": { "one": "...", "other": "..." }, "term2": { ... } }
*/
function restructurePluralForms(data) {
if (!data.one && !data.other) return data;
if (typeof data.one === 'object' && typeof data.other === 'object') {
const restructured = {};
for (const [key, value] of Object.entries(data)) {
if (key !== 'one' && key !== 'other' && typeof value === 'string') {
restructured[key] = value;
}
}
const termKeys = new Set([...Object.keys(data.one || {}), ...Object.keys(data.other || {})]);
for (const term of termKeys) {
const pluralForms = {};
if (data.one != null && data.one[term] !== undefined && data.one[term] !== null) pluralForms.one = data.one[term];
if (data.other != null && data.other[term] !== undefined && data.other[term] !== null) pluralForms.other = data.other[term];
if (Object.keys(pluralForms).length > 0) restructured[term] = pluralForms;
}
return restructured;
}
return data;
}

@DawoudIO DawoudIO Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated PR Review — this comment was posted by the pr-reviewer bot, not by a human or the implementer agent

[HIGH] Truthy check silently drops all untranslated plural terms

if (data.one && data.one[term]) pluralForms.one = data.one[term];
if (data.other && data.other[term]) pluralForms.other = data.other[term];
if (Object.keys(pluralForms).length > 0) restructured[term] = pluralForms;

POEditor returns "" (empty string) for every untranslated term — exactly the case this function is meant to handle. Empty strings are falsy in JavaScript, so data.one[term] always fails the guard, pluralForms stays {}, and every term is silently discarded. The function returns {} for its primary input, causing saveBatchedMissingTerms to write empty batch files for all plural-form locales.

Fix:

if (data.one != null && data.one[term] !== undefined && data.one[term] !== null)
    pluralForms.one = data.one[term];
if (data.other != null && data.other[term] !== undefined && data.other[term] !== null)
    pluralForms.other = data.other[term];

@DawoudIO DawoudIO Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated PR Review — this comment was posted by the pr-reviewer bot, not by a human or the implementer agent

[HIGH] Truthy check silently drops all untranslated plural terms — still unaddressed

if (data.one && data.one[term]) pluralForms.one = data.one[term];   // "" is falsy!
if (data.other && data.other[term]) pluralForms.other = data.other[term];
if (Object.keys(pluralForms).length > 0) restructured[term] = pluralForms;

POEditor returns "" for every untranslated term. "" is falsy → pluralForms stays {} → term silently dropped → function returns {} → saveBatchedMissingTerms writes empty batch files for every plural-form locale.

Fix:

if (data.one != null && data.one[term] !== undefined && data.one[term] !== null)
    pluralForms.one = data.one[term];
if (data.other != null && data.other[term] !== undefined && data.other[term] !== null)
    pluralForms.other = data.other[term];


/**
* Save missing terms in batched JSON files under MISSING_OUTPUT_DIR/{poEditorCode}/
* Returns array of written file paths.
*/
function saveBatchedMissingTerms(poEditorCode, missingTerms) {
missingTerms = restructurePluralForms(missingTerms);

const localeOutDir = path.join(MISSING_OUTPUT_DIR, poEditorCode);

if (!fs.existsSync(localeOutDir)) {
Expand Down
30 changes: 29 additions & 1 deletion locale/scripts/poeditor-upload-missing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,33 @@ function loadEnglishOkAllowlist() {
}
}

// ── Plural form restructuring ───────────────────────────────────────────────

/**
* Defensive restructure for malformed plural forms from POEditor export.
* If batch file has top-level "one"/"other", fixes to proper nesting.
*/
function restructurePluralForms(data) {
if (!data.one && !data.other) return data;
if (typeof data.one === 'object' && typeof data.other === 'object') {
const restructured = {};
for (const [key, value] of Object.entries(data)) {
if (key !== 'one' && key !== 'other' && typeof value === 'string') {
restructured[key] = value;
}
}
const termKeys = new Set([...Object.keys(data.one || {}), ...Object.keys(data.other || {})]);
for (const term of termKeys) {
const pluralForms = {};
if (data.one != null && data.one[term] !== undefined && data.one[term] !== null) pluralForms.one = data.one[term];
if (data.other != null && data.other[term] !== undefined && data.other[term] !== null) pluralForms.other = data.other[term];
if (Object.keys(pluralForms).length > 0) restructured[term] = pluralForms;
}
return restructured;
}
return data;
}

@DawoudIO DawoudIO Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated PR Review — this comment was posted by the pr-reviewer bot, not by a human or the implementer agent

[HIGH] Same truthy-check bug silently drops all plural terms during analysis

Identical logic to the downloader: data.one[term] is "" for untranslated terms → falsy → pluralForms never populated → term discarded → analyzeFile returns empty localizedTerms, suspectTerms, and emptyTerms for every plural-form batch file. The script reports "0 terms to upload" even when terms exist, and the emptyTerms bucket — which should catch untranslated terms — is never reachable.

The fix is the same: replace truthiness checks with explicit !== undefined && !== null guards to allow empty strings through.

@DawoudIO DawoudIO Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated PR Review — this comment was posted by the pr-reviewer bot, not by a human or the implementer agent

[HIGH] Same truthy-check bug — analyzeFile can never populate emptyTerms for plural-form locales

Identical logic to the downloader. After raw = restructurePluralForms(raw), all plural-form terms are silently dropped and Object.entries(raw) iterates over nothing. emptyTerms is never populated, so the upload script reports "0 terms to upload" for any locale that has only untranslated plural-form terms.

Apply the same null-guard fix as in the downloader.


// ── Term analysis ────────────────────────────────────────────────────────────

/**
Expand Down Expand Up @@ -229,7 +256,8 @@ function isNotIdenticalToKey(termKey, value) {
* @param {Set<string>} [englishOkSet] - optional set of terms safe to upload as-is
*/
function analyzeFile(filePath, englishOkSet = new Set()) {
const raw = JSON.parse(fs.readFileSync(filePath, 'utf8'));
let raw = JSON.parse(fs.readFileSync(filePath, 'utf8'));
raw = restructurePluralForms(raw); // Defensive: fix malformed plural structure if present
const localizedTerms = {};
const suspectTerms = {};
const emptyTerms = {};
Expand Down
Loading