fix(progresses): prevent duplicate progress documents via atomic create (#1158)

[JSap0914]

Issue Ticket Number

Closes #1158

Description

createProgressDocument created duplicate documents in the progresses collection under concurrent load. It used a check-then-add sequence: query for an existing progress document for the current day and, if none was found, call progressesCollection.add() with an auto-generated id. Two requests made within the same second can both observe an empty result and each insert a document, producing the duplicates reported in #1158.

Fix: derive a deterministic document id from the uniqueness key (type + user/task id + day timestamp) and insert with an atomic create(). Firestore's create() fails with ALREADY_EXISTS when a document with that id already exists, so concurrent creates for the same user/task and day can never both succeed. The losing request is surfaced as the existing 409 Conflict (... Progress for the day has already been created.), preserving current API behaviour. The pre-write query check is retained as a fast path.

Regression tests added in test/unit/models/progresses.test.js:

• fires two concurrent createProgressDocument calls for the same user/day via Promise.allSettled and asserts exactly one document persists (one fulfilled, one 409);
• asserts a sequential duplicate for the same day still throws 409 and leaves a single document.

Documentation Updated?

• Yes
• No

Under Feature Flag

• Yes
• No

Database Changes

• Yes
• No

New progress documents now use a deterministic id (type:id:date) instead of an auto-generated id. Existing documents are unaffected; reads are by query, not by id.

Breaking Changes

• Yes
• No

Test Coverage

Focused unit run against the Firestore emulator:

progressModel
  createProgressDocument
    ✔ stores exactly one progress document when two requests race for the same day
    ✔ throws a Conflict when a progress document already exists for the user on the same day

2 passing

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 76d374c6-26dd-42a3-ab4e-6fe7faefbed5

📥 Commits

Reviewing files that changed from the base of the PR and between 227988d and 7d60d31.

📒 Files selected for processing (2)

• models/progresses.js
• test/unit/models/progresses.test.js

---

Summary by CodeRabbit

• Bug Fixes

  • Prevented duplicate progress records when requests are made concurrently or repeatedly for the same user and day.
  • Duplicate progress submissions now return a clear 409 Conflict response.

• Tests

  • Added coverage for concurrent and sequential duplicate progress creation scenarios.

Walkthrough

Progress creation now derives deterministic daily document IDs and uses atomic Firestore creation. Existing-document conflicts become 409 errors, with tests covering concurrent and sequential duplicate attempts.

Changes

Progress creation

Layer / File(s)	Summary
Atomic progress document creation models/progresses.js	createProgressDocument derives an ID from the progress uniqueness fields, uses docRef.create(), and converts Firestore already-exists errors into Conflict.
Duplicate creation validation test/unit/models/progresses.test.js	Tests verify concurrent and sequential duplicate attempts result in one stored document and the expected 409 response.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RequestA
  participant RequestB
  participant Firestore
  RequestA->>Firestore: Create deterministic progress document
  RequestB->>Firestore: Create deterministic progress document
  Firestore-->>RequestA: Creation succeeds
  Firestore-->>RequestB: Already-exists error mapped to 409 Conflict

Loading

Poem

A rabbit guards the daily page,
One ID keeps the race in cage.
Two paws knock, one gets through,
The other hears, “Already knew.”
One progress leaf remains in view.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: preventing duplicate progress documents with atomic create.
Description check	✅ Passed	The description is directly related to the code changes and regression tests in the pull request.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}