Skip to content

Feature/refund eligibility helper - #145

Open
laxjovial wants to merge 7 commits into
karagozemin:masterfrom
laxjovial:feature/refund-eligibility-helper
Open

Feature/refund eligibility helper#145
laxjovial wants to merge 7 commits into
karagozemin:masterfrom
laxjovial:feature/refund-eligibility-helper

Conversation

@laxjovial

Copy link
Copy Markdown
Contributor

closes #72

@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the karagoz's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Jun 29, 2026

Copy link
Copy Markdown

@laxjovial Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the refund eligibility helper work. I checked the latest commit, but this is not mergeable yet.

Commands run:

pnpm install --frozen-lockfile        # passed
pnpm --filter @oversync/sdk test     # failed
pnpm --filter @oversync/sdk build    # failed
git diff --check origin/master...HEAD # failed

Blocking failures:

  1. packages/sdk/src/ethereum/index.ts has a syntax error around claimOrder, so both tests and tsc fail:
packages/sdk/src/ethereum/index.ts:110:18: ERROR: Expected "=>" but found "("
src/ethereum/index.ts(110,3): error TS1434: Unexpected keyword or identifier.
  1. Existing Soroban order parsing tests now throw OverSyncError is not defined instead of the expected validation messages:
test/soroban-order.test.ts: 6 failed
expected ... to throw error including 'expected an object' but got 'OverSyncError is not defined'\n```\n\n3. `git diff --check origin/master...HEAD` reports trailing whitespace in:\n\n```\npackages/sdk/src/errors/index.ts:29\npackages/sdk/src/errors/index.ts:34\npackages/sdk/src/errors/index.ts:38\npackages/sdk/src/utils/explorer.ts:6\npackages/sdk/src/utils/explorer.ts:12\npackages/sdk/src/utils/explorer.ts:29\npackages/sdk/src/utils/explorer.ts:35\npackages/sdk/src/utils/explorer.ts:52\npackages/sdk/src/utils/explorer.ts:58\npackages/sdk/test/errors.test.ts:39\npackages/sdk/test/refunds.test.ts:89\npackages/sdk/test/refunds.test.ts:91\n```\n\nPlease fix these and re-run SDK test/build before I re-check.

@laxjovial

Copy link
Copy Markdown
Contributor Author

done , that . let me know if any other issues. also, im on the other issues

@karagozemin

Copy link
Copy Markdown
Owner

I checked the current PR state before reviewing code, and GitHub still reports this branch as CONFLICTING / DIRTY against current master. I cannot merge or give a final code approval while the branch cannot be merged.

Please rebase or merge latest master, resolve the conflicts, and push the updated branch. Once GitHub reports it as mergeable, I can run the relevant package tests and re-review.

@laxjovial

Copy link
Copy Markdown
Contributor Author

done please review

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for rebasing the refund eligibility helper PR. I rechecked this under the karagozemin account, but the PR is blocked before SDK tests because whitespace validation fails:

git diff --check origin/master...HEAD
packages/sdk/src/errors/index.ts:29: trailing whitespace.
packages/sdk/src/errors/index.ts:34: trailing whitespace.
packages/sdk/src/errors/index.ts:38: trailing whitespace.
packages/sdk/src/utils/explorer.ts:6: trailing whitespace.
packages/sdk/src/utils/explorer.ts:12: trailing whitespace.
packages/sdk/src/utils/explorer.ts:29: trailing whitespace.
packages/sdk/src/utils/explorer.ts:35: trailing whitespace.
packages/sdk/src/utils/explorer.ts:52: trailing whitespace.
packages/sdk/src/utils/explorer.ts:58: trailing whitespace.
packages/sdk/test/errors.test.ts:39: trailing whitespace.
packages/sdk/test/refunds.test.ts:89: trailing whitespace.
packages/sdk/test/refunds.test.ts:91: trailing whitespace.

Please strip trailing whitespace across the SDK files, then rerun:

  • git diff --check origin/master...HEAD
  • pnpm --filter @oversync/sdk test
  • pnpm --filter @oversync/sdk build

After diff check is clean I can continue the functional review.

@laxjovial

Copy link
Copy Markdown
Contributor Author

i have reviewed and worked , better now?

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the update. I rechecked the merge ref from the maintainer account and the SDK checks are green:

git diff --check master...pr-145-merge
pnpm --filter @oversync/sdk test
pnpm --filter @oversync/sdk build

Result: SDK tests pass (75) and SDK build passes.

I still cannot merge this exact PR because the diff is not scoped to #72. It includes refund eligibility, but also adds the explorer URL helper files/tests from #73 and SDK error normalization files/tests from #78:

packages/sdk/src/errors/index.ts
packages/sdk/src/utils/explorer.ts
packages/sdk/test/errors.test.ts
packages/sdk/test/explorer.test.ts

Those already have separate open PRs (#143 and #142). Please narrow this branch to the refund eligibility helper/tests only, or explicitly consolidate/close the other two PRs and link all affected issues in one PR. Once the scope is clean, the functional checks look ready.

@laxjovial

Copy link
Copy Markdown
Contributor Author

All three of my PR branches are now cleanly scoped to their respective issues.

@laxjovial

Copy link
Copy Markdown
Contributor Author

@karagozemin

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the contribution! ✅ Logic looks fine, but pnpm build fails in packages/sdk because two barrel imports point to files that aren't in the PR:

src/index.ts(5,15): error TS2307: Cannot find module './errors/index.js'
src/utils/index.tsrc/utils/index.t2307: Cansrc/utils/index.tsrc/plosrc/utils/index.tsrc/utadsrc/utils/index.tsrc/utils/index.t2307: Cansrc/utils/index.tsrc/plosrc/utils/index.tsrc/utadsrc/utils/index.tsrc/utils/index.t2307: Cansrc/utils/index.tsrc/plosrc/utils/index.tsrc/utadsrc/utils/index.tsrc/utils/index.t2307: Cansrc/utils/index.tsrc/plosrc/utils/index.tsrc/utadsrc/utils/index.tsrc/utils/index.t2307: Cansrc/utils/index.tsrc/plosrc/utils/index.tsrc/utadsrc/utils/index.tsrc/utils/index.t2307: Cansrc/utils/index.tsrc/plosrc/utils/index.tsrc/utadsrc/utils/index.tsrc/utils/index.t2307: Cansrc/utils/index.tsrc/plosrc/utils/index.tsrc/utadsrc/utils/index.tsrc/utils/index.t2307: Cansrc/utils/index.tsrc/plosrc/utils/index.tsrc/utadsrc/utils/iocally ✅, but GitHub reports **merge conflicts** with the base branch. Could you rebase onto the latest `master` and resolve the conflicts? Once it's mergeable I'll get it merged. 🙏

@laxjovial
laxjovial force-pushed the feature/refund-eligibility-helper branch from c458b2d to ff79cac Compare July 6, 2026 07:30
@laxjovial
laxjovial force-pushed the feature/refund-eligibility-helper branch from afa4ce8 to ff79cac Compare July 6, 2026 07:32
@laxjovial

Copy link
Copy Markdown
Contributor Author

done @karagozemin

@laxjovial
laxjovial force-pushed the feature/refund-eligibility-helper branch from 0c68542 to 6c54cf3 Compare July 6, 2026 07:54
@laxjovial

Copy link
Copy Markdown
Contributor Author

@karagozemin come and see

@karagozemin

karagozemin commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Rechecked: thanks for the updates. However this PR now has merge conflicts with main, so I cannot merge it as-is. Could you rebase/merge the latest main, resolve the conflicts and push? I will take another look and merge afterwards.

@laxjovial

Copy link
Copy Markdown
Contributor Author

Done

@laxjovial

Copy link
Copy Markdown
Contributor Author

Uploading IMG_8507.png…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Wave 7 3/10] Read-only refund eligibility endpoint for transaction history

2 participants