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

Renal handover #738

Merged
merged 23 commits into from
Aug 1, 2019
Merged

Renal handover #738

merged 23 commits into from
Aug 1, 2019

Conversation

fredkingham
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Jul 19, 2019

Coverage Status

Coverage increased (+0.3%) to 77.012% when pulling 69fd2ce on renal-handover into cd8e509 on v0.13.

@@ -31,10 +31,14 @@ <h1 >
{% endfor %}
</ul>
</div>
<a ng-show="currentTag === 'renal'" href="/letters/#{% url 'renal_handover' %}" target="_blank" class="btn btn-secondary pull-left left-offset-10">
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why this has a hard coded href component and a url tag ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a consequence of the back and forward of the letters plugin, it is a bit weird...

The theory is that we want it rendered within the angular context to give us access to our angular filters e.g. markdown. I'd rather not accidentally have 2 different markdown renderers. Also the copy and paste directive.

I agree though its a bit awks!

@@ -18,6 +18,11 @@
url(r'stories/$', views.TemplateView.as_view(template_name='stories.html')),
url(r'elcid/v0.1/', include(api.elcid_router.urls)),
url(r'labtest/v0.1/', include(api.lab_test_router.urls)),
url(
r'^elcid/renal_handover',
Copy link
Member

Choose a reason for hiding this comment

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

We're prefixing urls with the name of the application now ? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed this is to namespace the views in terms of the service that's using them.
Definitely true that using elcid as a sometime synonym for infection_service is not idea.

expected, sorted(wards, key=views.ward_sort_key)
)

def test_grouping(self):
Copy link
Member

Choose a reason for hiding this comment

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

What is this actually testing ? Can we have a comment / docstring / method name ?

patient here is...
their name
their hospital number
their unit/ward
Copy link
Member

Choose a reason for hiding this comment

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

This is actually 'their ward/bed' - stealing the terminology makes the docstring confusing...

elcid/views.py Outdated
patient__episode=episode
).get()
microbiology_inputs = models.MicrobiologyInput.objects.filter(
episode__patient__episode=episode
Copy link
Member

Choose a reason for hiding this comment

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

Is this definitely right ?
Does it not then put us back in the same episodic advice situation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we do give me inner join on patient then inner join on the episode table again which is basically expected, but as discussed, let just get the patient and that can save lots of joins.

SELECT "elcid_microbiologyinput"."id", "elcid_microbiologyinput"."created", "elcid_microbiologyinput"."updated", "elcid_microbiologyinput"."created_by_id", "elcid_microbiologyinput"."updated_by_id", "elcid_microbiologyinput"."consistency_token", "elcid_microbiologyinput"."episode_id", "elcid_microbiologyinput"."when", "elcid_microbiologyinput"."initials", "elcid_microbiologyinput"."infection_control", "elcid_microbiologyinput"."clinical_discussion", "elcid_microbiologyinput"."agreed_plan", "elcid_microbiologyinput"."discussed_with", "elcid_microbiologyinput"."clinical_advice_given", "elcid_microbiologyinput"."infection_control_advice_given", "elcid_microbiologyinput"."change_in_antibiotic_prescription", "elcid_microbiologyinput"."referred_to_opat", "elcid_microbiologyinput"."white_cell_count", "elcid_microbiologyinput"."c_reactive_protein", "elcid_microbiologyinput"."maximum_temperature", "elcid_microbiologyinput"."liver_function_fk_id", "elcid_microbiologyinput"."liver_function_ft", "elcid_microbiologyinput"."reason_for_interaction_fk_id", "elcid_microbiologyinput"."reason_for_interaction_ft", "elcid_microbiologyinput"."renal_function_fk_id", "elcid_microbiologyinput"."renal_function_ft" FROM 
"elcid_microbiologyinput" INNER JOIN "opal_episode" ON ("elcid_microbiologyinput"."episode_id" = "opal_episode"."id") INNER JOIN "opal_patient" ON ("opal_episode"."patient_id" = "opal_patient"."id") INNER JOIN "opal_episode" T4 ON ("opal_patient"."id" = T4."patient_id") WHERE T4."id" = 12 ORDER BY "elcid_microbiologyinput"."when" DESC; args=(12,)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we actually have episode.patient_id so we can remove the join with no actual cost. updated.

elcid/views.py Outdated Show resolved Hide resolved
elcid/views.py Outdated Show resolved Hide resolved
…can just look up clinical advice with the patient id to save on a join. Adds a comment to explain what we are doing with the clinical advice
@davidmiller davidmiller merged commit df443bf into v0.13 Aug 1, 2019
@davidmiller davidmiller deleted the renal-handover branch August 1, 2019 15:38
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

3 participants