Skip to content

2972 place of stay in bulk case edit#3144

Merged
MateStrysewske merged 12 commits intodevelopmentfrom
2972-place-of-stay-in-bulk-case-edit
Oct 14, 2020
Merged

2972 place of stay in bulk case edit#3144
MateStrysewske merged 12 commits intodevelopmentfrom
2972-place-of-stay-in-bulk-case-edit

Conversation

@danRosca
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@MateStrysewske MateStrysewske left a comment

Choose a reason for hiding this comment

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

  • Please use proper code formatting
  • Warning text should be shown next to the warning icon, not below it
  • Warning icon does not disappear when I change the value back to "Facility"
  • Facility fields should be read-only if "Change facility" has not been checked
  • Make sure that no logic concerning facility/type of place is executed if "Change facility" is not checked
  • Selecting "Home" and then changing the region, district or community hides the Place Description field; that shouldn't happen
  • Place of Stay must be required if "Change facility" has been selected

messageDatabaseExportFailed = Please contact an admin and notify them about this problem
messageEntryCreated = Entry created
messageEpiDataHint = <i>Please indicate if any of the following is relevant for the patient during the incubation period.</i>
pseudonymisedCasesSelectedWarning =<p> For of the bulked edited cases you have only limited access to the sensitive data.</p><p> For those cases the value you put into "Place Description" will be ignored.</p>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some words seem to be missing here. Also remove the blank space after the <p> tag.
Additionally: "bulked edited" => "bulk-edited"

String promptTypeToAdd = "promptTypeToAdd";
String promptUserSearch = "promptUserSearch";
String promtSampleDataType = "promtSampleDataType";
String pseudonymisedCasesSelectedWarning = "pseudonymisedCasesSelectedWarning";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Pseudonymised" => "Pseudonymized" since we're using American English

TYPE_GROUP_LOC,
TYPE_LOC,
CaseBulkEditData.HEALTH_FACILITY);
fluidRowLocs(FACILITY_OR_HOME_LOC,TYPE_GROUP_LOC, TYPE_LOC)+
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

White spaces after , and before +

private ComboBox facilityType;
private TextField healthFacilityDetails;
private Label warningLabel;
private Collection selectedCases;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be typed here as well (=> Collection<? extends CaseIndexDto>)

updatedCase.setCommunity(updatedCaseBulkEditData.getCommunity());
updatedCase.setHealthFacility(updatedCaseBulkEditData.getHealthFacility());
updatedCase.setHealthFacilityDetails(updatedCaseBulkEditData.getHealthFacilityDetails());
updatedCase.setHealthFacilityDetails(updatedCaseBulkEditData.getHealthFacilityDetails());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Duplicated line of code

return infoLayout;
}

public static HorizontalLayout createGenericComponent(String htmlContent, String iconName) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a lot of duplicated code, also the name of the method isn't really helpful; since we have two very specific use cases: rename this method to "createIconComponent", remove the code from createInfoComponent and just call createIconComponent with the info icon and add another "createWarningComponent" method that calls createIconComponent with the warning icon and use that new method

facilityOrHome.setWidth(100, Unit.PERCENTAGE);
facilityOrHome.setEnabled(false);
CssStyles.style(facilityOrHome, ValoTheme.OPTIONGROUP_HORIZONTAL);
getContent().addComponent(facilityOrHome, FACILITY_OR_HOME_LOC);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should rather add the field with addField which also makes the this.getValue().setHealthFacilityDetails(healthFacilityDetails.getValue()) call later in this file obsolete.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is possible since facilityOrHome is not a CaseBulkEditData field. I also looked in the CaseDataForm class and facilityorHome is created the same way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, yeah, my comment doesn't make sense. See below.

FieldHelper
.setRequiredWhen(getFieldGroup(), investigationStatusCheckBox, Arrays.asList(CaseBulkEditData.INVESTIGATION_STATUS), Arrays.asList(true));
FieldHelper.setRequiredWhen(getFieldGroup(), outcomeCheckBox, Arrays.asList(CaseBulkEditData.OUTCOME), Arrays.asList(true));
FieldHelper.setRequiredWhen(healthFacilityCheckbox, Arrays.asList(facilityOrHome), Arrays.asList(true), false, null);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove this line and add the facilityOrHome field to the setRequiredWhen call in the next line

if (!visibleAndRequired) {
tfFacilityDetails.clear();
}
} else {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

=> else if

@@ -15,155 +15,150 @@
# You should have received a copy of the GNU General Public License
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You have to revert the changes to this file. We'll run into heavy merge problems with the I18n branch and during a potential hotfix like this.

facilityOrHome.setWidth(100, Unit.PERCENTAGE);
facilityOrHome.setEnabled(false);
CssStyles.style(facilityOrHome, ValoTheme.OPTIONGROUP_HORIZONTAL);
getContent().addComponent(facilityOrHome, FACILITY_OR_HOME_LOC);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, yeah, my comment doesn't make sense. See below.

.updateItems(district, regionDto != null ? FacadeProvider.getDistrictFacade().getAllActiveByRegion(regionDto.getUuid()) : null);
});
healthFacilityDetails.addValueChangeListener(e -> {
this.getValue().setHealthFacilityDetails(healthFacilityDetails.getValue());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this necessary? The healthFacilityDetails entered should automatically be added to the bound CaseBulkEditData.

@MateStrysewske MateStrysewske merged commit 738abf6 into development Oct 14, 2020
@MateStrysewske MateStrysewske deleted the 2972-place-of-stay-in-bulk-case-edit branch October 14, 2020 10:16
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