One Character Too Few: Fixing Translation Updates in Directus

Merged: directus/directus#28001 · Issue: #27986 · Affected: Directus v12.1.1

Some bugs are exotic. This one was a comparison operator that was off by one, sitting in front of a database query that couldn't tell a row apart from itself. Together they made a whole category of legitimate edits impossible.

The report

bryantgillespie filed #27986: updating a single translation record through the REST API, the SDK, or the Data Studio would fail with

400 Bad Request — Duplicate key and language combination

…even when no other row in the table held that (key, language) pair. Labelled Bug / CRUD / Engine / Med Impact / Med Reach.

The trigger was specific and, once you see it, common: the payload had to contain both key and language. That's not an edge case. It's what the Studio sends when you edit a translation, and it's what every import or sync tool sends, because they ship the complete record rather than a diff. So in practice: open a translation, change the value, hit save, get a 400 about a duplicate that doesn't exist.

Reading the guard

Everything lives in api/src/services/translations.ts. TranslationsService overrides updateMany to protect the (key, language) uniqueness invariant:

override async updateMany(keys: PrimaryKey[], data: Partial<Item>, opts?: MutationOptions) {
  if (keys.length > 0 && 'key' in data && 'language' in data) {
    throw new InvalidPayloadError({ reason: 'Duplicate key and language combination' });
  } else if ('key' in data || 'language' in data) {
    const items = await this.readMany(keys);

    for (const item of items) {
      const updatedData = { ...item, ...data };

      if (await this.translationKeyExists(updatedData['key'], updatedData['language'])) {
        throw new InvalidPayloadError({ reason: 'Duplicate key and language combination' });
      }
    }
  }
  // ...
}

The intent of that first branch is sound. updateMany applies one payload to many rows. If that payload pins down both key and language, every targeted row collapses onto the same pair — a guaranteed collision, and you can reject it without touching the database.

The intent is right; the boundary is wrong. keys.length > 0 is true for a single row too, and a single row updated to one (key, language) pair collides with nothing. The condition wanted > 1.

The second defect

Fixing the operator alone doesn't fix the bug — it just moves execution into the else if branch, where the second defect waits:

private async translationKeyExists(key: string, language: string) {
  const result = await this.knex.select('id').from(this.collection).where({ key, language });
  return result.length > 0;
}

This asks "does any row hold this pair?" — and the row being updated is a row. Re-saving a translation without changing anything at all finds itself, counts itself as a duplicate, and throws. The check needed to ask a narrower question: does any other row hold this pair?

Two independent defects, both required to see the bug. That's usually why something obvious survives in a mature codebase: each piece looks defensible in isolation.

The fix

translationKeyExists gained an exclusion list:

private async translationKeyExists(key: string, language: string, excludeKeys: PrimaryKey[] = []) {
  const query = this.knex.select('id').from(this.collection).where({ key, language });

  if (excludeKeys.length > 0) {
    query.whereNotIn('id', excludeKeys);
  }

  const result = await query;
  return result.length > 0;
}

Optional parameter, default empty — every existing caller keeps its current behaviour.

What review caught

My first pass excluded the whole batch — every id in keys — from the lookup. It made the tests pass and it was wrong.

ComfortablyCoding pushed back over a few rounds, and the reasoning is the part of this worth remembering. Excluding the entire batch means the rows in that batch become invisible to each other. A multi-row update that collapses two of its own rows onto the same (key, language) pair would query the table, find nothing (because both offenders are excluded), and sail straight through into a broken write. I'd fixed a false negative by creating a false positive.

The correct shape is narrower on one axis and wider on the other:

• Per-row exclusion. Each row excludes only itself — [item['id']] — so it can't self-collide, but it can still collide with its batch siblings already in the table.

• In-memory duplicate tracking. For pairs that don't exist in the database yet but appear twice within the same batch, a Set catches the second occurrence before it's written.

• Fail fast on bulk. Keep the keys.length > 1 early throw so the guaranteed-collision case never issues a read at all.

if (keys.length > 1 && 'key' in data && 'language' in data) {
  throw new InvalidPayloadError({ reason: 'Duplicate key and language combination' });
} else if ('key' in data || 'language' in data) {
  const items = await this.readMany(keys);
  const seenCombinations = new Set<string>();

  for (const item of items) {
    const updatedData = { ...item, ...data };
    const combination = `${updatedData['key']}-${updatedData['language']}`;

    if (seenCombinations.has(combination)) {
      throw new InvalidPayloadError({ reason: 'Duplicate key and language combination' });
    }

    seenCombinations.add(combination);

    if (await this.translationKeyExists(updatedData['key'], updatedData['language'], [item['id']])) {
      throw new InvalidPayloadError({ reason: 'Duplicate key and language combination' });
    }
  }
}

Two collision sources, two checks: one against rows already on disk, one against rows in the current payload.

Tests

The service had no test file, so api/src/services/translations.test.ts is new — 118 lines covering the three behaviours that matter:

1. A single-row re-save carrying an identical key + language succeeds (the bug).

2. Updating a row onto a pair another row already owns still fails (the invariant).

3. A multi-row update that collapses several rows onto one pair still fails (the batch case, including the intra-batch variant that my first attempt would have let through).

That third case is the one that only exists because of the review. Every subsequent test run enforces the correction — which is the real payoff of a reviewer catching you.

Shipping it

Directus wants a few things beyond the diff, and they're easy to miss on a first PR:

• A changeset. pnpm changeset → pick the package (@directus/api), pick patch, describe the change in past tense. Mine: "Fixed TranslationsService.updateMany incorrectly rejecting single-row updates that included both key and language."

• The CLA. A bot prompts you; signing adds your name to contributors.yml in the same branch.

• Conventional commits and a fix/<issue>-<slug> branch name.

• pnpm lint && pnpm format before pushing.

Merged into main on 4 August 2026, tagged for the next release, with the review closing on "LGTM! Thanks and congratulations on your first contribution."

Takeaways

A guard's boundary is a design decision, not a detail. > 0 versus > 1 is one keystroke and the entire difference between "reject impossible writes" and "reject all writes." Worth stating the invariant in words first — a batch is only invalid if it targets more than one row — and then checking the operator against the sentence.

Uniqueness checks need to know who's asking. Any "does this value already exist?" query run during an update has to exclude the row being updated, or every no-op save is a conflict. It's a small, extremely repeatable class of bug.

The obvious widening of a fix is usually too wide. Excluding the batch instead of the row turned a false positive into a false negative. When you loosen a constraint to fix an over-rejection, the question to ask immediately is: what does this now let through?

Read the reviewer's objection for the invariant, not the instruction. ComfortablyCoding didn't hand me the Set. They pointed out that batch rows would stop seeing each other, and the Set fell out of that. Fixing the sentence beats applying the patch.