fix: complete working booking flow integration tests#148
fix: complete working booking flow integration tests#148respp merged 5 commits intoStellar-Rent:mainfrom
Conversation
WalkthroughAdds backend integration test scaffolding, Jest blockchain mocks, test fixtures, a new npm test:int script and docs; introduces a PUT booking confirmation route and updates confirmPayment to validate bookingId from URL params and require a sourcePublicKey field. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Router as Express Router
participant Validator as validateConfirmPayment
participant Controller as BookingController
participant DB as Database
participant Chain as Blockchain/Soroban
Note over Client,Router: Confirm via POST /:bookingId/confirm-payment or PUT /:bookingId/confirm
Client->>Router: HTTP Confirm (bookingId in URL, body with tx data)
Router->>Validator: validateConfirmPayment
Validator-->>Router: valid / invalid
alt invalid
Router-->>Client: 400 Bad Request
else valid
Router->>Controller: confirmPayment(req)
Controller->>Controller: BookingParamsSchema.safeParse(req.params)
alt params invalid
Controller-->>Client: 400 Invalid bookingId
else params valid
Controller->>DB: fetch booking by id
DB-->>Controller: booking
Controller->>Chain: verify transaction (txHash, sourcePublicKey)
Chain-->>Controller: verification result
alt verified
Controller->>DB: update booking -> confirmed
DB-->>Controller: updated booking
Controller-->>Client: 200 Confirmed
else not verified
Controller-->>Client: 400 Verification failed
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/backend/src/controllers/booking.controller.ts (1)
206-211: Potential bug: escrow address property name mismatch (snake vs camel).
getBookingByIdappears to return camelCase elsewhere (validated byBookingResponseSchema), but here you readescrow_address. This can causeexpectedDestinationto be undefined and trigger a 500.Apply this defensive fix:
- const expectedDestination = bookingDetails.escrow_address; + const expectedDestination = + (bookingDetails as any).escrowAddress ?? (bookingDetails as any).escrow_address;Optionally, standardize service return shapes to camelCase and adjust callers accordingly.
apps/backend/tests/fixtures/booking.fixtures.ts (1)
365-456: Use ESM import for jsonwebtoken and convert fixture IDs to UUIDs
- In apps/backend/tests/fixtures/booking.fixtures.ts, add
import jwt from 'jsonwebtoken';
at the top and remove therequire('jsonwebtoken')call ingenerateAuthToken.- Replace all non-UUID IDs in this file—specifically
conflict-property-123,double-conflict-property-789, andinvalid-property-id—with valid UUIDv4 strings to satisfyz.string().uuid()validators.
🧹 Nitpick comments (8)
apps/backend/tests/mocks/blockchain.mocks.ts (4)
23-35: Add __esModule to the TrustlessWork mock for ESM interop.Prevents edge cases with mixed CJS/ESM imports.
Apply this diff:
return { - // Class constructor returns the mocked instance + __esModule: true, + // Class constructor returns the mocked instance TrustlessWorkClient: jest.fn().mockImplementation(() => mock),
5-16: Strengthen types for mockedTrustlessWork to match the real client.Typing with
unknown[]/Promise<unknown>weakens safety. Consider a type-only import andjest.Mocked<Pick<...>>so signatures and call args are enforced at compile time.Example:
// type-only import, no runtime coupling import type { TrustlessWorkClient } from '../../src/blockchain/trustlessWork'; type MockedTlw = jest.Mocked<Pick< TrustlessWorkClient, 'createEscrow' | 'getEscrowStatus' | 'fundEscrow' | 'releaseEscrow' | 'cancelEscrow' >> & { verifyTransaction: jest.Mock<Promise<boolean>, any[]> }; export const mockedTrustlessWork: MockedTlw = { /* ... */ };
37-40: Optionally set a sane default for verifyStellarTransaction.Defaults help tests pass deterministically when a mock isn’t explicitly configured.
Apply this diff:
export const mockedSoroban = { - verifyStellarTransaction: jest.fn<Promise<boolean>, unknown[]>(), + verifyStellarTransaction: jest.fn<Promise<boolean>, unknown[]>().mockResolvedValue(true), };
27-34: Align mocked named exports with the real module’s surface.If
trustlessWorkClientis not a named export in the real module, consider removing it to avoid confusion. Keep only the class and actual named functions you expect SUT to import.Apply this diff if not exported by the real module:
- // Named exports proxy to the mocked instance methods if referenced directly - trustlessWorkClient: mock, + // Named exports proxy to mocked instance methods if referenced directlyapps/backend/package.json (1)
13-13: Add CI note and consider a longer timeout (90s) to match plan.
- Ensure CI runners have Bun installed; the script invokes
bun testeven when run via npm.- Consider
--timeout=90000to align with task.md’s "~90s" target and reduce flakiness.Apply this diff if you agree:
- "test:int": "bun test tests/integration --timeout=60000", + "test:int": "bun test tests/integration --timeout=90000",apps/backend/src/controllers/booking.controller.ts (1)
115-160: Avoid string-matching error messages; prefer typed errors/codes.Mapping generic
Error.messagestrings to status codes is brittle. Use domain error types or error codes (likeBookingError) for consistent mapping.apps/backend/src/types/booking.types.ts (1)
76-80: Stellar public key validation added—tighten with a regex.Great to require length 56 and prefix 'G'. Consider a stricter regex (base32 charset) to prevent subtle invalid keys.
Example:
const STELLAR_PUBKEY_RE = /^G[A-Z2-7]{55}$/; export const confirmPaymentSchema = z.object({ transactionHash: z.string().min(10, 'Transaction hash is required and must be valid'), sourcePublicKey: z.string().regex(STELLAR_PUBKEY_RE, 'Invalid Stellar public key'), });Note: BookingResponseSchema.status currently omits 'ongoing' while other parts accept it; align enums to avoid 500s on valid states.
apps/backend/src/routes/booking.routes.ts (1)
36-38: PUT confirm route addition LGTM; plan deprecation for POST.Keeping POST for compatibility is fine. Consider documenting deprecation for
/:bookingId/confirm-paymentand consolidating to one route to reduce surface area.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/backend/README.md(1 hunks)apps/backend/package.json(1 hunks)apps/backend/src/controllers/booking.controller.ts(5 hunks)apps/backend/src/routes/booking.routes.ts(1 hunks)apps/backend/src/types/booking.types.ts(1 hunks)apps/backend/tests/fixtures/booking.fixtures.ts(2 hunks)apps/backend/tests/integration/booking-flow.int.test.ts(1 hunks)apps/backend/tests/mocks/blockchain.mocks.ts(1 hunks)task.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/backend/tests/fixtures/booking.fixtures.ts (2)
apps/backend/src/config/supabase.ts (1)
supabase(400-420)apps/backend/tests/utils/booking-test.utils.ts (1)
generateAuthToken(193-195)
apps/backend/src/routes/booking.routes.ts (2)
apps/backend/src/validators/booking.validator.ts (1)
validateConfirmPayment(6-28)apps/backend/src/controllers/booking.controller.ts (1)
confirmPayment(174-247)
apps/backend/src/controllers/booking.controller.ts (2)
apps/backend/src/types/booking.types.ts (1)
BookingParamsSchema(51-53)apps/backend/src/types/shared.types.ts (1)
formatErrorResponse(43-47)
apps/backend/tests/integration/booking-flow.int.test.ts (1)
apps/backend/src/index.ts (1)
app(40-40)
🔇 Additional comments (3)
task.md (1)
1-47: Plan looks solid and actionable.Clear scope, acceptance criteria, and references. No blockers from this doc.
apps/backend/src/controllers/booking.controller.ts (1)
176-183: Good switch to param-based bookingId validation.Validating
bookingIdfrom URL params via Zod is correct and matches the new PUT route.apps/backend/tests/fixtures/booking.fixtures.ts (1)
2-2: Using app supabase client in tests is fine with test-mode switch.This import will use the mock client under
NODE_ENV=test. Good for isolated tests.
| jest.mock('../../src/blockchain/trustlessWork', () => { | ||
| const mock = mockedTrustlessWork; | ||
|
|
||
| return { | ||
| // Class constructor returns the mocked instance | ||
| TrustlessWorkClient: jest.fn().mockImplementation(() => mock), | ||
|
|
||
| // Named exports proxy to the mocked instance methods if referenced directly | ||
| trustlessWorkClient: mock, | ||
| createEscrow: (...args: unknown[]) => mock.createEscrow(...args), | ||
| getEscrowStatus: (...args: unknown[]) => mock.getEscrowStatus(...args), | ||
| fundEscrow: (...args: unknown[]) => mock.fundEscrow(...args), | ||
| releaseEscrow: (...args: unknown[]) => mock.releaseEscrow(...args), | ||
| cancelEscrow: (...args: unknown[]) => mock.cancelEscrow(...args), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Ensure this mocks file loads before SUT and path specifiers match actual imports.
- This file must be imported before any module that imports
../../src/blockchain/trustlessWorkor../../src/blockchain/soroban. Prefer registering it in JestsetupFilesAfterEnvor top of each integration test. - Verify that your source actually imports those modules with the exact resolvable paths (after ts-jest/moduleNameMapper). If paths differ (e.g., alias imports), the mock won’t apply.
Run:
Also applies to: 42-52
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Jest config files:"
fd -a -t f -g 'jest*.config*' | sed 's/^/ - /'
echo -e "\nCheck for setupFiles / setupFilesAfterEnv:"
fd -a -t f -g 'jest*.config*' | xargs -I{} rg -nC2 -H 'setupFiles(AfterEnv)?' {}
echo -e "\nReferences to blockchain.mocks in tests or setup:"
rg -n -C2 'blockchain\.mocks' apps || true
echo -e "\nVerify import specifiers used by SUT for blockchain modules:"
rg -n -C2 -g '!**/tests/**' -P "from\s+['\"](\.{1,2}/)+blockchain/(trustlessWork|soroban)['\"]|require\(['\"](\.{1,2}/)+blockchain/(trustlessWork|soroban)['\"]\)" apps || true
echo -e "\nIf using path aliases, show moduleNameMapper:"
fd -a -t f -g 'jest*.config*' | xargs -I{} rg -n -H 'moduleNameMapper' {}Length of output: 1602
Load blockchain mocks before any SUT imports
- In apps/backend/tests/setup.ts (registered via setupFilesAfterEnv) add
import './mocks/blockchain.mocks'; - Verify your code imports
src/blockchain/trustlessWorkandsrc/blockchain/sorobanwith those exact paths—if you’re using TS path aliases, configuremoduleNameMapperin jest.config.js accordingly.
🤖 Prompt for AI Agents
In apps/backend/tests/mocks/blockchain.mocks.ts around lines 20 to 35: the test
mocks must be loaded before any system-under-test imports; open
apps/backend/tests/setup.ts and add an import that loads this file (i.e., import
'./mocks/blockchain.mocks';) so Jest registers the mock via setupFilesAfterEnv,
and if your codebase uses TypeScript path aliases ensure jest.config.js has
moduleNameMapper entries that map the TS paths to the real file paths so imports
of src/blockchain/trustlessWork and src/blockchain/soroban resolve to the mocked
files; also audit code to use those exact import paths (not alternate aliases)
so the mock is applied.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/backend/README.md (1)
155-157: Keep command usage consistent with Bun tooling.Every other test command in this README uses Bun (
bun test,bun run dev). Swapping this example tobun run test:intavoids mixing package managers in the docs and mirrors the real script invocation.-```bash -npm run test:int -``` +```bash +bun run test:int +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/backend/README.md(1 hunks)apps/backend/tests/integration/booking-flow.int.test.ts(1 hunks)apps/backend/tests/mocks/blockchain.mocks.ts(1 hunks)apps/backend/tests/setup.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/backend/tests/integration/booking-flow.int.test.ts (2)
apps/backend/src/middleware/rateLimiter.ts (1)
rateLimiter(3-12)apps/backend/src/middleware/error.middleware.ts (1)
errorMiddleware(10-41)
🔇 Additional comments (1)
apps/backend/tests/setup.ts (1)
6-8: Mocks now load ahead of the SUT—nice!Importing
./mocks/blockchain.mockshere guarantees the jest stubs register before any production modules, eliminating the race we hit earlier.
|
please could you provide a summary with media files? thks |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
PROJECT_SUMMARY.md (1)
192-200: Specify code fence language to satisfy markdownlint
markdownlintis flagging this fenced block because the language is missing (MD040). Adding a language identifier such astextwill keep the structure diagram readable while silencing the lint warning.-``` +```text stellar-rent/ ├── apps/ │ ├── web/ # Next.js frontend │ ├── backend/ # Node.js/Express API │ └── stellar-contracts/ # Soroban smart contracts ├── assets/ # Media files and branding └── docs/ # Documentation</blockquote></details> <details> <summary>MEDIA_ASSETS_INVENTORY.md (1)</summary><blockquote> `12-55`: **Insert blank lines around tables** Several tables sit directly beneath headings, which triggers markdownlint rule MD058. Inserting a blank line before and after each table (e.g., under “Primary Branding”, “UI Icons”, “Property Images”, and “UI Elements”) keeps the document compliant without changing the rendered output. ```diff ### Primary Branding (`/assets/`) + | File | Type | Dimensions | Purpose | Usage | ... | `flow-stellar-rent.png` | PNG | Variable | Architecture diagram | Technical documentation, README | + ### Web Application Assets (`/apps/web/public/`)Apply the same pattern to the subsequent tables in this section. As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MEDIA_ASSETS_INVENTORY.md(1 hunks)PROJECT_SUMMARY.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
MEDIA_ASSETS_INVENTORY.md
12-12: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
22-22: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
37-37: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
47-47: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
PROJECT_SUMMARY.md
8-8: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
192-192: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
|
Hello @respp
Includes all branding assets, UI icons, property images, and technical specifications. Both files are committed and ready for review. The project is ~85% complete with strong foundations - just need to finish the USDC payment integration for MVP. Thanks! |
I was actually referring to the media section in the issue summary, not in the repository itself. These files are unnecessary and create redundancy. |
any updates??? |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/public/stellarrentlogo.pngis excluded by!**/*.png
📒 Files selected for processing (5)
MEDIA_ASSETS_INVENTORY.md(1 hunks)PROJECT_SUMMARY.md(1 hunks)apps/web/src/app/page.tsx(1 hunks)apps/web/src/components/search/PropertyGrid.tsx(5 hunks)apps/web/src/lib/data/properties.ts(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
MEDIA_ASSETS_INVENTORY.md
8-8: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (1)
apps/web/src/app/page.tsx (1)
13-13: Logo asset verified.apps/web/public/stellarrentlogo.pngexists.
| **StellarRent** is a decentralized peer-to-peer (P2P) rental platform built on the Stellar blockchain, designed to revolutionize the rental market by offering instant USDC payments, ultra-low fees, and complete transparency. | ||
|
|
||
| ### 🎯 Mission Statement | ||
| *"Empowering Global Rentals - Everyone deserves access to a fair rental marketplace with minimal fees, instant payments, and complete transparency, powered by blockchain technology."* |
There was a problem hiding this comment.
Resolve markdownlint MD036 violation in mission statement
markdownlint flags Line 8 as “Emphasis used instead of a heading” (MD036). Dropping the italics (or switching to a blockquote) clears the warning and keeps CI green. Based on static analysis hints.
-*"Empowering Global Rentals - Everyone deserves access to a fair rental marketplace with minimal fees, instant payments, and complete transparency, powered by blockchain technology."*
+> "Empowering Global Rentals - Everyone deserves access to a fair rental marketplace with minimal fees, instant payments, and complete transparency, powered by blockchain technology."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| *"Empowering Global Rentals - Everyone deserves access to a fair rental marketplace with minimal fees, instant payments, and complete transparency, powered by blockchain technology."* | |
| > "Empowering Global Rentals - Everyone deserves access to a fair rental marketplace with minimal fees, instant payments, and complete transparency, powered by blockchain technology." |
🤖 Prompt for AI Agents
In PROJECT_SUMMARY.md around line 8, the mission statement is wrapped in
emphasis (italics) causing a markdownlint MD036 "Emphasis used instead of a
heading" warning; remove the surrounding asterisks (or convert the line to a
blockquote or proper heading) so the sentence is plain text or a heading,
preserving the content "Empowering Global Rentals - Everyone deserves access to
a fair rental marketplace with minimal fees, instant payments, and complete
transparency, powered by blockchain technology." and thereby clear the MD036
violation.
2 participants
Pull Request | StellarRent
📝 Summary### 🧪 Testing
Comprehensive testing was performed to ensure all scenarios work correctly:
Test Scenarios Verified:
Test Infrastructure:
✅ Testing Checklist
.env.testis properly configured in CI/CD🚀 Next Steps & Improvements
This change lays a solid foundation for further optimizations. Some areas that could benefit from future improvements include:
💬 Comments
This implementation successfully addresses the critical booking flow test coverage requirement. The tests are designed to be maintainable, comprehensive, and provide confidence in the booking and payment confirmation processes. The mocking strategy ensures tests run quickly and reliably without external dependencies.
Key Achievements:
The implementation is ready for production use and provides a solid foundation for future booking flow enhancements.
Provide a brief description of what this PR accomplishes.
🔗 Related Issues
Closes #(issue number) (Replace with the actual issue number).
🔄 Changes Made
Provide a general description of the changes. Include any relevant background information or context to help reviewers understand the purpose of this PR.
🖼️ Current Output
Provide visual evidence of the changes:
🧪 Testing
If applicable, describe the tests performed. Include screenshots, test outputs, or any resources that help reviewers understand how the changes were tested.
✅ Testing Checklist
List any possible issues that might arise with this change.
🚀 Next Steps & Improvements
This change lays a solid foundation for further optimizations. Some areas that could benefit from future improvements include:
💬 Comments
Any additional context, questions, or considerations for reviewers.
Summary by CodeRabbit
New Features
Documentation
Tests