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.

Sign up for GitHub

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

Jump to bottom

Feature/attachment report #22

Open
wants to merge 9 commits into
base: development
Choose a base branch
from
Open

Feature/attachment report #22

wants to merge 9 commits into from

Conversation

idelcano
Copy link
Contributor

@idelcano idelcano commented Aug 3, 2022 •
edited by eperedo
Loading

📌 References

https://app.clickup.com/t/2k5fm6v

📝 Implementation

Added nhwa-attachments variant
already installed in dev:
https://extranet.who.int/dhis2-dev/dhis-web-reports/index.html#/standard-report/view/nRtut85yUKl

looks like the dockers don't have the files but load right in dev.
Started from a copy of nhwa-comments
the report needs the following sqlview:
https://extranet.who.int/dhis2-dev/dhis-web-maintenance/index.html#/edit/otherSection/sqlView/eXSJr2ZBJYJ

  • added a download button instead opening the file in the same tab

🎨 Screenshots

Download button
image

🔥 Notes to the tester

@idelcano idelcano requested a review from tokland August 3, 2022 15:06
tokland
tokland reviewed Aug 8, 2022
View reviewed changes
Copy link
Contributor

@tokland tokland left a comment

Choose a reason for hiding this comment

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

I've checked the core (let me know if I have to check the feature). Looks good, just some comments:

src/compositionRoot.ts Outdated
@@ -45,6 +49,10 @@ export function getCompositionRoot(api: D2Api) {
config: getExecute({
get: new GetConfig(configRepository),
}),
attachements: getExecute({
Copy link
Contributor

@tokland tokland Aug 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

typo: attachments, in variables/functions and file names.

src/webapp/reports/nhwa-attachments/data-attachment-list/DataAttachmentsList.tsx Outdated
text: "Download CSV",
icon: <StorageIcon />,
onClick: async () => {
if (!sorting) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can set an initial default sorting (in the useState)?

src/webapp/reports/nhwa-attachments/data-attachment-list/FiltersBox.tsx Outdated
@@ -0,0 +1,46 @@
import React from "react";
Copy link
Contributor

@tokland tokland Aug 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This file is exactly the same as data-comments-list/FiltersBox.ts and it's unlikely their functionality will diverge, it's a generic component, so it can be moved to another folder and used in both places. Use prop children.

src/webapp/reports/nhwa-attachments/data-attachment-list/Filters.tsx Outdated
}, [options]);
}

function useMemoOptionsFromNamedRef(options: NamedRef[]) {
Copy link
Contributor

@tokland tokland Aug 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

These two functions are repeated in many places, they can be abstracted to some helper method.

@reviewpad reviewpad bot added large Pull request is large waiting-for-review labels Nov 30, 2023
@reviewpad Reviewpad
Copy link

reviewpad bot commented Nov 30, 2023 •
edited
Loading

Reviewpad Report

‼️ Errors

  • Unconventional commit detected: 'added new attachement report' (1da263d)
  • Unconventional commit detected: 'remove dataelement and add working link' (f63c0da)
  • Unconventional commit detected: 'added variant name into readme' (08657e5)
  • Unconventional commit detected: 'fix typo' (37b9006)
  • Unconventional commit detected: 'Merge branch 'development' of github.com:EyeSeeTea/d2-reports into feature/attachment_report' (84dd153)
  • Unconventional commit detected: 'Reuse filter component' (d9e0821)
  • Unconventional commit detected: 'remove duplicate components, methods, entities and refactor code' (7a5ad29)
  • Unconventional commit detected: 'add download file option' (c7d7601)
  • Unconventional commit detected: 'update i18n' (d36dc90)
  • Unconventional title detected: 'Feature/attachment report' expecting colon (':') character, got 'u' character: col=04

@reviewpad Reviewpad
Copy link

reviewpad bot commented Nov 30, 2023

AI-Generated Summary: This pull request introduces new features related to the handling of NHWA attachments in the application. It provides users with the ability to view and download NHWA attachment reports.

Here are the significant changes:

  • A new section for NHWA attachments has been added in the README.md file.
  • Additions have been done in internationalization files (i18n/en.pot and i18n/es.po) to include messages related to NHWA Attachment Report.
  • A new repository (NHWAAttachementsDefaultRepository) has been created to handle the fetching and saving of NHWA attachments.
  • New domain entities (like DataAttachmentItem) and repositories (NHWADataAttachmentsRepository) have been introduced under the domain layer to support the NHWA attachments feature.
  • Two new use cases (GetAttachementsUseCase and ExportAttachmentsUseCase) have been added to encapsulate the operations related to fetching and exporting attachment data.
  • Changes are made in Reports.tsx and new files (NHWAAttachmentReport.tsx, DataAttachmentsViewModel.ts, DataAttachmentsList.tsx etc.) are added under the reports directory to provide a view for NHWA attachments.
  • There are updates in the Dhis2ConfigRepository to accommodate NHWA attachments data.
  • A few minor adjustments and bug fixes have been done in other parts of the code to ensure the seamless integration of the new feature.

These updates will enhance the functionality of the application by providing more comprehensive and efficient handling of NHWA attachments data.

@eperedo eperedo requested a review from ifoche November 30, 2023 22:33
@reviewpad Reviewpad
Copy link

reviewpad bot commented Dec 1, 2023

AI-Generated Summary: This pull request includes multiple changes to support a new feature known as "NHWA Attachments".

  1. New components are added namely NHWAAttachmentReport, DataAttachmentsList, and uses cases for fetching and exporting attachment data.
  2. The getCompositionRoot function is updated to allow a dependency injection for attachments.
  3. A new SQLView named "NHWA attachments" is added.
  4. Additional Repository classes and related components are created to support the NHWA attachments logic.
  5. Support for new filters in DataCommentsList and NHWAAttachments are provided.
  6. Translations for the new components are updated in 'i18n'.
  7. The README file is updated to include information about the new nhwa-attachments variant.

The changes primarily aim to provide user ability to download and view NHWA attachments .

@reviewpad Reviewpad
Copy link

reviewpad bot commented Dec 1, 2023

AI-Generated Summary: This pull request makes numerous changes to add a new feature related to NHWA attachments reporting.

It makes modifications to README.md to include a new variant. Updates to i18n translations for English and Spanish locales indicate added translations related to the new feature.

It introduces multiple new files in the src/domain/nhwa-attachments directory, defining new interfaces for DataAttachmentItem and NHWADataAttachmentsRepository among others. This suggests the introduction of a new use case – handling data attachments in NHWA reports.

Further, it includes new React component NHWAAttachmentReport and its dependencies that seem to handle displaying, sorting, filtering, and downloading data attachments in the NHWA reports.

Updates to existing files like src/data/common/Dhis2ConfigRepository.ts, src/compositionRoot.ts, and src/webapp/reports/Reports.tsx suggest a proper integration of these new features with the existing codebase - adding new repositories, attaching new use cases, and including a new report variant respectively.

There's also a new SQL view added for NHWA attachments.

This pull request looks like an effort to add considerable new functionality related to handling and reporting NHWA attachments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tokland tokland tokland left review comments

@ifoche ifoche Awaiting requested review from ifoche

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
large Pull request is large waiting-for-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@idelcano @tokland @eperedo