sync from polaris datas to erxes #5041
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Type: Enhancement
PR Summary: This pull request introduces several enhancements aimed at synchronizing data from Polaris to Erxes. It adds new utility functions for fetching and updating customer, loan, and saving contract data. Additionally, it extends the UI components to include new functionalities such as checking and syncing customers, loans, and savings accounts directly from the UI. The changes span across multiple files, including utility functions for API calls, GraphQL schema updates for new mutations, and UI updates for handling the new functionalities.
Decision: Comment
📝 Type: 'Enhancement' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
- Files deleted: Sourcery does not currently approve diffs with deleted files.
- Big diff: the diff is too large to approve with confidence.
General suggestions:
- Consider abstracting common logic in utility functions to reduce code duplication and improve maintainability.
- Ensure that newly introduced state management and asynchronous operations in UI components are well-documented and tested to maintain code quality.
- Review the necessity of direct state updates from props in UI components to avoid potential redundancy and ensure optimal performance.
- Evaluate the complexity introduced by new GraphQL mutations and consider abstracting similar functionalities to simplify the schema and improve maintainability.
- Given the significant changes, especially in data processing and UI interactions, ensure comprehensive testing, including unit, integration, and end-to-end tests, to maintain reliability and user experience.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| const onClickCheck = (e) => { | ||
| toCheckCustomers(); | ||
| this.setState({ items: items }); | ||
| }; |
There was a problem hiding this comment.
question (llm): The onClickCheck function directly modifies the component's state using this.setState({ items: items }), which might not be necessary since items is already part of the component's props. Ensure that this state update is required for the component's functionality, or consider using props directly if the state update is redundant.
| const responseData = await getSavingDetail(subdomain, { | ||
| number: preSavingContract.number, | ||
| }); |
There was a problem hiding this comment.
suggestion (llm): Consider handling the scenario where getSavingDetail might return an error or null response. This could happen due to network issues or if the saving detail for the provided number does not exist. Currently, the code assumes a successful response with data.
| @@ -30,6 +30,18 @@ export const initBroker = async () => { | |||
| }, | |||
| ); | |||
|
|
|||
| consumeRPCQueue( | |||
There was a problem hiding this comment.
issue (llm): The addition of separate queue consumers for specific operations (loans:contracts.updateOne and loans:contractType.find) introduces a higher level of complexity and redundancy, particularly with the inclusion of verbose logging that might not be necessary (console.log('selector:', selector, modifier);). This approach increases the cognitive load for developers who need to understand and maintain multiple consumers, potentially leading to confusion and maintenance challenges.
A more streamlined approach could involve consolidating these operations into a more generic queue consumer that handles various types of updates. This would not only reduce the number of consumers but also make the system more flexible and easier to maintain. For example, a single consumer that takes in a query and an update object could handle different update operations without the need for separate consumers for each specific case.
Here's a suggested refactor that encapsulates the update functionality in a more generic manner:
consumeRPCQueue('loans:contract.update', async ({ subdomain, data }) => {
const models = await generateModels(subdomain);
const { query, update } = data; // Assuming data contains query and update objects
// Perform the update operation based on the provided query and update data
const result = await models.Contracts.updateOne(query, update);
return {
status: 'success',
data: result,
};
});This approach minimizes the complexity and redundancy in the codebase, making it more maintainable and easier to extend in the future.
| @@ -60,6 +60,17 @@ export const initBroker = async () => { | |||
| }, | |||
| ); | |||
|
|
|||
| consumeRPCQueue( | |||
There was a problem hiding this comment.
issue (llm): The addition of a new consumeRPCQueue call for 'savings:contracts.updateOne' alongside the existing pattern for handling RPC queue messages introduces a notable increase in code complexity. This is primarily due to the increased code length, duplication of logic, and the added cognitive load required to understand and maintain these separate yet similar operations.
To mitigate these issues, consider abstracting the common logic into a more generic function or handler. This approach can significantly reduce duplication and simplify understanding the overall flow of RPC message handling. For instance, creating a function that handles the update operation based on parameters can consolidate the logic into a single, more manageable place. This not only makes the codebase cleaner but also eases future modifications and maintenance.
Here's a suggested refactor that encapsulates the common logic into a single function, thereby reducing complexity and improving code maintainability:
async function handleRPCUpdate(modelName, subdomain, data, isCustomUpdate = false) {
const models = await generateModels(subdomain);
let updateResult;
if (isCustomUpdate) {
// Handle custom update logic
updateResult = await models[modelName].updateOne(data.selector, data.modifier);
} else {
// Handle direct update logic
updateResult = await models[modelName].updateOne(
{ _id: data._id },
{ $set: { number: data.number } }
);
}
return {
status: 'success',
data: updateResult,
};
}
// Usage for RPC queue message handling
consumeRPCQueue('savings:contracts.updateOne', async ({ subdomain, data }) => {
return handleRPCUpdate('Contracts', subdomain, data, true);
});This refactor not only reduces the code complexity but also enhances the scalability of handling future RPC queue messages by providing a unified method to manage updates.
| const savingDetail = await fetchPolaris({ | ||
| subdomain, | ||
| op: '13610100', | ||
| data: [params.number, 0], | ||
| }).then((response) => JSON.parse(response)); | ||
|
|
||
| return savingDetail; |
There was a problem hiding this comment.
suggestion (rule): Sourcery has identified the following issue:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
| const savingDetail = await fetchPolaris({ | |
| subdomain, | |
| op: '13610100', | |
| data: [params.number, 0], | |
| }).then((response) => JSON.parse(response)); | |
| return savingDetail; | |
| return await fetchPolaris({ | |
| subdomain, | |
| op: '13610100', | |
| data: [params.number, 0], | |
| }).then((response) => JSON.parse(response)); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
| if (dateNames.includes(key)) { | ||
| const polarisDate = new Date( | ||
| new Date(polarisData[key]).getTime() - | ||
| new Date(polarisData[key]).getTimezoneOffset() * 60000, | ||
| ); | ||
| const mainDate = new Date(String(value)); | ||
| if (polarisDate.getTime() !== mainDate.getTime()) { | ||
| return mainData; | ||
| } | ||
| } else return mainData; |
There was a problem hiding this comment.
suggestion (rule): Sourcery has identified the following issue:
- Use block braces for ifs, whiles, etc. (
use-braces)
| if (dateNames.includes(key)) { | |
| const polarisDate = new Date( | |
| new Date(polarisData[key]).getTime() - | |
| new Date(polarisData[key]).getTimezoneOffset() * 60000, | |
| ); | |
| const mainDate = new Date(String(value)); | |
| if (polarisDate.getTime() !== mainDate.getTime()) { | |
| return mainData; | |
| } | |
| } else return mainData; | |
| if (dateNames.includes(key)) { | |
| const polarisDate = new Date( | |
| new Date(polarisData[key]).getTime() - | |
| new Date(polarisData[key]).getTimezoneOffset() * 60000, | |
| ); | |
| const mainDate = new Date(String(value)); | |
| if (polarisDate.getTime() !== mainDate.getTime()) { | |
| return mainData; | |
| } | |
| } else { | |
| return mainData; | |
| } | |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| if (polarisDate.getTime() !== mainDate.getTime()) | ||
| updateData[key] = polarisDate; |
There was a problem hiding this comment.
suggestion (rule): Sourcery has identified the following issue:
- Use block braces for ifs, whiles, etc. (
use-braces)
| if (polarisDate.getTime() !== mainDate.getTime()) | |
| updateData[key] = polarisDate; | |
| if (polarisDate.getTime() !== mainDate.getTime()) { | |
| updateData[key] = polarisDate; | |
| } | |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| } else if (fields.find((c) => c.code === key)) { | ||
| const field = fields.find((c) => c.code === key); | ||
| const index = customFieldsData.findIndex((c) => c.field === field._id); | ||
| index >= 0 | ||
| ? (customFieldsData[index].value = polarisData[key]) | ||
| : customFieldsData.push({ | ||
| field: field._id, | ||
| value: polarisData[key], | ||
| }); | ||
| updateData['customFieldsData'] = customFieldsData; | ||
| } else updateData[key] = polarisData[key]; |
There was a problem hiding this comment.
suggestion (rule): Sourcery has identified the following issue:
- Use block braces for ifs, whiles, etc. (
use-braces)
| } else if (fields.find((c) => c.code === key)) { | |
| const field = fields.find((c) => c.code === key); | |
| const index = customFieldsData.findIndex((c) => c.field === field._id); | |
| index >= 0 | |
| ? (customFieldsData[index].value = polarisData[key]) | |
| : customFieldsData.push({ | |
| field: field._id, | |
| value: polarisData[key], | |
| }); | |
| updateData['customFieldsData'] = customFieldsData; | |
| } else updateData[key] = polarisData[key]; | |
| } else if (fields.find((c) => c.code === key)) { | |
| const field = fields.find((c) => c.code === key); | |
| const index = customFieldsData.findIndex((c) => c.field === field._id); | |
| index >= 0 | |
| ? (customFieldsData[index].value = polarisData[key]) | |
| : customFieldsData.push({ | |
| field: field._id, | |
| value: polarisData[key], | |
| }); | |
| updateData['customFieldsData'] = customFieldsData; | |
| } else { | |
| updateData[key] = polarisData[key]; | |
| } | |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| const fields = await sendCommonMessage({ | ||
| subdomain, | ||
| serviceName: 'forms', | ||
| action: 'fields.find', | ||
| data: { | ||
| query: { | ||
| contentType: customFieldType, | ||
| code: { $exists: true, $ne: '' }, | ||
| }, | ||
| projection: { | ||
| groupId: 1, | ||
| code: 1, | ||
| _id: 1, | ||
| }, | ||
| }, | ||
| isRPC: true, | ||
| defaultValue: [], | ||
| }); | ||
| return fields; |
There was a problem hiding this comment.
suggestion (rule): Sourcery has identified the following issue:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
| const fields = await sendCommonMessage({ | |
| subdomain, | |
| serviceName: 'forms', | |
| action: 'fields.find', | |
| data: { | |
| query: { | |
| contentType: customFieldType, | |
| code: { $exists: true, $ne: '' }, | |
| }, | |
| projection: { | |
| groupId: 1, | |
| code: 1, | |
| _id: 1, | |
| }, | |
| }, | |
| isRPC: true, | |
| defaultValue: [], | |
| }); | |
| return fields; | |
| return await sendCommonMessage({ | |
| subdomain, | |
| serviceName: 'forms', | |
| action: 'fields.find', | |
| data: { | |
| query: { | |
| contentType: customFieldType, | |
| code: { $exists: true, $ne: '' }, | |
| }, | |
| projection: { | |
| groupId: 1, | |
| code: 1, | |
| _id: 1, | |
| }, | |
| }, | |
| isRPC: true, | |
| defaultValue: [], | |
| }); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
|
ISSUE
Context
Your context here. Additionally, any screenshots. Delete this line.
// Delete the below section once completed
PR Checklist