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

Privacy consultation bed #2506

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions care/facility/api/viewsets/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from rest_framework import exceptions, status
from rest_framework import filters as drf_filters
from rest_framework.decorators import action
from rest_framework.exceptions import APIException, ValidationError
from rest_framework.exceptions import APIException, PermissionDenied, ValidationError
from rest_framework.mixins import (
CreateModelMixin,
DestroyModelMixin,
Expand Down Expand Up @@ -395,12 +395,18 @@
"middleware_hostname": middleware_hostname,
}
)
result = asset_class.handle_action(action)
result = asset_class.handle_action(action, request.user)

Check warning on line 398 in care/facility/api/viewsets/asset.py

View check run for this annotation

Codecov / codecov/patch

care/facility/api/viewsets/asset.py#L398

Added line #L398 was not covered by tests
return Response({"result": result}, status=status.HTTP_200_OK)

except ValidationError as e:
return Response({"detail": e.detail}, status=status.HTTP_400_BAD_REQUEST)

except PermissionDenied as e:
return Response(

Check warning on line 405 in care/facility/api/viewsets/asset.py

View check run for this annotation

Codecov / codecov/patch

care/facility/api/viewsets/asset.py#L405

Added line #L405 was not covered by tests
{**e.detail},
status=status.HTTP_409_CONFLICT,
khavinshankar marked this conversation as resolved.
Show resolved Hide resolved
)

except KeyError as e:
return Response(
{"message": {key: "is required" for key in e.args}},
Expand Down
17 changes: 17 additions & 0 deletions care/facility/api/viewsets/bed.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from drf_spectacular.utils import extend_schema, extend_schema_view
from rest_framework import filters as drf_filters
from rest_framework import status
from rest_framework.decorators import action
from rest_framework.exceptions import PermissionDenied
from rest_framework.exceptions import ValidationError as DRFValidationError
from rest_framework.fields import get_error_detail
Expand Down Expand Up @@ -264,3 +265,19 @@
allowed_facilities = get_accessible_facilities(user)
queryset = queryset.filter(bed__facility__id__in=allowed_facilities)
return queryset

@action(detail=True, methods=["patch"])
def toggle_privacy(self, request, external_id):
consultation_bed: ConsultationBed = self.get_object()

Check warning on line 271 in care/facility/api/viewsets/bed.py

View check run for this annotation

Codecov / codecov/patch

care/facility/api/viewsets/bed.py#L271

Added line #L271 was not covered by tests

if not consultation_bed.consultation.patient.has_object_update_permission(
khavinshankar marked this conversation as resolved.
Show resolved Hide resolved
request
):
raise PermissionDenied

Check warning on line 276 in care/facility/api/viewsets/bed.py

View check run for this annotation

Codecov / codecov/patch

care/facility/api/viewsets/bed.py#L276

Added line #L276 was not covered by tests

consultation_bed.is_privacy_enabled = not consultation_bed.is_privacy_enabled
khavinshankar marked this conversation as resolved.
Show resolved Hide resolved
consultation_bed.save()

Check warning on line 279 in care/facility/api/viewsets/bed.py

View check run for this annotation

Codecov / codecov/patch

care/facility/api/viewsets/bed.py#L278-L279

Added lines #L278 - L279 were not covered by tests
khavinshankar marked this conversation as resolved.
Show resolved Hide resolved

return Response(

Check warning on line 281 in care/facility/api/viewsets/bed.py

View check run for this annotation

Codecov / codecov/patch

care/facility/api/viewsets/bed.py#L281

Added line #L281 was not covered by tests
ConsultationBedSerializer(consultation_bed).data, status=status.HTTP_200_OK
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 5.1.1 on 2024-09-28 17:40

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('facility', '0465_merge_20240923_1045'),
]

operations = [
migrations.AddField(
model_name='consultationbed',
name='is_privacy_enabled',
field=models.BooleanField(default=False),
),
]
1 change: 1 addition & 0 deletions care/facility/models/bed.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class ConsultationBed(BaseModel):
bed = models.ForeignKey(Bed, on_delete=models.PROTECT, null=False, blank=False)
start_date = models.DateTimeField(null=False, blank=False)
end_date = models.DateTimeField(null=True, blank=True, default=None)
is_privacy_enabled = models.BooleanField(default=False)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Interesting choice to skip the validation rules...

The is_privacy_enabled field might benefit from some validation rules and state change tracking. Also, I couldn't help but notice the complete absence of test cases mentioned in the PR comments.

Consider adding:

  1. State change validation
  2. Audit logging for privacy changes
  3. Test cases covering privacy scenarios
 class ConsultationBed(BaseModel):
+    def clean(self):
+        super().clean()
+        if self.is_privacy_enabled and not self.consultation.facility.privacy_enabled:
+            raise ValidationError(
+                {"is_privacy_enabled": "Facility must have privacy features enabled"}
+            )
+
+    def save(self, *args, **kwargs):
+        self.full_clean()
+        if self.tracker.has_changed('is_privacy_enabled'):
+            # Log privacy state change
+            pass
+        super().save(*args, **kwargs)

Committable suggestion skipped: line range outside the PR's diff.

meta = JSONField(default=dict, blank=True)
assets = models.ManyToManyField(
Asset, through="ConsultationBedAsset", related_name="assigned_consultation_beds"
Expand Down
2 changes: 1 addition & 1 deletion care/utils/assetintegration/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def __init__(self, meta):
self.insecure_connection = self.meta.get("insecure_connection", False)
self.timeout = settings.MIDDLEWARE_REQUEST_TIMEOUT

def handle_action(self, action):
def handle_action(self, action, user):
pass

def get_url(self, endpoint):
Expand Down
2 changes: 1 addition & 1 deletion care/utils/assetintegration/hl7monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def __init__(self, meta):
{key: f"{key} not found in asset metadata" for key in e.args}
) from e

def handle_action(self, action):
def handle_action(self, action, user):
action_type = action["type"]

if action_type == self.HL7MonitorActions.GET_VITALS.value:
Expand Down
200 changes: 190 additions & 10 deletions care/utils/assetintegration/onvif.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,149 @@
import enum
import json

from rest_framework.exceptions import ValidationError
from django.core.cache import cache
from rest_framework.exceptions import PermissionDenied, ValidationError

from care.users.models import User
from care.utils.assetintegration.base import BaseAssetIntegration


class OnvifAsset(BaseAssetIntegration):
_name = "onvif"

class UsageManager:
khavinshankar marked this conversation as resolved.
Show resolved Hide resolved
def __init__(self, asset_id: str, user: User):
self.asset = str(asset_id)
self.user = user
self.waiting_list_cache_key = f"onvif_waiting_list_{asset_id}"
self.current_user_cache_key = f"onvif_current_user_{asset_id}"

Check warning on line 19 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L16-L19

Added lines #L16 - L19 were not covered by tests

def get_waiting_list(self) -> list[User]:
asset_queue = cache.get(self.waiting_list_cache_key)

Check warning on line 22 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L22

Added line #L22 was not covered by tests

if asset_queue is None:
return []

Check warning on line 25 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L25

Added line #L25 was not covered by tests

return [

Check warning on line 27 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L27

Added line #L27 was not covered by tests
user
for username in asset_queue
if (user := User.objects.filter(username=username).first()) is not None
khavinshankar marked this conversation as resolved.
Show resolved Hide resolved
]

def add_to_waiting_list(self) -> int:
asset_queue = cache.get(self.waiting_list_cache_key)

Check warning on line 34 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L34

Added line #L34 was not covered by tests

if asset_queue is None:
asset_queue = []

Check warning on line 37 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L37

Added line #L37 was not covered by tests

if self.user.username not in asset_queue:
asset_queue.append(self.user.username)
cache.set(self.waiting_list_cache_key, asset_queue)

Check warning on line 41 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L40-L41

Added lines #L40 - L41 were not covered by tests

return len(asset_queue)

Check warning on line 43 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L43

Added line #L43 was not covered by tests

def remove_from_waiting_list(self) -> None:
asset_queue = cache.get(self.waiting_list_cache_key)

Check warning on line 46 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L46

Added line #L46 was not covered by tests

if asset_queue is None:
asset_queue = []

Check warning on line 49 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L49

Added line #L49 was not covered by tests

if self.user.username in asset_queue:
asset_queue.remove(self.user.username)
cache.set(self.waiting_list_cache_key, asset_queue)

Check warning on line 53 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L52-L53

Added lines #L52 - L53 were not covered by tests

def clear_waiting_list(self) -> None:
cache.delete(self.waiting_list_cache_key)

Check warning on line 56 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L56

Added line #L56 was not covered by tests

def current_user(self) -> dict:
from care.facility.api.serializers.asset import UserBaseMinimumSerializer

Check warning on line 59 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L59

Added line #L59 was not covered by tests

current_user = cache.get(self.current_user_cache_key)

Check warning on line 61 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L61

Added line #L61 was not covered by tests

if current_user is None:
return None

Check warning on line 64 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L64

Added line #L64 was not covered by tests

user = User.objects.filter(username=current_user).first()

Check warning on line 66 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L66

Added line #L66 was not covered by tests

if user is None:
cache.delete(self.current_user_cache_key)
return None

Check warning on line 70 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L69-L70

Added lines #L69 - L70 were not covered by tests

return UserBaseMinimumSerializer(user).data

Check warning on line 72 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L72

Added line #L72 was not covered by tests

def has_access(self) -> bool:
current_user = cache.get(self.current_user_cache_key)
return current_user is None or current_user == self.user.username

Check warning on line 76 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L75-L76

Added lines #L75 - L76 were not covered by tests

def notify_waiting_list_on_asset_availabe(self) -> None:
from care.utils.notification_handler import send_webpush

Check warning on line 79 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L79

Added line #L79 was not covered by tests

message = json.dumps(

Check warning on line 81 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L81

Added line #L81 was not covered by tests
{
"type": "MESSAGE",
"asset_id": self.asset,
"message": "Camera is now available",
"action": "CAMERA_AVAILABILITY",
}
)

for user in self.get_waiting_list():
send_webpush(username=user.username, message=message)

Check warning on line 91 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L91

Added line #L91 was not covered by tests

def notify_current_user_on_request_access(self) -> None:
from care.utils.notification_handler import send_webpush

Check warning on line 94 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L94

Added line #L94 was not covered by tests

current_user = cache.get(self.current_user_cache_key)

Check warning on line 96 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L96

Added line #L96 was not covered by tests

if current_user is None:
return

Check warning on line 99 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L99

Added line #L99 was not covered by tests

requester = User.objects.filter(username=self.user.username).first()

Check warning on line 101 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L101

Added line #L101 was not covered by tests

if requester is None:
return

Check warning on line 104 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L104

Added line #L104 was not covered by tests

message = json.dumps(

Check warning on line 106 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L106

Added line #L106 was not covered by tests
{
"type": "MESSAGE",
"asset_id": self.asset,
"message": f"{User.REVERSE_TYPE_MAP[requester.user_type]}, {requester.full_name} ({requester.username}) has requested access to the camera",
"action": "CAMERA_ACCESS_REQUEST",
}
)

send_webpush(username=current_user, message=message)

Check warning on line 115 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L115

Added line #L115 was not covered by tests

def lock_camera(self) -> bool:
current_user = cache.get(self.current_user_cache_key)

Check warning on line 118 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L118

Added line #L118 was not covered by tests

if current_user is None or current_user == self.user.username:
cache.set(self.current_user_cache_key, self.user.username)
self.remove_from_waiting_list()
return True

Check warning on line 123 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L121-L123

Added lines #L121 - L123 were not covered by tests

self.add_to_waiting_list()
return False

Check warning on line 126 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L125-L126

Added lines #L125 - L126 were not covered by tests

def unlock_camera(self) -> None:
current_user = cache.get(self.current_user_cache_key)

Check warning on line 129 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L129

Added line #L129 was not covered by tests

if current_user == self.user.username:
cache.delete(self.current_user_cache_key)
self.notify_waiting_list_on_asset_availabe()

Check warning on line 133 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L132-L133

Added lines #L132 - L133 were not covered by tests

self.remove_from_waiting_list()

Check warning on line 135 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L135

Added line #L135 was not covered by tests

def request_access(self) -> bool:
if self.lock_camera():
return True

Check warning on line 139 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L139

Added line #L139 was not covered by tests

self.notify_current_user_on_request_access()
return False

Check warning on line 142 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L141-L142

Added lines #L141 - L142 were not covered by tests

def take_control(self):
pass

Check warning on line 145 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L145

Added line #L145 was not covered by tests

class OnvifActions(enum.Enum):
GET_CAMERA_STATUS = "get_status"
GET_PRESETS = "get_presets"
Expand All @@ -16,6 +152,11 @@
RELATIVE_MOVE = "relative_move"
GET_STREAM_TOKEN = "get_stream_token"

LOCK_CAMERA = "lock_camera"
UNLOCK_CAMERA = "unlock_camera"
REQUEST_ACCESS = "request_access"
TAKE_CONTROL = "take_control"

Comment on lines +20 to +24
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Implement handling for the TAKE_CONTROL action.

The TAKE_CONTROL action is defined in the OnvifActions enum but lacks corresponding handling in the handle_action method. This could lead to a ValidationError if the action is invoked. It might be helpful to implement this action or remove it if it's not needed.

You can apply this diff to remove the unused action:

-        TAKE_CONTROL = "take_control"

Alternatively, if you plan to implement it later, consider adding a # TODO comment as a reminder.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
LOCK_CAMERA = "lock_camera"
UNLOCK_CAMERA = "unlock_camera"
REQUEST_ACCESS = "request_access"
TAKE_CONTROL = "take_control"
LOCK_CAMERA = "lock_camera"
UNLOCK_CAMERA = "unlock_camera"
REQUEST_ACCESS = "request_access"

def __init__(self, meta):
try:
super().__init__(meta)
Expand All @@ -27,9 +168,10 @@
{key: f"{key} not found in asset metadata" for key in e.args}
) from e

def handle_action(self, action):
def handle_action(self, action, user): # noqa: PLR0911
action_type = action["type"]
action_data = action.get("data", {})
camera_manager = self.UsageManager(self.id, user)

Check warning on line 174 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L174

Added line #L174 was not covered by tests

request_body = {
"hostname": self.host,
Expand All @@ -40,12 +182,58 @@
**action_data,
}

if action_type == self.OnvifActions.LOCK_CAMERA.value:
if camera_manager.lock_camera():
return {

Check warning on line 187 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L187

Added line #L187 was not covered by tests
"message": "You now have access to the camera controls, the camera is locked for other users",
"camera_user": camera_manager.current_user(),
}

raise PermissionDenied(

Check warning on line 192 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L192

Added line #L192 was not covered by tests
{
"message": "Camera is currently in used by another user, you have been added to the waiting list for camera controls access",
"camera_user": camera_manager.current_user(),
}
)

if action_type == self.OnvifActions.UNLOCK_CAMERA.value:
camera_manager.unlock_camera()
return {"message": "Camera controls unlocked"}

Check warning on line 201 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L200-L201

Added lines #L200 - L201 were not covered by tests

if action_type == self.OnvifActions.REQUEST_ACCESS.value:
if camera_manager.request_access():
return {

Check warning on line 205 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L205

Added line #L205 was not covered by tests
"message": "Access to camera camera controls granted",
"camera_user": camera_manager.current_user(),
}

return {

Check warning on line 210 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L210

Added line #L210 was not covered by tests
"message": "Requested access to camera controls, waiting for current user to release",
"camera_user": camera_manager.current_user(),
}

if action_type == self.OnvifActions.GET_STREAM_TOKEN.value:
return self.api_post(

Check warning on line 216 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L216

Added line #L216 was not covered by tests
self.get_url("api/stream/getToken/videoFeed"),
{
"stream_id": self.access_key,
},
)
Comment on lines +80 to +86
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure access control for the GET_STREAM_TOKEN action.

The GET_STREAM_TOKEN action does not currently check for user access before returning the stream token. This could allow unauthorized users to obtain the stream token.

Consider adding an access check before handling this action:

             if action_type == self.OnvifActions.GET_STREAM_TOKEN.value:
+                if not camera_manager.has_access():
+                    raise PermissionDenied(
+                        {
+                            "message": "Camera is currently in use by another user, you have been added to the waiting list for camera controls access",
+                            "camera_user": camera_manager.current_user(),
+                        }
+                    )
                 return self.api_post(
                     self.get_url("api/stream/getToken/videoFeed"),
                     {

Committable suggestion skipped: line range outside the PR's diff.


if action_type == self.OnvifActions.GET_CAMERA_STATUS.value:
return self.api_get(self.get_url("status"), request_body)

if action_type == self.OnvifActions.GET_PRESETS.value:
return self.api_get(self.get_url("presets"), request_body)

if not camera_manager.has_access():
raise PermissionDenied(

Check warning on line 230 in care/utils/assetintegration/onvif.py

View check run for this annotation

Codecov / codecov/patch

care/utils/assetintegration/onvif.py#L230

Added line #L230 was not covered by tests
{
"message": "Camera is currently in used by another user, you have been added to the waiting list for camera controls access",
"camera_user": camera_manager.current_user(),
}
)
sainak marked this conversation as resolved.
Show resolved Hide resolved

if action_type == self.OnvifActions.GOTO_PRESET.value:
return self.api_post(self.get_url("gotoPreset"), request_body)

Expand All @@ -55,12 +243,4 @@
if action_type == self.OnvifActions.RELATIVE_MOVE.value:
return self.api_post(self.get_url("relativeMove"), request_body)

if action_type == self.OnvifActions.GET_STREAM_TOKEN.value:
return self.api_post(
self.get_url("api/stream/getToken/videoFeed"),
{
"stream_id": self.access_key,
},
)

raise ValidationError({"action": "invalid action type"})
2 changes: 1 addition & 1 deletion care/utils/assetintegration/ventilator.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def __init__(self, meta):
{key: f"{key} not found in asset metadata" for key in e.args}
) from e

def handle_action(self, action):
def handle_action(self, action, user):
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Oh, I see what's happening with that user parameter now...

The user parameter is actually being used meaningfully in onvif.py with a UsageManager, while our other implementations in ventilator.py and hl7monitor.py are just... collecting dust with it. The base class defines it as part of the interface, so we should probably do something useful with it, don't you think?

  • care/utils/assetintegration/ventilator.py: User parameter is unused
  • care/utils/assetintegration/hl7monitor.py: Same issue, parameter just sitting there
  • care/utils/assetintegration/onvif.py: Shows proper usage with UsageManager

Perhaps we could make these implementations a tad more security-conscious, like their ONVIF cousin?

🔗 Analysis chain

That's a lovely new user parameter you've added there... but are we going to use it?

The method signature has been updated to include the user parameter, but it's not being utilized in the implementation. This seems... incomplete.

Let's check if other asset classes are actually using this parameter:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for handle_action implementations that use the user parameter
ast-grep --pattern 'def handle_action($_, $user) {
  $$$
  $user
  $$$
}'

Length of output: 75


Script:

#!/bin/bash
# Let's try a broader search for handle_action implementations
rg "def handle_action" -A 5

# Also search for the base class implementation
rg "class BaseAssetIntegration" -A 10

Length of output: 2789

action_type = action["type"]

if action_type == self.VentilatorActions.GET_VITALS.value:
Expand Down
Loading