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

Implement Middleware Override in AssetLocation #1642

Merged
merged 6 commits into from
Oct 16, 2023
Merged

Conversation

Ashesh3
Copy link
Member

@Ashesh3 Ashesh3 commented Sep 26, 2023

This PR addresses issue #6310, which identified the need for a middleware override feature in the location management page. This feature is crucial for integrating multiple ICUs, each running on different servers with various teleicu middleware, under a single facility on the care platform.

Previously, the lack of this feature necessitated the redundant duplication of facilities for middleware configuration and deployment on care. With the implementation of the middleware override feature, we can now efficiently manage multiple servers in a single facility with multiple ICUs.

The changes in this PR include:

  1. The addition of the middleware override feature in the location management page.
  2. Updated logic to handle multiple servers under a single facility.
  3. Enhanced facility management efficiency by eliminating the need for repetitive facility duplication.

This feature enhances the overall usability and efficiency of the care platform, especially for larger institutions like hospitals with multiple departments.

@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (2829067) 59.11% compared to head (7b28d79) 59.09%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1642      +/-   ##
==========================================
- Coverage   59.11%   59.09%   -0.03%     
==========================================
  Files         207      207              
  Lines       11264    11278      +14     
  Branches     1599     1602       +3     
==========================================
+ Hits         6659     6665       +6     
- Misses       4405     4412       +7     
- Partials      200      201       +1     
Files Coverage Δ
care/facility/models/asset.py 95.23% <100.00%> (+0.04%) ⬆️
care/facility/api/viewsets/asset.py 73.09% <0.00%> (-0.30%) ⬇️
care/facility/api/serializers/asset.py 84.30% <71.42%> (-0.55%) ⬇️
care/facility/api/serializers/facility.py 80.00% <0.00%> (-2.06%) ⬇️
care/facility/tasks/asset_monitor.py 19.04% <0.00%> (-0.96%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nihal467
Copy link
Member

nihal467 commented Oct 8, 2023

@Ashesh3 does the dummy data facility disappear after the middleware was fixed, which we discussed during the huddle

@Ashesh3
Copy link
Member Author

Ashesh3 commented Oct 9, 2023

@Ashesh3 does the dummy data facility disappear after the middleware was fixed, which we discussed during the huddle

Issue was with dummy data itself; PR is fine. Will make a new PR to fix the dummy data. No changes are needed on the PR.

@nihal467
Copy link
Member

nihal467 commented Oct 9, 2023

LGTM

"middleware_hostname",
asset.current_location.facility.middleware_address,
hostname = (
asset.meta.get(
Copy link
Member

Choose a reason for hiding this comment

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

Maybe move this to a common function in the future? too much clutter now.

@vigneshhari vigneshhari merged commit 0794d08 into master Oct 16, 2023
9 of 11 checks passed
@vigneshhari vigneshhari deleted the location-middleware branch October 16, 2023 13:03
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