add changes to report for insight

[batmnkh2344]

No description provided.

[sonarcloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[sourcery-ai]

PR Type: Enhancement

PR Summary: This pull request introduces several enhancements to the reports module, including the ability to duplicate reports, map report template types to service types during report creation and updates, and extend the message broker functionality with new operations for finding and updating reports in a more scalable manner. The changes aim to improve the flexibility, usability, and maintainability of the reports module.

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.

✅ Small diff: the diff is small enough to approve with confidence.

No details provided.

General suggestions:

• Consider abstracting the direct assignment of serviceType from doc.reportTemplateType to a function for future flexibility and easier maintenance.
• Implement error handling or transactional integrity for the duplication of charts to handle potential partial failures and ensure data consistency.
• Review the overall approach to duplicating reports and charts to ensure it aligns with the application's data integrity and error handling strategies.
• Ensure that the new message broker operations are well-integrated with the existing system and that they follow best practices for asynchronous communication and error handling.

packages/plugin-reports-api/src/graphql/resolvers/mutations.ts: Consider abstracting repeated logic for setting audit fields into a utility function to reduce complexity and enhance maintainability.

The addition of the reportsDuplicate function and modifications to existing functions introduce a higher level of complexity, primarily due to the increased number of database operations and the expanded responsibilities of the mutation functions. While the intent to extend functionality is clear and appreciated, there are opportunities to simplify the implementation and make the codebase easier to maintain.

For instance, both reportsAdd and reportsDuplicate share common logic for setting audit fields (createdBy, createdAt, updatedBy, updatedAt). This repeated logic could be abstracted into a utility function to streamline the code and enhance maintainability. Similarly, the duplication logic in reportsDuplicate could potentially be made more generic to handle duplicating documents with less specificity, reducing the complexity and making the function more adaptable.

Here's a suggested refactor for handling audit fields, which could be applied across your changes:

// Utility function to set audit fields
function setAuditFields(doc, userId) {
  const now = new Date();
  return {
    ...doc,
    createdBy: userId,
    createdAt: now,
    updatedBy: userId,
    updatedAt: now,
  };
}

// Example usage in reportsAdd
async reportsAdd(_root, doc: IReport, { models, user }: IContext) {
  const report = await models.Reports.createReport(setAuditFields(doc, user._id));
  // Additional logic...
}

// Example usage in reportsDuplicate
async reportsDuplicate(_root, _id: string, { models, user }: IContext) {
  const report = await models.Reports.findById(_id);
  if (!report) {
    throw new Error('Report not found');
  }
  const duplicatedReport = await models.Reports.createReport(setAuditFields(report.toObject(), user._id));
  // Duplication of charts...
}

This approach not only simplifies the current changes but also enhances the overall maintainability of the code by reducing repetition and making common operations more modular.

packages/plugin-reports-api/src/messageBroker.ts: Consider abstracting repeated logic into a single function to reduce complexity and improve maintainability.

While reviewing the proposed changes, I noticed that the new code introduces additional complexity, primarily due to the increased length, new dependencies, and the repetition of a similar pattern for handling different types of operations (find, updateMany) on reports. This could make the code harder to maintain and understand over time, especially as more operations might be added or existing ones need modifications.

To address these concerns, I suggest abstracting the repeated logic into a single function that handles the operation based on a parameter. This approach not only reduces the overall code length but also simplifies understanding and maintaining the code by centralizing the logic for generating models and performing database operations. Here's a refactored snippet that demonstrates this suggestion:

async function handleReportOperation(operation, subdomain, data) {
  const models = await generateModels(subdomain);
  switch (operation) {
    case 'find':
      return { data: await models.Reports.find(data).lean(), status: 'success' };
    case 'updateMany':
      const { selector, modifier } = data;
      return { data: await models.Reports.updateMany(selector, modifier), status: 'success' };
    default:
      throw new Error('Unsupported operation');
  }
}

export const initBroker = async () => {
  consumeRPCQueue('reports:find', async ({ subdomain, data }) => {
    return handleReportOperation('find', subdomain, data);
  });

  consumeRPCQueue('reports:updateMany', async ({ subdomain, data }) => {
    return handleReportOperation('updateMany', subdomain, data);
  });
};

This refactoring aims to make the code more maintainable by reducing repetition and centralizing the operation handling logic. It should also make it easier to add new operations or modify existing ones in the future.

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? ✨

Share Sourcery

• X
• Mastodon
• LinkedIn
• Facebook

---

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

[sourcery-ai]

suggestion (llm): The addition of serviceType directly from doc.reportTemplateType assumes a 1:1 relationship between these fields. If this mapping is always valid, this is fine. However, if there's a possibility of serviceType needing a different value or additional processing in the future, consider abstracting this assignment to a function for easier maintenance.

[sourcery-ai]

suggestion (llm): When duplicating charts, consider the potential for partial failures where some chart duplications might fail. It might be beneficial to implement transactional integrity or at least error handling that can rollback or notify in case of partial failures.