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

Patient Status CSV Report #1265

Closed
wants to merge 13 commits into from
Closed

Conversation

pascagihozo
Copy link
Contributor

Pull Requests Requirements

  • The PR title includes a brief description of the work done, including the
    Issue number if applicable.
  • The PR includes a video showing the changes for the work done.
  • The PR title follows conventional commit label standards.
  • The changes confirm to the OpenElis Global x3 Styleguide and design
    documentation.
  • The changes include tests or are validated by existing tests.
  • I have read and agree to the Contributing Guidelines of this project.

Summary

This PR is to introduce the CSV downloading functionality in OpenElis at Patient Status Report which was initially generating information in pdf but the users would like to have information in CSV as well to facilitate them in making statistical decisions. New class CSVPatientStatusReport and CSVPatientStatusReportColumn builder were created which copies the same mechanisms as CSVSampleRejectionReport and its column builder. The feature is patially working on my side i.e the endpoint is tested to be working( status 200) and the issue is that instead of receiving the CSV of patient data we are receiving html content in CSV file. Additionally the CSV Patient Status Report was not modified to have the sql querires that picks the data that is specific to the Patient Status Report so further help on writing SQL queries is beneficial as well. Please review the PR and help out @adityadeshlahre @mozzy11 @caseyi

Screenshots

Screencast.from.2024-09-16.22-19-47.webm

Related Issue

[Add a link to the related issue or mention it here if applicable]

Other

[Add any additional information or notes here]

…nt in the rejection controller and a function at the frontend to call that point
@mozzy11
Copy link
Collaborator

mozzy11 commented Sep 17, 2024

can you run mvn spotless:apply to format your changes

@pascagihozo
Copy link
Contributor Author

can you run mvn spotless:apply to format your changes

Hello @mozzy11
Done, thank you!

@mozzy11
Copy link
Collaborator

mozzy11 commented Sep 17, 2024

Thanks @pascagihozo .
The screen recording does not seem to be clear , can you attach a clear screen record or sample of the report generated ??

@pascagihozo
Copy link
Contributor Author

PatientStatusReport (6).csv
Screencast from 2024-09-17 15-14-53.webm

Hello @mozzy11

Please find the file generated and video capture of the Console &Network above

Thank you!

@@ -108,6 +108,23 @@ public ModelAndView showRejectionForm(HttpServletRequest request, @Validated Rej
return findForward(FWD_SUCCESS, form);
}

@RequestMapping(value = "/patientStatus", method = RequestMethod.GET)
Copy link
Collaborator

@mozzy11 mozzy11 Sep 23, 2024

Choose a reason for hiding this comment

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

you dont need a new endpoint for handling the report.
we already have this endpoint used by every report

@@ -61,6 +62,46 @@ function PatientStatusReport(props) {
});
};

// function to handle csv report start
Copy link
Collaborator

Choose a reason for hiding this comment

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

this may not be needed . we only need to change the report type param

Copy link
Collaborator

@mozzy11 mozzy11 Sep 23, 2024

Choose a reason for hiding this comment

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

you just need add a link in the report menu in this component
ie

{
     title: <FormattedMessage id="sidenav.title.statusreport.csv" />,
     icon: IbmWatsonDiscovery,
     SideNavMenuItem: [
       {
         link: "/RoutineReport?type=patient&report=CSVPatientStatusReport",
         label: <FormattedMessage id="sidenav.label.statusreport.csv" />,
       },
     ],
   },

then add report component here

ie

 {type === "patient" && report === "CSVPatientStatusReport" && (
        <PatientStatusReport
          report={"CSVPatientStatusReport"}
          id={"openreports.patientTestStatus.csv"}
        />
      )}

<br />
<br />
</Column>
<Column lg={16} md={8} sm={4}>
Copy link
Collaborator

@mozzy11 mozzy11 Sep 23, 2024

Choose a reason for hiding this comment

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

you can also get rid of this button. the existing button should be enough

@@ -143,6 +143,8 @@ public static IReportParameterSetter getParameterSetter(String report) {
return new CovidResultsReport();
} else if (report.equals("statisticsReport")) {
return new StatisticsReport();
} else if (report.equals("patientReport")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should use a consistent report param here ie CSVPatientStatusReport

@@ -62,6 +62,10 @@
* @since Mar 18, 2011
*/
public abstract class CSVColumnBuilder {
//
// public abstract String[] getHeaders();
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can remove unused comented code

Copy link
Collaborator

@mozzy11 mozzy11 left a comment

Choose a reason for hiding this comment

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

Thanks @pascagihozo for attempting to work on this report.
In general you can get rid of the new endpoint you created ie /patientStatus and use the existing endpoint /reportPrint. And also see other comments.

@pascagihozo pascagihozo closed this by deleting the head repository Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants