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

Remove the deprecated filter into patient registry #1969

Merged
merged 3 commits into from
Aug 14, 2017

Conversation

DedrickEnc
Copy link
Contributor

In this PR :

  • The deprecated filter is removed from the patient registry file,

  • The search modal was also updated to follow the new standard

  • A new component was implemented bhPatientGroupSelect and its test

This PR references #1942
Thank you for contributing!

Before submitting this pull request, please verify that you have:

  • Run your code through eslint.
  • Run integration tests.
  • Run end-to-end tests.
  • Accurately described the changes your are making in this pull request.

For a more detailed checklist, see the online review checklist that this PR will be evaluated against.

Ensuring that the above checkboxes are completed will help speed the review process and help build a stronger application. Thanks!

Copy link
Collaborator

@jniles jniles left a comment

Choose a reason for hiding this comment

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

@DedrickEnc, this look pretty good!

There are a couple small changes to make - the links from other registries are broken here, but in general the code is well written.

@@ -62,8 +62,9 @@
</div>
</div>

<div class="flex-util bh-filter-bar">
<div class="flex-util" style="min-height : 35px; padding-top : 7px; max-height: initial">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think these styles should have been updated - was there a reason?

@@ -193,15 +185,6 @@ function PatientRegistryController($state, Patients, Notify, AppCache,

// startup function. Checks for cached filters and loads them. This behavior could be changed.
function startup() {

if ($state.params.filters) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You still need to do something like this -- it will break the links from the Payments and Invoice Registries into the Patient Registry.

I suggest you implement something like #1935 to solve this.

@@ -15,94 +15,90 @@
<uib-tabset>
<uib-tab index="0" heading="{{ 'FORM.LABELS.SEARCH_QUERRIES' | translate }}" data-custom-filter-tab>
<div class="tab-body">
<div class="form-group" ng-class="{ 'has-error' : ModalForm.$submitted && ModalForm.name.$invalid }">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch! 👍

.then(function (result) {
vm.patientGroups = result;
});
vm.onSelectPatientGroup = function onSelectPatientGroup(patientGroup) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a nice improvement 👍


$ctrl.$onInit = function onInit() {
// fired when a patient group has been selected
$ctrl.onSelectCallback = $ctrl.onSelectCallback || angular.noop;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is required, you don't actually need to instantiate it - angular will default to angular.noop if it is not defined.

PatientGroups.read()
.then(function (pgs) {
$ctrl.patientGroups = pgs;
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should have a .catch() method.

@sfount sfount changed the title Remove the debrecated filter into patient registry Remove the deprecated filter into patient registry Aug 12, 2017
@DedrickEnc DedrickEnc force-pushed the filterDeprecated branch 4 times, most recently from d44ddb4 to fed4ff6 Compare August 14, 2017 15:10
@DedrickEnc
Copy link
Contributor Author

@jniles
The test passed!

@jniles
Copy link
Collaborator

jniles commented Aug 14, 2017

Looks good to me.

@jniles
Copy link
Collaborator

jniles commented Aug 14, 2017

bors r+

bors bot added a commit that referenced this pull request Aug 14, 2017
1969: Remove the deprecated filter into patient registry r=jniles

In this PR : 

- The deprecated filter is removed from the patient registry file,

- The search modal was also updated to follow the new standard

- A new component was implemented ```bhPatientGroupSelect``` and its test

This PR references #1942
@bors
Copy link
Contributor

bors bot commented Aug 14, 2017

Build succeeded

@bors bors bot merged commit ad90506 into Third-Culture-Software:master Aug 14, 2017
@DedrickEnc DedrickEnc deleted the filterDeprecated branch August 18, 2017 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants