fix: Ebarimt improve, putResponseDetail and put response with user #6084
Conversation
Reviewer's GuideThis PR enhances the Ebarimt integration by introducing a new temporary receipt generation query, propagating user context through receipt operations, extending GraphQL schema and UI to display the operation user, and adding port configuration support for Golomt payments. Class diagram for GraphQL PutResponse resolver extensionclassDiagram
class PutResponseResolver {
+user(putResponse: IEbarimtDocument): User
}
PutResponseResolver --> User
Class diagram for updated IContext and order mutation usageclassDiagram
class IContext {
+user: IUserDocument
+posUser?: IPosUserDocument
+config: IConfigDocument
+models: IModels
+subdomain: string
}
class OrderMutations {
+ordersMakePayment(..., context: IContext)
+ordersSettlePayment(..., context: IContext)
+ordersConvertToDeal(..., context: IContext)
}
OrderMutations --> IContext
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@munkhsaikhan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe changes introduce user tracking and association features across the ebarimt and POS plugins. User information is now captured and stored with ebarimt put response records, propagated through backend models, GraphQL schemas, and resolvers. The UI is updated to display user details in tables. Additionally, Golomt payment terminal configuration is enhanced to allow dynamic port number assignment from local storage. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant GQL as GraphQL API
participant Model as PutResponse Model
participant DB as Database
UI->>GQL: Query putResponseDetail(contentType, contentId, ...)
GQL->>Model: getPutResponse(contentType, contentId, ...)
Model->>DB: Find or create PutResponse (with userId)
Model-->>GQL: PutResponse (includes userId)
GQL-->>UI: PutResponse { userId, user { ... } }
UI->>UI: Display user info in table
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Hey @munkhsaikhan - I've reviewed your changes - here's some feedback:
- In the putResponseDetail resolver, handle non-‘deal’ contentType cases more explicitly (e.g. throw an error or return a default shape) instead of falling through to undefined to avoid unexpected null responses.
- Avoid mutating the module‐level
initialDataobject in the Golomt component—spread it into a new object before applyingportNoso you don’t introduce cross‐render side effects. - Make
contentTypeandcontentIdnon‐null (String!) in the GraphQL schema for putResponseDetail to enforce required inputs and prevent runtime null checks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the putResponseDetail resolver, handle non-‘deal’ contentType cases more explicitly (e.g. throw an error or return a default shape) instead of falling through to undefined to avoid unexpected null responses.
- Avoid mutating the module‐level `initialData` object in the Golomt component—spread it into a new object before applying `portNo` so you don’t introduce cross‐render side effects.
- Make `contentType` and `contentId` non‐null (`String!`) in the GraphQL schema for putResponseDetail to enforce required inputs and prevent runtime null checks.
## Individual Comments
### Comment 1
<location> `pos/app/(main)/cover/components/golomt.tsx:20` </location>
<code_context>
+ const terminalID = getLocal("golomtId");
+ const devicePortNo = getLocal("golomtPortNo");
+
+ if (devicePortNo) {
+ initialData.portNo = devicePortNo as string;
+ }
</code_context>
<issue_to_address>
Directly mutating imported initialData may cause side effects.
Instead, create a local copy of initialData and update portNo there to avoid unintended side effects.
</issue_to_address>
### Comment 2
<location> `packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts:5` </location>
<code_context>
+import { sendCoreMessage } from '../../messageBroker';
+import { IEbarimtDocument } from '../../models/definitions/ebarimt';
+
+const cars = {
+ async user(putResponse: IEbarimtDocument) {
+ if (!putResponse.userId) {
+ return;
+ }
+
+ return {
+ __typename: 'User',
+ _id: putResponse.userId
+ }
+ },
+};
+
+export default cars;
</code_context>
<issue_to_address>
The resolver object is named 'cars', which is misleading.
Consider renaming the resolver object to something like 'PutResponseResolvers' for better clarity and maintainability.
</issue_to_address>
### Comment 3
<location> `packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts:233` </location>
<code_context>
+ putResponseDetail: async (
</code_context>
<issue_to_address>
putResponseDetail returns undefined if contentType is not 'deal', which may be ambiguous.
Consider returning a clear error or a consistent null value instead of undefined to avoid ambiguity for clients.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (devicePortNo) { | ||
| initialData.portNo = devicePortNo as string; |
There was a problem hiding this comment.
issue (bug_risk): Directly mutating imported initialData may cause side effects.
Instead, create a local copy of initialData and update portNo there to avoid unintended side effects.
| const cars = { | ||
| async user(putResponse: IEbarimtDocument) { | ||
| if (!putResponse.userId) { | ||
| return; | ||
| } | ||
|
|
||
| return { | ||
| __typename: 'User', | ||
| _id: putResponse.userId | ||
| } |
There was a problem hiding this comment.
suggestion: The resolver object is named 'cars', which is misleading.
Consider renaming the resolver object to something like 'PutResponseResolvers' for better clarity and maintainability.
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 8f37502 in 2 minutes and 16 seconds. Click for details.
- Reviewed
832lines of code in22files - Skipped
0files when reviewing. - Skipped posting
16draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. pos/modules/checkout/hooks/useGolomt.tsx:47
- Draft comment:
Validate and possibly sanitize 'devicePortNo' before assigning it to the 'sendData' object to ensure correct format. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The value comes from local storage via getLocal(). While validation could be good practice, there's no clear evidence that whitespace or format issues are actually a problem here. The code already has a default "9" in initialData, and this is just an override. The comment is speculative without showing why validation is needed. Perhaps port numbers do need validation to ensure they're in the correct format for the Golomt payment system. Maybe there have been issues with malformed port numbers in the past. Without evidence of actual problems or specific format requirements, this is a speculative suggestion. If port validation was critical, it should be handled where the value is first stored, not here. Delete the comment as it makes a speculative suggestion without clear evidence that validation is needed here.
2. pos/modules/checkout/hooks/useGolomt.tsx:10
- Draft comment:
Consider checking the spelling/consistency of 'bandWidth'. Typically 'bandwidth' is preferred. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is likely an API field name for the Golomt payment terminal integration. Changing API field names could break the integration. The fact that it was explicitly changed from 'bandwidth' to 'bandWidth' suggests this may have been done to match the API requirements. Without knowing the API specs, suggesting to change it back could be harmful. The comment assumes this is just about code style, but it could be an intentional change to match external API requirements. We don't have the API documentation to verify. Given this is likely an integration with an external payment system, we should be very cautious about suggesting changes to field names without documentation. Delete this comment as it could be suggesting a harmful change to an API field name without sufficient context to know if it's correct.
3. pos/modules/settings/components/GolomtConfig.tsx:21
- Draft comment:
Optionally add client-side validation for terminal ID and port number inputs to improve robustness. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. packages/plugin-posclient-api/src/utils.ts:238
- Draft comment:
When config is not found in 'updateMobileAmount', consider throwing an error instead of only logging via debugError to avoid silent failures. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. packages/plugin-posclient-api/src/models/PutResponses.ts:117
- Draft comment:
Consider refactoring the promise chain (using .then/.catch) for the fetch call into async/await for improved readability and error handling. - Reason this comment was not posted:
Comment was on unchanged code.
6. packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts:244
- Draft comment:
Typo/grammar issue: The error message 'has not ebarimt' appears to be grammatically incorrect. Consider revising it to something like 'Ebarimt not available' or 'No ebarimt found'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the suggested message is more grammatically correct, error messages are primarily for developers and the current message, though not perfect English, clearly conveys its meaning. The change is not critical for functionality. This seems like a minor stylistic suggestion rather than an important code quality issue. The error message could be confusing to non-native English speakers. Clear error messages help with debugging and maintenance. However, the current message is understandable despite its grammar, and error message wording is not a critical issue that requires a code change. This falls into the category of "obvious" or "unimportant" comments that we should avoid. Delete this comment as it suggests a minor stylistic change to an error message that, while not perfect, is still functional and understandable.
7. packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts:265
- Draft comment:
Typo/grammar issue: The error message 'has not found ebarimt config' is grammatically off. It might be clearer as 'Ebarimt config not found' or similar. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the suggested message is indeed more grammatically correct, error messages are primarily for developers and logging. The current message, while not perfect English, still clearly conveys the meaning. This seems like a minor stylistic change that doesn't impact functionality or code quality significantly. Per the rules, we should not make purely informative comments or obvious suggestions. The error message could potentially appear in logs or user-facing error handling, where proper grammar might matter for professionalism. The current message could be confusing to non-native English speakers. While proper grammar is good, this is an internal error message in a technical context. The meaning is clear enough for debugging purposes. This kind of minor stylistic feedback creates noise in PR reviews. The comment should be removed as it suggests a minor grammatical improvement that doesn't materially impact code quality or functionality.
8. packages/plugin-ebarimt-api/src/models/Ebarimt.ts:50
- Draft comment:
Typo: The comment on line 50 says "check previously post". Consider revising it to a more grammatically correct phrase, e.g., "check previously posted responses". - Reason this comment was not posted:
Comment was on unchanged code.
9. packages/plugin-ebarimt-api/src/models/Ebarimt.ts:182
- Draft comment:
It looks like there is a duplicate 'await' in this line. Consider removing one of them: 'const delResponse = await returnResponse(url, data)'. - Reason this comment was not posted:
Comment was on unchanged code.
10. packages/plugin-pos-api/src/graphql/resolvers/mutations/orders.ts:17
- Draft comment:
Typo in the error message: 'not found order' seems awkward. Consider using a phrase like 'Order not found' to improve readability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. packages/plugin-posclient-api/src/graphql/resolvers/mutations/covers.ts:8
- Draft comment:
Typographical note: The error message 'Must login' may be improved. Consider rephrasing it to something like 'User must be logged in' for better clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the suggested message is slightly more grammatically correct and clearer, this is an extremely minor change that doesn't affect functionality. Error messages are shown to end users, but this seems like a backend API where the error would be caught and handled by the frontend. The current message is not incorrect or confusing enough to warrant a comment. The error message could be user-facing and clarity in error messages is generally good practice. The change would improve the code quality slightly. However, this is a very minor stylistic suggestion that doesn't meaningfully impact code quality or functionality. We should focus on more substantial issues. Delete this comment as it suggests a trivial wording change that doesn't meaningfully improve the code.
12. packages/plugin-posclient-api/src/graphql/resolvers/mutations/covers.ts:54
- Draft comment:
Typographical suggestion: Change 'Must login' to 'Must log in' to ensure correct grammar. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While technically correct grammar, this is a very minor change to an error message that users will rarely see. The current message is still completely understandable. The comment doesn't point out any actual bugs or code quality issues. According to the rules, we should not make purely informative comments or unimportant suggestions. The grammar correction could improve code professionalism and consistency. Error messages are part of the user experience. While true, this is still an extremely minor change that doesn't impact functionality or maintainability. The current message is clear enough. Delete this comment as it suggests an unimportant change that doesn't meaningfully improve the code.
13. packages/plugin-posclient-api/src/graphql/resolvers/mutations/covers.ts:60
- Draft comment:
Consider capitalizing the error message: 'cannot update (confirmed)' could be updated to 'Cannot update (confirmed)' for consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
14. packages/plugin-posclient-api/src/graphql/resolvers/mutations/covers.ts:79
- Draft comment:
Typographical suggestion: Consider rephrasing the error message from 'Must login' to 'Must be logged in' for better grammar and clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While technically correct grammar, this is an error message that will likely only be seen by developers in logs or API responses. The current message is clear enough to convey the meaning. The change would be purely cosmetic and doesn't affect functionality. The comment doesn't point out a real issue that needs fixing. The improved grammar could make the API more professional and consistent with common conventions. Error messages are part of the API contract. While better grammar would be nice, this is an internal error message and the current form is clear enough. This kind of minor wording suggestion creates noise in PR reviews. Delete this comment as it suggests a purely cosmetic change that doesn't meaningfully improve the code.
15. packages/plugin-posclient-api/src/models/definitions/putResponses.ts:157
- Draft comment:
Typo: The label 'Created user' might be intended to be 'Created by' or simply 'Creator'. Please check if this is what you meant. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a purely cosmetic suggestion about label text. Labels are typically used for UI display or documentation, but there's no strong technical reason why 'Created user' is incorrect. The current label is clear enough in meaning. This seems like a matter of personal preference rather than a technical issue that needs fixing. The label 'Created by' could be marginally more grammatically correct in English. Other labels in the file like 'Created at' follow proper English phrasing. While 'Created by' might be slightly more elegant, 'Created user' is perfectly understandable and the difference is purely stylistic. This doesn't meet the bar of being a clearly required code change. This comment should be deleted as it suggests a purely cosmetic change that isn't clearly necessary. The existing label is functional and clear enough.
16. pos/modules/checkout/hooks/useGolomt.tsx:10
- Draft comment:
It looks like the property name changed from "bandwidth" to "bandWidth". Please check if this change was intentional, as it might be a typographical or lexicographical error. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_yiESLfsjcNTTJweK
You can customize 
| const devicePortNo = getLocal("golomtPortNo"); | ||
|
|
||
| if (devicePortNo) { | ||
| initialData.portNo = devicePortNo as string; |
There was a problem hiding this comment.
Avoid mutating the imported 'initialData' object directly; use a local clone to prevent side effects across components.
| stageId = stageId || deal.stageId; | ||
|
|
||
| if (!deal?._id || !stageId) { | ||
| throw new Error('has not found deal') |
There was a problem hiding this comment.
Typo/grammar issue: The error message 'has not found deal' is awkward. Consider updating it to 'Deal not found' or a similar phrasing.
| throw new Error('has not found deal') | |
| throw new Error('Deal not found') |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 7cdc37b in 1 minute and 19 seconds. Click for details.
- Reviewed
19lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts:1
- Draft comment:
Remove unused imports (IContext and sendCoreMessage) since they aren’t used in this module. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts:6
- Draft comment:
Consider renaming the resolver’s parameter (currently 'putResponse') to avoid shadowing the outer 'putResponse' object. Also, since there’s no asynchronous operation inside, verify if 'async' is needed. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_1yAHn1IzRkpskvBe
You can customize
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
pos/modules/settings/components/GolomtConfig.tsx (1)
39-44: Consider adding validation for port number input.The port number input field implementation looks good, but consider adding validation to ensure the port number is within valid range (1-65535).
<Input id="golomtPortNo" placeholder="Терминалийн dev-PORT-г оруулана уу" + type="number" + min="1" + max="65535" value={portNo} => setPortNo(e.target.value)} />packages/plugin-ebarimt-ui/src/components/PutResponses.tsx (1)
92-92: Use internationalization for consistency.The "User" header should use the
__()function for localization consistency with other table headers.- <th>User</th> + <th>{__('User')}</th>packages/plugin-ebarimt-ui/src/components/PutResponseRow.tsx (1)
63-63: Consider adding a fallback display for better UX.The optional chaining safely handles missing user data, but an empty cell might not provide the best user experience. Consider adding a fallback display.
- <td key={'user'}>{putResponse.user?.email} </td> + <td key={'user'}>{putResponse.user?.email || '-'} </td>packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts (2)
5-16: Rename the resolver variable for clarity.The resolver logic is correct, but the variable name
carsis confusing and doesn't reflect its purpose.-const cars = { +const PutResponseResolvers = { async user(putResponse: IEbarimtDocument) { if (!putResponse.userId) { return; } return { __typename: 'User', _id: putResponse.userId } }, };
18-18: Update the export to match the renamed variable.-export default cars; +export default PutResponseResolvers;packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts (1)
233-316: Improve error messages and consider refactoringThe error messages are too generic and the function is quite complex. Consider:
- Making error messages more descriptive for better debugging
- Extracting the deal-specific logic into a separate function
- throw new Error('has not ebarimt') + throw new Error('No ebarimt record found for the specified content') - throw new Error('has not found deal') + throw new Error(`Deal not found with ID: ${contentId}`) - throw new Error('has not found ebarimt config') + throw new Error(`No ebarimt configuration found for stage: ${stageId}`)Consider extracting the deal handling logic (lines 247-314) into a separate function like
generateTempDealEbarimtfor better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
packages/plugin-ebarimt-api/src/afterMutations.ts(2 hunks)packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts(1 hunks)packages/plugin-ebarimt-api/src/graphql/resolvers/index.ts(1 hunks)packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts(5 hunks)packages/plugin-ebarimt-api/src/graphql/schema/ebarimt.ts(3 hunks)packages/plugin-ebarimt-api/src/messageBroker.ts(2 hunks)packages/plugin-ebarimt-api/src/models/Ebarimt.ts(5 hunks)packages/plugin-ebarimt-api/src/models/definitions/ebarimt.ts(2 hunks)packages/plugin-ebarimt-ui/src/components/PutResponseRow.tsx(1 hunks)packages/plugin-ebarimt-ui/src/components/PutResponses.tsx(1 hunks)packages/plugin-ebarimt-ui/src/graphql/queries.ts(1 hunks)packages/plugin-ebarimt-ui/src/types.ts(2 hunks)packages/plugin-pos-api/src/graphql/resolvers/mutations/orders.ts(3 hunks)packages/plugin-posclient-api/src/graphql/resolvers/mutations/covers.ts(4 hunks)packages/plugin-posclient-api/src/graphql/resolvers/mutations/orders.ts(9 hunks)packages/plugin-posclient-api/src/graphql/types.ts(1 hunks)packages/plugin-posclient-api/src/models/PutResponses.ts(5 hunks)packages/plugin-posclient-api/src/models/definitions/putResponses.ts(2 hunks)packages/plugin-posclient-api/src/utils.ts(4 hunks)pos/app/(main)/cover/components/golomt.tsx(1 hunks)pos/modules/checkout/hooks/useGolomt.tsx(4 hunks)pos/modules/settings/components/GolomtConfig.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (11)
packages/plugin-ebarimt-api/src/messageBroker.ts (1)
packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts (1)
user(6-15)
pos/app/(main)/cover/components/golomt.tsx (2)
pos/lib/utils.ts (1)
getLocal(60-72)pos/modules/checkout/hooks/useGolomt.tsx (1)
initialData(7-20)
packages/plugin-ebarimt-api/src/afterMutations.ts (1)
packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts (1)
user(6-15)
pos/modules/checkout/hooks/useGolomt.tsx (1)
pos/lib/utils.ts (1)
getLocal(60-72)
packages/plugin-ebarimt-ui/src/types.ts (1)
client-portal/modules/types.ts (1)
IUser(237-249)
packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts (1)
packages/plugin-ebarimt-api/src/models/definitions/ebarimt.ts (1)
IEbarimtDocument(97-100)
packages/plugin-posclient-api/src/utils.ts (2)
packages/plugin-posclient-api/src/graphql/resolvers/mutations/orders.ts (1)
ISettlePaymentParams(46-48)packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts (1)
user(6-15)
packages/plugin-posclient-api/src/models/PutResponses.ts (4)
packages/plugin-ebarimt-api/src/models/Ebarimt.ts (1)
IPutResponseModel(15-45)packages/plugin-posclient-api/src/models/definitions/putResponses.ts (2)
IEbarimtDocument(95-98)IEbarimtFull(81-93)packages/plugin-posclient-api/src/models/definitions/configs.ts (1)
IEbarimtConfig(4-24)packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts (1)
user(6-15)
packages/plugin-pos-api/src/graphql/resolvers/mutations/orders.ts (1)
packages/plugin-posclient-api/src/graphql/types.ts (1)
IContext(8-16)
packages/plugin-ebarimt-api/src/models/Ebarimt.ts (4)
packages/plugin-ebarimt-api/src/models/definitions/ebarimt.ts (2)
IEbarimtDocument(97-100)IEbarimtFull(82-95)packages/plugin-ebarimt-api/src/models/utils.ts (1)
IDoc(7-49)packages/plugin-ebarimt-api/src/models/definitions/configs.ts (1)
IEbarimtConfig(2-22)packages/plugin-ebarimt-api/src/graphql/resolvers/PutResponse.ts (1)
user(6-15)
packages/plugin-posclient-api/src/graphql/resolvers/mutations/orders.ts (2)
packages/plugin-posclient-api/src/graphql/types.ts (1)
IContext(8-16)packages/plugin-posclient-api/src/graphql/utils/orderUtils.ts (1)
checkCouponCode(772-798)
🪛 GitHub Actions: Plugin posclient Api CI
packages/plugin-posclient-api/src/utils.ts
[error] 1-1: TypeScript error TS2307: Cannot find module 'dayjs' or its corresponding type declarations.
🪛 GitHub Actions: Plugin ebarimt Api CI
packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts
[error] 2-2: TypeScript error TS2307: Cannot find module 'dayjs' or its corresponding type declarations.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pos-ui
- GitHub Check: ui
- GitHub Check: runtest
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (32)
pos/modules/settings/components/GolomtConfig.tsx (2)
15-15: LGTM - Port number state initialization.The port number state initialization from local storage is implemented correctly.
24-24: LGTM - Port number persistence.The port number is properly persisted to local storage on form submission.
pos/modules/checkout/hooks/useGolomt.tsx (4)
10-10: Good typo fix.Correcting "bandwidth" to "bandWidth" improves consistency.
19-19: LGTM - Added portNo property to initialData.Adding the
portNoproperty with a default value of"9"is a reasonable addition to the configuration object.
31-31: LGTM - Local storage retrieval for port number.The port number retrieval from local storage is implemented consistently with other files.
48-50: LGTM - Conditional port number override.The conditional override of
sendData.portNois clean and follows good practices by creating a local copy before modification.packages/plugin-posclient-api/src/graphql/types.ts (1)
12-12: posUser optional: verified safeI’ve searched every
posUseraccess and confirmed that all direct property uses (posUser._id,posUser.loginToken, etc.) are properly guarded by null checks or optional chaining (e.g.if (!posUser) throw…,posUser?._id,posUser ? posUser._id : ""). ThecheckDirectDiscountutil also short-circuits on a missingposUser. No unguarded accesses remain.No further changes are required.
packages/plugin-ebarimt-api/src/graphql/resolvers/index.ts (1)
3-3: LGTM! Proper resolver integration.The PutResponse resolver is correctly imported and exposed in the resolvers object, following the established pattern. This properly enables user field resolution for put responses.
Also applies to: 6-6
packages/plugin-ebarimt-ui/src/types.ts (1)
3-3: LGTM! Type definitions properly extended.The type definitions are correctly extended to include user information, properly importing the IUser type and adding the optional userId and user fields to the IPutResponse interface. This aligns well with the backend schema changes.
Also applies to: 55-56
packages/plugin-posclient-api/src/graphql/resolvers/mutations/covers.ts (1)
7-9: LGTM: Consistent authentication enforcement across all cover mutations.The authentication checks are properly implemented across all mutation resolvers. The consistent error message "Must login" and uniform validation pattern enhance security by ensuring user authentication is required for all cover operations.
Also applies to: 23-25, 53-55, 78-80
packages/plugin-posclient-api/src/models/definitions/putResponses.ts (1)
78-78: LGTM: User tracking implementation is consistent and well-structured.The addition of the optional
userIdfield to both the interface and schema is properly implemented. This enables tracking which user created the put response record, aligning with the broader user context integration across the codebase.Also applies to: 157-157
packages/plugin-ebarimt-api/src/messageBroker.ts (1)
70-70: LGTM: User context integration in message brokerThe
userparameter is correctly destructured on line 70 and passed through toPutResponses.returnBillon lines 81–82.• File: packages/plugin-ebarimt-api/src/messageBroker.ts
• Lines: 70, 81–82Please manually verify that the
returnBillmethod signature in your upstream/external dependency (wherePutResponsesis defined) has been updated to accept the optionaluserparameter.packages/plugin-ebarimt-api/src/models/definitions/ebarimt.ts (2)
76-76: Good fix: Added missing semicolon.The semicolon after the
sendInfoproperty improves code consistency.
79-79: LGTM: User tracking field implementation is consistent.The
userIdfield is properly added to both the interface and schema with appropriate optional configuration. This maintains consistency with the corresponding changes in the posclient-api package and supports the broader user context integration.Also applies to: 160-160
packages/plugin-ebarimt-api/src/afterMutations.ts (2)
51-53: LGTM: User context propagation to returnBill method.The user parameter is properly passed to the
returnBillmethod, ensuring user context is tracked for return bill operations.
133-135: LGTM: User context propagation to putData method.The user parameter is properly passed to the
putDatamethod, maintaining consistency with the user tracking pattern throughout the ebarimt processing flow.packages/plugin-ebarimt-ui/src/graphql/queries.ts (1)
86-96: LGTM! User fields properly added to GraphQL query.The addition of
userIdand nesteduserobject fields to the response fields is well-structured and aligns with the PR objective to track user information with put responses.packages/plugin-pos-api/src/graphql/resolvers/mutations/orders.ts (2)
13-13: LGTM! User context properly extracted from GraphQL context.The addition of
userto the destructured context parameters enables user tracking for the return bill operation.
37-37: LGTM! User context correctly propagated to ebarimt message.The user object is properly passed to the ebarimt message system for downstream processing.
packages/plugin-ebarimt-api/src/graphql/schema/ebarimt.ts (3)
2-4: LGTM! GraphQL federation directives properly configured.The User type extension with federation directives is correctly implemented for cross-service user data resolution.
46-47: LGTM! User fields appropriately added to PutResponse type.The addition of
userIdanduserfields enables proper user association with put response records.
92-92: LGTM! New query added for detailed put response data.The
putResponseDetailquery provides a useful way to fetch detailed put response information including user context.packages/plugin-posclient-api/src/utils.ts (3)
361-361: LGTM! User parameter appropriately added to function signature.The optional
userparameter enables proper user context tracking in the payment settlement process.
403-403: LGTM! User context correctly passed to putData method.The user parameter is properly propagated to the PutResponses.putData call for user association.
410-417: LGTM! Enhanced error response with comprehensive metadata.The error response object now includes detailed metadata including
userId, formatted timestamp, and other relevant fields for better error tracking and debugging.packages/plugin-posclient-api/src/models/PutResponses.ts (2)
15-20: Missing documentation for posToken parameter additionThe addition of the
posTokenparameter to theputDatamethod is not mentioned in the AI summary, which only documents the user parameter changes. This appears to be a significant API change that should be documented.Also applies to: 66-66
Likely an incorrect or invalid review comment.
21-25: LGTM! User tracking properly implementedThe optional user parameter is correctly added to the
returnBillmethod with safe optional chaining for the userId assignment.Also applies to: 170-170, 204-204
packages/plugin-ebarimt-api/src/models/Ebarimt.ts (1)
13-13: LGTM! User tracking consistently implementedThe optional user parameter is properly added to both
putDataandreturnBillmethods with safe userId assignment using optional chaining.Also applies to: 18-19, 23-24, 49-49, 106-106, 145-145, 179-179
packages/plugin-posclient-api/src/graphql/resolvers/mutations/orders.ts (4)
634-634: LGTM! User context properly passed to putDataThe
posUseris correctly extracted from context and passed to theputDatamethod.Also applies to: 667-668
1121-1121: LGTM! Proper null check and user trackingGood use of optional chaining for the
posUsernull check, and the user context is correctly passed toreturnBill.Also applies to: 1193-1194
903-904: LGTM! Conditional user assignment handled correctlyThe
assignedUserIdsandwatchedUserIdsare properly set only whenposUseris available.
756-756: checkCouponCode signature verifiedThe function
checkCouponCodeis declared as:
packages/plugin-posclient-api/src/graphql/utils/orderUtils.ts:772export const checkCouponCode = async ({ order, subdomain }: { order: IOrderDocument; subdomain: string }) => { … }Passing
{ subdomain, order }matches this signature. No changes required.
| const devicePortNo = getLocal("golomtPortNo"); | ||
|
|
||
| if (devicePortNo) { | ||
| initialData.portNo = devicePortNo as string; | ||
| } |
There was a problem hiding this comment.
Avoid mutating shared imported objects.
Mutating the imported initialData object creates a side effect that could affect other components using the same object. This violates the principle of immutability and can lead to unexpected behavior.
Consider creating a local copy or passing the port number as a parameter instead:
- const devicePortNo = getLocal("golomtPortNo");
-
- if (devicePortNo) {
- initialData.portNo = devicePortNo as string;
- }
+ const devicePortNo = getLocal("golomtPortNo");
+ const localInitialData = {
+ ...initialData,
+ ...(devicePortNo && { portNo: devicePortNo as string })
+ };Then use localInitialData instead of initialData in the handleFetchCover function.
📝 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.
| const devicePortNo = getLocal("golomtPortNo"); | |
| if (devicePortNo) { | |
| initialData.portNo = devicePortNo as string; | |
| } | |
| const devicePortNo = getLocal("golomtPortNo"); | |
| const localInitialData = { | |
| ...initialData, | |
| ...(devicePortNo && { portNo: devicePortNo as string }) | |
| }; |
🤖 Prompt for AI Agents
In pos/app/(main)/cover/components/golomt.tsx around lines 18 to 22, avoid
mutating the imported initialData object directly as it causes side effects.
Instead, create a local copy of initialData (e.g., localInitialData) and assign
the portNo to this copy. Then use this local copy in the handleFetchCover
function to maintain immutability and prevent unexpected behavior in other
components.
packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts
Outdated
Show resolved
Hide resolved
| @@ -292,7 +292,7 @@ const ordersEdit = async ( | |||
| models, | |||
| subdomain | |||
| }: { | |||
| posUser: IPosUserDocument; | |||
| posUser?: IPosUserDocument; | |||
There was a problem hiding this comment.
Add null checks for optional posUser usage
The posUser parameter is now optional, but it's still used without null checks in several places (lines 193, 203, 314). This could cause runtime errors if posUser is undefined.
Consider using optional chaining or conditional checks:
- userId: posUser._id : "",
+ userId: posUser?._id || "",Also applies to: 193-193, 203-203, 314-314
🤖 Prompt for AI Agents
In packages/plugin-posclient-api/src/graphql/resolvers/mutations/orders.ts at
lines 193, 203, and 314, the optional parameter posUser is used without null
checks, risking runtime errors if posUser is undefined. Add null checks before
accessing posUser properties by using optional chaining (e.g.,
posUser?.property) or explicit conditional checks (e.g., if (posUser) { ... })
to safely handle cases when posUser is undefined.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed c44fb5b in 48 seconds. Click for details.
- Reviewed
20lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts:258
- Draft comment:
Improved error message for missing deal. Consider adding context (e.g. deal ID) if useful. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts:264
- Draft comment:
Clarified error message for missing Ebarimt config. This is more explicit. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_iPlDKFSMJFPjIzjq
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed e581972 in 1 minute and 6 seconds. Click for details.
- Reviewed
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts:312
- Draft comment:
Removal of the premature 'return;' allows the temporary response logic (especially for deals) to execute. Confirm that non-'deal' content types are handled properly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to confirm that non-'deal' content types are handled properly. This falls under asking the author to confirm their intention or to ensure behavior is intended, which is against the rules.
Workflow ID: wflow_O9dIoBSx1jjuLSJP
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 0f85b3b in 2 minutes and 5 seconds. Click for details.
- Reviewed
47lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts:301
- Draft comment:
Typographical issue: the date format string contains an extra quote and uses lowercase letters for the year. The format string is written as"yyyy-MM-dd HH:mm:ssbut likely should beYYYY-MM-DD HH:mm:ss(without the extra quotation mark) to match expected behavior. - Reason this comment was not posted:
Marked as duplicate.
2. packages/plugin-ebarimt-api/src/graphql/resolvers/queries/ebarimt.ts:310
- Draft comment:
The format string seems to have an extra quote. It currently reads"yyyy-MM-dd HH:mm:sswhich might be unintended. Consider changing it to either'yyyy-MM-dd HH:mm:ss'or"yyyy-MM-dd HH:mm:ss". - Reason this comment was not posted:
Marked as duplicate.
3. packages/plugin-posclient-api/src/utils.ts:411
- Draft comment:
It looks like there's an extra double-quote at the beginning of the date format string. Instead ofmoment(new Date()).format('"yyyy-MM-dd HH:mm:ss'), it appears you intended to use something likemoment(new Date()).format('YYYY-MM-DD HH:mm:ss'). - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_YJhyaD9AE3E3ZqGB
You can customize
| ...data, | ||
| id: "Түр баримт", | ||
| status: "SUCCESS", | ||
| date: moment(new Date()).format('"yyyy-MM-dd HH:mm:ss'), |
There was a problem hiding this comment.
The moment format string is incorrect—it includes an extraneous quote and uses lowercase 'yyyy'. This will output a literal quote and 'yyyy' instead of the year. Use "YYYY-MM-DD HH:mm:ss" (and consider using moment() without new Date()).
| date: moment(new Date()).format('"yyyy-MM-dd HH:mm:ss'), | |
| date: moment(new Date()).format('YYYY-MM-DD HH:mm:ss'), |
| ); | ||
| putData && ebarimtResponses.push(putData); | ||
| innerData && ebarimtResponses.push(innerData); | ||
| } catch (e) { | ||
| ebarimtResponses.push({ | ||
| _id: `Err${Math.random()}`, | ||
| id: 'Error', | ||
| type: ebarimtData.type, | ||
| date: moment(new Date()).format('"yyyy-MM-dd HH:mm:ss'), |
There was a problem hiding this comment.
The date formatting using moment is misconfigured—an extra quote and lowercase tokens result in incorrect output. Replace format('"yyyy-MM-dd HH:mm:ss') with format('YYYY-MM-DD HH:mm:ss').
| date: moment(new Date()).format('"yyyy-MM-dd HH:mm:ss'), | |
| date: moment(new Date()).format('YYYY-MM-DD HH:mm:ss'), |
|
Summary by Sourcery
Enable user-aware Ebarimt integration by passing user context into putData and returnBill operations, extend GraphQL schema and UI to surface response authors, introduce putResponseDetail query for temporary response generation, and add support for Golomt terminal port number configuration.
New Features:
Enhancements:
Chores:
Important
Enhance Ebarimt integration with user context propagation, new GraphQL queries, UI updates, and Golomt terminal configuration support.
putResponseDetailGraphQL query inebarimt.tsto fetch or generate temporary Ebarimt responses.golomt.tsxanduseGolomt.tsx.putDataandreturnBilloperations inEbarimt.tsandPutResponses.ts.PutResponsemodel and schema to includeuserIdanduserfields inebarimt.tsandputResponses.ts.PutResponseRow.tsxandPutResponses.tsx.covers.tsmutations.posUserthrough order and payment mutations inorders.ts.This description was created by
for 0f85b3b. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
User Interface