Skip to content

Commit

Permalink
Merge Third-Culture-Software#2616
Browse files Browse the repository at this point in the history 
2616: fix: correct filter display values r=jniles a=jniles

The search filters must update their display values if the underlying value changes.  Previously, the default case was to preserve display values, but it didn't take into account the changes in the underlying value, leading to display values being out of sync with the values.

This was reported in Third-Culture-Software#2595.
  • Loading branch information
@bors
bors[bot] committed Mar 26, 2018
2 parents 8cfe5e1 + 826d729 commit 3d8cc58
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 61 deletions.
4 changes: 1 addition & 3 deletions client/src/js/services/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ function UtilService(moment) {
}
}



/** @todo comments showing usage */
service.filterFormElements = function filterFormElements(formDefinition, requireDirty) {
var response = {};
Expand Down Expand Up @@ -270,4 +268,4 @@ function UtilService(moment) {
return array.indexOf(value) !== -1;
});
};
}
}
61 changes: 36 additions & 25 deletions client/src/modules/cash/payments/templates/search.modal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,41 @@ angular.module('bhima.controllers')
.controller('SearchCashPaymentModalController', SearchCashPaymentModalController);

SearchCashPaymentModalController.$inject = [
'NotifyService', '$uibModalInstance', 'filters', 'Store', 'PeriodService', 'util', 'CashService', 'CurrencyService',
'NotifyService', '$uibModalInstance', 'filters', 'Store', 'PeriodService',
'util', 'CashService', 'CurrencyService',
];

/**
* Search Cash Payment controller
*
* @description
* This controller powers the Invoice Search modal. Invoices are passed in from the registry as
* This controller powers the Cash Search modal. Cash filters are passed in from the registry as
* POJO and are attached to the view. They are modified here and returned to the parent controller
* as a POJO.
*/
function SearchCashPaymentModalController(Notify, Instance, filters, Store, Periods, util, Cash, Currencies) {
var vm = this;
var changes = new Store({ identifier : 'key' });
// @TODO ideally these should be passed in when the modal is initialised
// these are known when the filter service is defined
var searchQueryOptions = [
'is_caution', 'reference', 'cashbox_id', 'user_id', 'reference_patient', 'currency_id', 'reversed', 'debtor_group_uuid',
];
const vm = this;
const changes = new Store({ identifier : 'key' });

vm.filters = filters;
const searchQueryOptions = [
'is_caution', 'reference', 'cashbox_id', 'user_id', 'reference_patient',
'currency_id', 'reversed', 'debtor_group_uuid',
];

vm.searchQueries = {};
vm.defaultQueries = {};

// displayValues will be an id:displayValue pair
var displayValues = {};
var lastDisplayValues = Cash.filters.getDisplayValueMap();
const displayValues = {};
const lastDisplayValues = Cash.filters.getDisplayValueMap();

// assign already defined custom filters to searchQueries object
vm.searchQueries = util.maskObjectFromKeys(filters, searchQueryOptions);

// keep track of the initial search queries to make sure we properly restore
// default display values
const initialSearchQueries = angular.copy(vm.searchQueries);

// assign default filters
if (filters.limit) {
vm.defaultQueries.limit = filters.limit;
Expand All @@ -51,9 +54,9 @@ function SearchCashPaymentModalController(Notify, Instance, filters, Store, Peri

// load all the available currencies
Currencies.read()
.then(function (currencies) {
.then(currencies => {
// cache a label for faster view rendering
currencies.forEach(function (currency) {
currencies.forEach(currency => {
currency.label = Currencies.format(currency.id);
});

Expand All @@ -75,7 +78,7 @@ function SearchCashPaymentModalController(Notify, Instance, filters, Store, Peri
};

vm.setCurrency = function setCurrency(currencyId) {
vm.currencies.forEach(function (currency) {
vm.currencies.forEach(currency => {
if (currency.id === currencyId) {
displayValues.currency_id = currency.label;
}
Expand All @@ -84,9 +87,9 @@ function SearchCashPaymentModalController(Notify, Instance, filters, Store, Peri

// default filter period - directly write to changes list
vm.onSelectPeriod = function onSelectPeriod(period) {
var periodFilters = Periods.processFilterChanges(period);
const periodFilters = Periods.processFilterChanges(period);

periodFilters.forEach(function (filterChange) {
periodFilters.forEach(filterChange => {
changes.post(filterChange);
});
};
Expand All @@ -95,11 +98,10 @@ function SearchCashPaymentModalController(Notify, Instance, filters, Store, Peri
vm.onSelectLimit = function onSelectLimit(value) {
// input is type value, this will only be defined for a valid number
if (angular.isDefined(value)) {
changes.post({ key : 'limit', value : value });
changes.post({ key : 'limit', value });
}
};


// deletes a filter from the custom filter object, this key will no longer be written to changes on exit
vm.clear = function clear(key) {
delete vm.searchQueries[key];
Expand All @@ -108,15 +110,24 @@ function SearchCashPaymentModalController(Notify, Instance, filters, Store, Peri
// returns the filters to the journal to be used to refresh the page
vm.submit = function submit() {
// push all searchQuery values into the changes array to be applied
angular.forEach(vm.searchQueries, function (value, key) {
angular.forEach(vm.searchQueries, (value, key) => {
if (angular.isDefined(value)) {
// default to the original value if no display value is defined
var displayValue = displayValues[key] || lastDisplayValues[key] || value;
changes.post({ key: key, value: value, displayValue: displayValue });
}

// To avoid overwriting a real display value, we first determine if the value changed in the current view.
// If so, we do not use the previous display value. If the values are identical, we can restore the
// previous display value without fear of data being out of date.
const usePreviousDisplayValue =
angular.equals(initialSearchQueries[key], value) &&
angular.isDefined(lastDisplayValues[key]);

// default to the raw value if no display value is defined
const displayValue = usePreviousDisplayValue ? lastDisplayValues[key] : displayValues[key] || value;

changes.post({ key, value, displayValue });
}
});

var loggedChanges = changes.getAll();
const loggedChanges = changes.getAll();

// return values to the CashController
return Instance.close(loggedChanges);
Expand Down
41 changes: 27 additions & 14 deletions client/src/modules/invoices/registry/search.modal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ InvoiceRegistrySearchModalController.$inject = [
* the underlying filters before passing them back to the parent controller.
*/
function InvoiceRegistrySearchModalController(ModalInstance, filters, Notify, Store, Periods, util, Invoices) {
var vm = this;
var changes = new Store({ identifier : 'key' });
const vm = this;
const changes = new Store({ identifier : 'key' });
vm.filters = filters;

vm.defaultQueries = {};

// displayValues will be an id:displayValue pair
var displayValues = {};
var lastDisplayValues = Invoices.filters.getDisplayValueMap();
const displayValues = {};
const lastDisplayValues = Invoices.filters.getDisplayValueMap();

// assign default limit filter
if (filters.limit) {
Expand All @@ -30,13 +30,18 @@ function InvoiceRegistrySearchModalController(ModalInstance, filters, Notify, St

// @TODO ideally these should be passed in when the modal is initialised
// these are known when the filter service is defined
var searchQueryOptions = [
'is_caution', 'reference', 'cashbox_id', 'user_id', 'reference_patient', 'currency_id', 'reversed', 'service_id', 'debtor_group_uuid',
const searchQueryOptions = [
'is_caution', 'reference', 'cashbox_id', 'user_id', 'reference_patient',
'currency_id', 'reversed', 'service_id', 'debtor_group_uuid',
];

// assign already defined custom filters to searchQueries object
vm.searchQueries = util.maskObjectFromKeys(filters, searchQueryOptions);

// keep track of the initial search queries to make sure we properly restore
// default display values
const initialSearchQueries = angular.copy(vm.searchQueries);

// set controller data
vm.cancel = ModalInstance.close;

Expand All @@ -62,15 +67,15 @@ function InvoiceRegistrySearchModalController(ModalInstance, filters, Notify, St
vm.onSelectLimit = function onSelectLimit(value) {
// input is type value, this will only be defined for a valid number
if (angular.isDefined(value)) {
changes.post({ key : 'limit', value : value });
changes.post({ key : 'limit', value });
}
};

// default filter period - directly write to changes list
vm.onSelectPeriod = function onSelectPeriod(period) {
var periodFilters = Periods.processFilterChanges(period);
const periodFilters = Periods.processFilterChanges(period);

periodFilters.forEach(function (filterChange) {
periodFilters.forEach(filterChange => {
changes.post(filterChange);
});
};
Expand All @@ -83,15 +88,23 @@ function InvoiceRegistrySearchModalController(ModalInstance, filters, Notify, St
// returns the filters to the journal to be used to refresh the page
vm.submit = function submit() {
// push all searchQuery values into the changes array to be applied
angular.forEach(vm.searchQueries, function (value, key) {
angular.forEach(vm.searchQueries, (value, key) => {
if (angular.isDefined(value)) {
// default to the original value if no display value is defined
var displayValue = displayValues[key] || lastDisplayValues[key] || value;
changes.post({ key: key, value: value, displayValue: displayValue });
// To avoid overwriting a real display value, we first determine if the value changed in the current view.
// If so, we do not use the previous display value. If the values are identical, we can restore the
// previous display value without fear of data being out of date.
const usePreviousDisplayValue =
angular.equals(initialSearchQueries[key], value) &&
angular.isDefined(lastDisplayValues[key]);

// default to the raw value if no display value is defined
const displayValue = usePreviousDisplayValue ? lastDisplayValues[key] : displayValues[key] || value;

changes.post({ key, value, displayValue });
}
});

var loggedChanges = changes.getAll();
const loggedChanges = changes.getAll();

// return values to the Invoice Registry Controller
return ModalInstance.close(loggedChanges);
Expand Down
38 changes: 27 additions & 11 deletions client/src/modules/patients/registry/search.modal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ angular.module('bhima.controllers')
.controller('PatientRegistryModalController', PatientRegistryModalController);

PatientRegistryModalController.$inject = [
'$uibModalInstance', 'filters',
'bhConstants', 'moment', 'Store', 'util', 'PeriodService',
'$uibModalInstance', 'filters', 'Store', 'util', 'PeriodService', 'PatientService',
];

/**
Expand All @@ -14,15 +13,17 @@ PatientRegistryModalController.$inject = [
* search functionality on the patient registry page. Filters that are already
* applied to the grid can be passed in via the filters inject.
*/
function PatientRegistryModalController(ModalInstance, filters, bhConstants, moment, Store, util, Periods) {
function PatientRegistryModalController(ModalInstance, filters, Store, util, Periods, Patients) {
const vm = this;
const changes = new Store({ identifier : 'key' });

// displayValues will be an id:displayValue pair
const displayValues = {};
// @TODO ideally these should be passed in when the modal is initialised these are known when the filter service is defined

const searchQueryOptions = [
'display_name', 'sex', 'hospital_no', 'reference', 'dateBirthFrom', 'dateBirthTo', 'dateRegistrationFrom', 'dateRegistrationTo',
'debtor_group_uuid', 'patient_group_uuid', 'user_id', 'defaultPeriod',
'display_name', 'sex', 'hospital_no', 'reference', 'dateBirthFrom', 'dateBirthTo',
'dateRegistrationFrom', 'dateRegistrationTo', 'debtor_group_uuid',
'patient_group_uuid', 'user_id', 'defaultPeriod',
];

vm.filters = filters;
Expand All @@ -36,9 +37,15 @@ function PatientRegistryModalController(ModalInstance, filters, bhConstants, mom
vm.defaultQueries.limit = filters.limit;
}

const lastDisplayValues = Patients.filters.getDisplayValueMap();

// assign already defined custom filters to searchQueries object
vm.searchQueries = util.maskObjectFromKeys(filters, searchQueryOptions);

// keep track of the initial search queries to make sure we properly restore
// default display values
const initialSearchQueries = angular.copy(vm.searchQueries);

// bind methods
vm.submit = submit;
vm.cancel = cancel;
Expand Down Expand Up @@ -81,11 +88,20 @@ function PatientRegistryModalController(ModalInstance, filters, bhConstants, mom
// returns the parameters to the parent controller
function submit() {
// push all searchQuery values into the changes array to be applied
angular.forEach(vm.searchQueries, (val, _key) => {
if (angular.isDefined(val)) {
// default to the original value if no display value is defined
const displayVal = displayValues[_key] || val;
changes.post({ key : _key, value: val, displayValue : displayVal });
angular.forEach(vm.searchQueries, (value, key) => {
if (angular.isDefined(value)) {

// To avoid overwriting a real display value, we first determine if the value changed in the current view.
// If so, we do not use the previous display value. If the values are identical, we can restore the
// previous display value without fear of data being out of date.
const usePreviousDisplayValue =
angular.equals(initialSearchQueries[key], value) &&
angular.isDefined(lastDisplayValues[key]);

// default to the raw value if no display value is defined
const displayValue = usePreviousDisplayValue ? lastDisplayValues[key] : displayValues[key] || value;

changes.post({ key, value, displayValue });
}
});

Expand Down
26 changes: 18 additions & 8 deletions client/src/modules/vouchers/modals/search.modal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ VoucherRegistrySearchModalController.$inject = [
* preset by passing in a filters object using filtersProvider().
*/
function VoucherRegistrySearchModalController(
ModalInstance, filters, Notify, Periods, Store, util,
Vouchers, TransactionTypeService, $translate,
ModalInstance, filters, Notify, Periods, Store, util, Vouchers,
TransactionTypeService, $translate,
) {
const vm = this;
const changes = new Store({ identifier : 'key' });
Expand All @@ -28,14 +28,18 @@ function VoucherRegistrySearchModalController(
const displayValues = {};
const lastDisplayValues = Vouchers.filters.getDisplayValueMap();

vm.filters = filters;
// searchQueries is the same id:value pair
vm.searchQueries = {};
vm.defaultQueries = {};


// assign already defined custom filters to searchQueries object
vm.searchQueries = util.maskObjectFromKeys(filters, searchQueryOptions);

// keep track of the initial search queries to make sure we properly restore
// default display values
const initialSearchQueries = angular.copy(vm.searchQueries);

if (filters.limit) {
vm.defaultQueries.limit = filters.limit;
}
Expand Down Expand Up @@ -101,21 +105,27 @@ function VoucherRegistrySearchModalController(

// submit the filter object to the parent controller.
vm.submit = function submit(form) {
let _displayValue;


if (form.$invalid) { return; }

// delete type_ids if there is no transaction type sent
if (vm.searchQueries.type_ids && vm.searchQueries.type_ids.length === 0) {
vm.clear('type_ids');
}


// push all searchQuery values into the changes array to be applied
angular.forEach(vm.searchQueries, (_value, _key) => {
if (angular.isDefined(_value)) {
// default to the original value if no display value is defined
_displayValue = displayValues[_key] || lastDisplayValues[_key] || _value;

// To avoid overwriting a real display value, we first determine if the value changed in the current view.
// If so, we do not use the previous display value. If the values are identical, we can restore the
// previous display value without fear of data being out of date.
const usePreviousDisplayValue =
angular.equals(initialSearchQueries[_key], _value) &&
angular.isDefined(lastDisplayValues[_key]);

// default to the raw value if no display value is defined
const _displayValue = usePreviousDisplayValue ? lastDisplayValues[_key] : displayValues[_key] || _value;
changes.post({ key : _key, value : _value, displayValue : _displayValue });
}
});
Expand Down

0 comments on commit 3d8cc58

Please sign in to comment.