Skip to content
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

Bug-Fix: Patient's Age parsing discrepancy during XLS export in Sample Management System #9247

Merged
merged 11 commits into from
Dec 11, 2024

Conversation

pooranjoyb
Copy link
Contributor

@pooranjoyb pooranjoyb commented Nov 29, 2024

Proposed Changes

@ohcnetwork/care-fe-code-reviewers

Screenshots of the fix :

image

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

@Jacobjeevan Please have a look if this is good to go. Thank You :)

Summary by CodeRabbit

  • New Features

    • Improved data export functionality in the Sample View Admin component for better formatting and readability of patient data.
  • Bug Fixes

    • Enhanced processing of date fields for consistency and accuracy, including better error handling for invalid dates.

@pooranjoyb pooranjoyb requested a review from a team as a code owner November 29, 2024 11:40
Copy link
Contributor

coderabbitai bot commented Nov 29, 2024

Walkthrough

The changes in this pull request focus on the parseExportData function within the SampleViewAdmin component. The function's implementation has been revised to enhance the parsing of exported data, particularly for patient information. The new approach involves splitting the input into rows, extracting headers, and applying specific formatting to certain fields, such as "Patient Age" and date fields. The overall structure has been changed to improve readability and maintainability.

Changes

Files Change Summary
src/components/Patient/SampleViewAdmin.tsx Modified parseExportData function to improve data parsing and formatting for export functionality.

Assessment against linked issues

Objective Addressed Explanation
Properly populate Patient's Age in exported XLS files (#9231)

Possibly related PRs

Suggested labels

needs review, needs testing

Suggested reviewers

  • rithviknishad

Poem

In the land of data, we hop and play,
Fixing ages in a bright new way.
With rows and headers, we dance with glee,
Exporting samples, as smooth as can be!
Hooray for changes, let the data flow,
A rabbit's delight in the code we sow! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 85ee3a6 and d33a5fd.

📒 Files selected for processing (1)
  • src/components/Patient/SampleViewAdmin.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Patient/SampleViewAdmin.tsx

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Nov 29, 2024

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 28cfdc0
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/675816180622ec0008f2a363
😎 Deploy Preview https://deploy-preview-9247--care-ohc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
src/components/Patient/SampleViewAdmin.tsx (1)

127-138: Consider adding type safety for date fields.

The date formatting could fail if the input is not a valid date string. Consider adding validation:

 const columns = row.split(",").map((field, index) => {
   const header = headerColumns[index]?.trim();

   if (header === "Patient Age") {
     return field.trim();
   }

   if (header === "Date of Sample" || header === "Date of Result") {
-    return formatDateTime(field.trim());
+    const trimmedField = field.trim();
+    if (!trimmedField) {
+      return "";
+    }
+    try {
+      return formatDateTime(trimmedField);
+    } catch (error) {
+      console.error(`Invalid date format for ${header}: ${trimmedField}`);
+      return trimmedField;
+    }
   }
   return field.includes(",") ? `"${field.trim()}"` : field.trim();
 });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 2a89332 and ee4518f.

📒 Files selected for processing (1)
  • src/components/Patient/SampleViewAdmin.tsx (1 hunks)
🔇 Additional comments (2)
src/components/Patient/SampleViewAdmin.tsx (2)

120-143: LGTM! The implementation fixes the age parsing issue.

The special handling for the "Patient Age" column correctly prevents age values from being misinterpreted as dates during export.


120-143: Verify the fix across different age formats.

Let's verify that the age parsing works correctly for different age formats (e.g., "25", "25y", "2m", etc.) and edge cases.

✅ Verification successful

The age parsing implementation is safe and sufficient

The current implementation in parseExportData simply trims the age field without any additional parsing or formatting, which is the correct approach because:

  1. The function only handles the CSV export formatting, where the age data is already validated and stored in the backend
  2. The implementation correctly preserves the original age value by only trimming whitespace
  3. No special age format parsing is needed during export since we want to maintain the original data format

The simple field.trim() approach ensures that any age format (e.g., "25", "25y", "2m") will be preserved exactly as stored in the database, which is the desired behavior for data exports.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for age-related patterns in the codebase to understand supported formats

# Look for age-related patterns in test files
echo "Searching for age-related test cases..."
rg -l "Patient Age|patient_age" --type test

# Look for age validation or parsing logic
echo "Searching for age validation/parsing logic..."
rg "age|Age" --type typescript --type javascript

Length of output: 350


Script:

#!/bin/bash
# Search for age-related patterns in test files and source code
echo "Searching for age-related test cases and validation logic..."
rg -l "Patient Age|patient_age|parseAge"

# Look for specific age-related code with context
echo "Searching for age parsing/validation implementations..."
rg -A 5 "Patient Age|patient_age" 

# Search for CSV export related tests
echo "Searching for CSV export tests..."
rg -A 10 "parseExportData|exportData.*csv"

Length of output: 2560


Script:

#!/bin/bash
# Search for test files related to SampleViewAdmin
echo "Searching for test files related to SampleViewAdmin..."
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts SampleViewAdmin

# Look for CSV-related test data or mock data
echo "Searching for CSV test data or mock data..."
rg -A 5 "csv|CSV" --type tsx --type ts

# Look for the actual component implementation to see how age is handled in the table
echo "Searching for table implementation with age column..."
rg -A 10 "columns.*Patient Age" --type tsx --type ts

Length of output: 551


Script:

#!/bin/bash
# Search for test files and CSV-related content without file type restrictions
echo "Searching for SampleViewAdmin tests..."
find . -type f -name "*SampleViewAdmin*test*" -o -name "*SampleViewAdmin*spec*"

echo "Searching for CSV and age-related patterns..."
rg -A 5 'csv|CSV|"Patient Age"'

# Look for any table or column definitions
echo "Searching for table column definitions..."
ast-grep --pattern 'const columns = [
  $$$
  {
    $$$
    title: "Patient Age",
    $$$
  },
  $$$
]'

Length of output: 35874

src/components/Patient/SampleViewAdmin.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@Jacobjeevan Jacobjeevan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pooranjoyb
Copy link
Contributor Author

Hii @nihal467 please check if this is good to go. :)

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I'd have recommended building a reusable utility function that could do something like shown, but this is also fine for now.

transformCSVData(
  data,
  {
    dateFields: ['field1', 'field2'],
    customTransforms: [
      ['field1']: (cell) => output
    ]
  }
)

@nihal467
Copy link
Member

nihal467 commented Dec 3, 2024

image

@pooranjoyb when the date of result is empty, its been shown as invalid date, if the data is not available, leave the column blank

@pooranjoyb
Copy link
Contributor Author

Okaay Updating it asap. @nihal467

@pooranjoyb
Copy link
Contributor Author

pooranjoyb commented Dec 3, 2024

@pooranjoyb when the date of result is empty, its been shown as invalid date, if the data is not available, leave the column blank

Changes are made as requested. Please have a look sir @nihal467

image

@github-actions github-actions bot added needs-triage question Further information is requested labels Dec 3, 2024
@pooranjoyb
Copy link
Contributor Author

Hey @nihal467 (sorry for the delay)
But it seems the age of the Patient is being directly fetched from the BE and not being handled here in FE.

This is the 3rd record in devdoctor account.
image

You can see the data is fetched directly from BE.

<ExportButton
action={async () => {
const { data } = await request(routes.getTestSampleList, {
query: { ...qParams, csv: true },
});
return data ?? null;
}}
parse={parseExportData}
filenamePrefix="samples"
/>

Please lemme know if I'm missing something here. @nihal467 @rithviknishad @Jacobjeevan

@nihal467 nihal467 added the question Further information is requested label Dec 9, 2024
@nihal467
Copy link
Member

nihal467 commented Dec 9, 2024

@rithviknishad can you look into the issue

@rithviknishad
Copy link
Member

@nihal467 We can show 0 itself right? Changes in the back-end will be required for this since we are not getting the DOB of the patient in this API.

@pooranjoyb
Copy link
Contributor Author

@nihal467 We can show 0 itself right? Changes in the back-end will be required for this since we are not getting the DOB of the patient in this API.

Yes, of course 0 can be done. If you'd ask, I can go with the requested changes.

@pooranjoyb
Copy link
Contributor Author

@rithviknishad @nihal467 Requested changes are pushed. Please review. Thanks :)

@rithviknishad rithviknishad added needs testing and removed question Further information is requested changes required labels Dec 10, 2024
@nihal467
Copy link
Member

LGTM

@Jacobjeevan Jacobjeevan added reviewed reviewed by a core member and removed needs review labels Dec 11, 2024
@rithviknishad rithviknishad merged commit 27b7228 into ohcnetwork:develop Dec 11, 2024
19 checks passed
Copy link

@pooranjoyb Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewed reviewed by a core member tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Exported XLS files has mismatched Patient's Age (Sample Test Management)
4 participants