-
Notifications
You must be signed in to change notification settings - Fork 0
New Flight Book in Create Order #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
nodes/Crossmint/actions/checkout/index.ts
, line 129-673 (link)style: Significant code duplication between flight fields for 'findProduct' platform and 'bookFlight' operation. Extract common flight field definitions into a shared constant to reduce maintenance burden.
Context Used: Rule from
dashboard
- Extract duplicated logic into reusable functions when the same calculation is used in multiple place... (source)
4 files reviewed, 7 comments
// If platform is flight, delegate to bookFlight operation | ||
if (platform === 'flight') { | ||
return await bookFlight(context, api, itemIndex); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The findProduct function now handles both product finding and flight booking, violating single responsibility principle. Consider renaming this function to something more generic like 'processOrder' or creating separate operations entirely.
const platform = context.getNodeParameter('platform', itemIndex) as string; | ||
|
||
// If platform is flight, delegate to bookFlight operation | ||
if (platform === 'flight') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: String literal comparison instead of using enum constant. Consider defining platform constants for consistency and type safety.
Context Used: Rule from dashboard
- Use enum constants (e.g., Chain.SOLANA, Chain.BASE) instead of string literals when comparing chain ... (source)
displayName: 'Passenger Email', | ||
name: 'passengerEmail', | ||
type: 'string', | ||
displayOptions: { show: { resource: ['checkout'], operation: ['bookFlight'] } }, | ||
default: '', | ||
placeholder: '[email protected]', | ||
description: 'Passenger email address', | ||
required: true, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Passenger email field only appears in the 'bookFlight' operation but not in the flight platform fields for 'findProduct'. This inconsistency could cause confusion if both operations are supposed to support the same flight booking functionality.
try { | ||
// Step 1: Search for flights using Worldstore Search API | ||
const flightIdsArray = flightIds.split(',').map(id => id.trim()); | ||
|
||
const searchBody = { | ||
uid: { | ||
originIATA: originIATA, | ||
destinationIATA: destinationIATA, | ||
cabinClass: cabinClass, | ||
passenger_number: passengerCount, | ||
departureFlightDetails: { | ||
departureDate, | ||
flightIds: flightIdsArray, | ||
}, | ||
}, | ||
}; | ||
|
||
const searchResponse = await api.post('ws/search', searchBody, 'unstable'); | ||
const { listings } = searchResponse; | ||
|
||
if (!listings || listings.length === 0) { | ||
throw new Error('No flights found for the specified criteria'); | ||
} | ||
|
||
// Use the first available listing | ||
const selectedListing = listings[0]; | ||
|
||
// Step 2: Create Worldstore order with passenger details | ||
const passengers = [{ | ||
title: passengerTitle, | ||
given_name: passengerFirstName, | ||
family_name: passengerLastName, | ||
born_on: passengerBirthDate, | ||
gender: passengerGender, | ||
email: passengerEmail, | ||
phone_number: passengerPhone, | ||
identity_documents: [{ | ||
type: 'passport', | ||
unique_identifier: passportNumber, | ||
issuing_country_code: passportCountry, | ||
expires_on: passportExpiry, | ||
}], | ||
}]; | ||
|
||
const wsOrderBody = { | ||
sellerId: '1', | ||
items: [{ | ||
listingId: selectedListing.id, | ||
listingParameters: { | ||
passengers, | ||
}, | ||
}], | ||
orderParameters: {}, | ||
}; | ||
|
||
const wsOrderResponse = await api.post('ws/orders', wsOrderBody, 'unstable'); | ||
const { order } = wsOrderResponse; | ||
|
||
// Step 3: Create Crossmint payment order | ||
const payment: any = { | ||
receiptEmail: passengerEmail, | ||
method: paymentMethod, | ||
currency: paymentCurrency, | ||
}; | ||
|
||
if (payerAddress) { | ||
payment.payerAddress = payerAddress; | ||
} | ||
|
||
const crossmintOrderBody = { | ||
recipient: { | ||
email: passengerEmail, | ||
}, | ||
locale: 'en-US', | ||
payment, | ||
externalOrder: order, | ||
}; | ||
|
||
const crossmintOrderResponse = await api.post('orders', crossmintOrderBody, '2022-06-09'); | ||
|
||
return { | ||
flightSearch: searchResponse, | ||
worldstoreOrder: wsOrderResponse, | ||
crossmintOrder: crossmintOrderResponse, | ||
selectedFlight: selectedListing, | ||
}; | ||
} catch (error: any) { | ||
throw new NodeApiError(context.getNode(), error); | ||
} | ||
} No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Function is 89 lines long and should be broken down into smaller helper functions for better maintainability
Context Used: Rule from dashboard
- When a function grows to 80+ lines, extract helper functions to improve readability and maintainabil... (source)
// Validation | ||
validateRequiredField(originIATA, 'Origin IATA', context, itemIndex); | ||
validateRequiredField(destinationIATA, 'Destination IATA', context, itemIndex); | ||
validateRequiredField(departureDate, 'Departure Date', context, itemIndex); | ||
validateRequiredField(passengerFirstName, 'Passenger First Name', context, itemIndex); | ||
validateRequiredField(passengerLastName, 'Passenger Last Name', context, itemIndex); | ||
validateEmail(passengerEmail, context, itemIndex); | ||
validateRequiredField(passportNumber, 'Passport Number', context, itemIndex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Missing validation for passport-related fields (passportCountry, passportExpiry) which are required for flight bookings
// Validation | |
validateRequiredField(originIATA, 'Origin IATA', context, itemIndex); | |
validateRequiredField(destinationIATA, 'Destination IATA', context, itemIndex); | |
validateRequiredField(departureDate, 'Departure Date', context, itemIndex); | |
validateRequiredField(passengerFirstName, 'Passenger First Name', context, itemIndex); | |
validateRequiredField(passengerLastName, 'Passenger Last Name', context, itemIndex); | |
validateEmail(passengerEmail, context, itemIndex); | |
validateRequiredField(passportNumber, 'Passport Number', context, itemIndex); | |
// Validation | |
validateRequiredField(originIATA, 'Origin IATA', context, itemIndex); | |
validateRequiredField(destinationIATA, 'Destination IATA', context, itemIndex); | |
validateRequiredField(departureDate, 'Departure Date', context, itemIndex); | |
validateRequiredField(passengerFirstName, 'Passenger First Name', context, itemIndex); | |
validateRequiredField(passengerLastName, 'Passenger Last Name', context, itemIndex); | |
validateEmail(passengerEmail, context, itemIndex); | |
validateRequiredField(passportNumber, 'Passport Number', context, itemIndex); | |
validateRequiredField(passportCountry, 'Passport Country', context, itemIndex); | |
validateRequiredField(passportExpiry, 'Passport Expiry', context, itemIndex); |
} catch (error: any) { | ||
throw new NodeApiError(context.getNode(), error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Missing error logging before re-throwing the error for debugging purposes
} catch (error: any) { | |
throw new NodeApiError(context.getNode(), error); | |
} catch (error: any) { | |
console.error('[bookFlight] Flight booking failed with error:', error); | |
throw new NodeApiError(context.getNode(), error); |
Context Used: Rule from dashboard
- Include console.error logging statements when providers fail, using the format `console.error('[Clas... (source)
New Flight Book in Create Order