fix: normalize funding receipt from/to/for field types - #186
Conversation
🦋 Changeset detectedLatest commit: 4408f61 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR normalizes the Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
87cc73d to
216ba8f
Compare
…rading to strings Move from/to toward the structured ref pattern (union of app.certified.defs#did and com.atproto.repo.strongRef), matching the badge award subject pattern. Keep from optional for anonymous senders. Keep for as strongRef. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This PR implemented it in the wrong direction. During the call we
This is a breaking change. |
|
As we didn't realize that this is a breaking change during the call, this PR will be deferred until I can check that the breaking change is not too breaking |
|
Marked as draft to align with the above comments. |
Thanks Adam. Just synced with Satyam, and this is good to merge. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/validate-funding-receipt.test.ts`:
- Around line 133-136: The test currently passes an invalid `to` shape so
validation can fail for the wrong reason; update the "should reject a record
missing the required 'createdAt' field" test to use a valid `to` object shape
matching the updated union fixtures (instead of the current `{ to: { did: ... }
}`) and remove only the `createdAt` property so the validator (call site:
validate, schema identifier: ids.OrgHypercertsFundingReceipt) fails specifically
due to the missing `createdAt` field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 291383bb-c787-481b-8f40-63462e2acff5
📒 Files selected for processing (4)
.changeset/funding-receipt-field-types.mdSCHEMAS.mdlexicons/org/hypercerts/funding/receipt.jsontests/validate-funding-receipt.test.ts
The test was using `{ did: ... }` without `$type`, causing validation
to fail for the wrong reason (invalid `to` union discriminator) rather
than specifically for the missing `createdAt` field.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
fromandtoonfunding.receiptare now both plain strings accepting a DID or name — symmetric and consistent with howcontributorInformation.identifierworks elsewhere in the protocol.fromis now optional (removed fromrequired), allowing anonymous funding.forchanged fromat-uristring tostrongReffor content-hash verification, consistent with all other cross-record references in the protocol.Why
from/to symmetry:
fromwas a typed DID ref whiletowas a free-text string. This asymmetry forced the sender to have an AT Protocol identity while the recipient could be anyone — the opposite of how funding often works (anonymous donors, named recipients). Making both plain strings with the same constraints treats senders and recipients uniformly.from optional: Required
fromprevented recording anonymous donations. The previous workaround ("leave empty") contradicted the required constraint.for as strongRef: An
at-urionly identifies a record by path — it doesn't pin to a specific version. If the referenced activity is updated, the funding receipt's meaning could drift.strongRefincludes a content hash (CID), ensuring the receipt always points to the exact state of the activity it funded.Test plan
npm run checkpasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
fromis now optional and may be a DID or a strong reference (allowing omission),toaccepts DID or strong references instead of plain text, andforis a structured strong reference for stronger linkage.Tests
Chores