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

Removes need for report xml attachments #7596

Merged
merged 210 commits into from Sep 7, 2022

Conversation

dianabarsan
Copy link
Member

@dianabarsan dianabarsan commented Apr 20, 2022

Description

Instead of relying on xml attachments, uses report fields to populate form model on report edits.
Some changes have been required:

  1. db-doc fields will be saved in the main report fields. To maintain old report content display, these fields will become "hidden".
  2. Because of (1), it's not safe to default to using fields when editing a report that has a content attachment, so if the content attachment exists, it will be used to populate the form, but it will be deleted upon saving.
  3. bindJsonToXml is refactored so it treats arrays (repeats) correctly.

Limitations:

  1. Editing a report that created extra docs duplicates all extra docs #7594 - this behavior existed before
  2. Reports created as extra docs in forms are not editable #7605 - the additional context part
    Reports created as extra docs might not always be faithful to the model of the form itself.

Also migrates a bunch of e2e tests from protractor to wdio.

#5549

Code review checklist

  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or e2e where appropriate
  • Internationalised: All user facing text
  • Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@dianabarsan
Copy link
Member Author

Hey @jkuester . Would you mind having a look at this? I'm worried I might be missing some key feature here :)
Thanks!

Copy link
Contributor

@jkuester jkuester left a comment

Choose a reason for hiding this comment

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

Okay, I have spent a bunch of time digging into this and I love it! The implementation is simple and elegant, but should have a huge positive impact on the amount of storage used by the DB! I also really appreaciate the work that went into all the tests. ❤️

I left some comments inline, mostly minor except the one regarding the forms2sms.service.

webapp/src/ts/services/enketo-translation.service.ts Outdated Show resolved Hide resolved
webapp/src/ts/services/enketo-translation.service.ts Outdated Show resolved Hide resolved
webapp/src/ts/services/enketo-translation.service.ts Outdated Show resolved Hide resolved
tests/e2e/forms/submit-default-delivery-form.wdio-spec.js Outdated Show resolved Hide resolved
@dianabarsan dianabarsan marked this pull request as ready for review May 4, 2022 04:52
@dianabarsan dianabarsan requested a review from jkuester May 4, 2022 07:41
Copy link
Contributor

@jkuester jkuester left a comment

Choose a reason for hiding this comment

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

Ship it!

webapp/src/ts/services/form2sms.service.ts Outdated Show resolved Hide resolved
Base automatically changed from 6345-enketo-uplift to master August 16, 2022 19:03
…attachment

# Conflicts:
#	api/package-lock.json
#	config/default/forms/app/delivery.xlsx
#	config/default/forms/app/delivery.xml
#	config/default/forms/contact/PLACE_TYPE-create.xlsx
#	config/default/forms/contact/clinic-create.xml
#	config/default/forms/contact/district_hospital-create.xml
#	config/default/forms/contact/health_center-create.xml
#	config/default/forms/contact/person-create.xlsx
#	config/default/forms/contact/person-create.xml
#	config/default/forms/contact/person-edit.xlsx
#	config/default/forms/contact/person-edit.xml
#	config/standard/forms/app/immunization_visit.xlsx
#	tests/e2e/forms/submit-default-delivery-form.specs.js
#	tests/e2e/submit-enketo-form.wdio-spec.js
#	tests/page-objects/forms/default-delivery-report.po.js
#	tests/page-objects/forms/photo-upload.wdio.page.js
#	tests/page-objects/reports/reports.wdio.page.js
#	webapp/package-lock.json
#	webapp/package.json
#	webapp/src/js/enketo/file-manager.js
#	webapp/src/js/enketo/medic-xpath-extensions.js
#	webapp/src/js/enketo/widgets/db-object-widget.js
#	webapp/tests/mocha/unit/enketo/medic-xpath-extensions.spec.js
…attachment

# Conflicts:
#	tests/e2e/protractor/forms/submit-default-delivery-form.specs.js
#	tests/e2e/protractor/forms/submit-photo-upload-form.spec.js
#	tests/e2e/protractor/forms/submit-z-score-form.spec.js
@dianabarsan dianabarsan merged commit 58ea748 into master Sep 7, 2022
@dianabarsan dianabarsan deleted the 5549-no-report-xml-attachment branch September 7, 2022 11:32
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.

None yet

5 participants