Restructure Airlock requests & add createdBy field

CHANGELOG.md

@@ -1,6 +1,6 @@
 <!-- markdownlint-disable MD041 -->
 <!-- line format short be: change short description (#pr_numer) -->
-## 0.7.0 (Unreleased)
+## 0.6.1 (Unreleased)
 
 **BREAKING CHANGES & MIGRATIONS**:
 
@@ -9,8 +9,10 @@ FEATURES:
 * Automatically validate all resources have tre_id tag via TFLint [#2774](https://github.com/microsoft/AzureTRE/pull/2774)
 
 ENHANCEMENTS:
+* Renamed several airlock fields to make them more descriptive and added a createdBy field. Included migration for backwards compatibility ([#2779](https://github.com/microsoft/AzureTRE/pull/2779))
 
 BUG FIXES:
+* Show the correct createdBy value for airlock requests in UI and in API queries ([#2779](https://github.com/microsoft/AzureTRE/pull/2779))
 
 COMPONENTS:
 

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.5.4"
+__version__ = "0.5.5"

api_app/api/routes/airlock_resource_helpers.py

@@ -33,7 +33,7 @@ async def save_and_publish_event_airlock_request(airlock_request: AirlockRequest
 
     try:
         logging.debug(f"Saving airlock request item: {airlock_request.id}")
-        airlock_request.user = user
+        airlock_request.updatedBy = user
         airlock_request.updatedWhen = get_timestamp()
         airlock_request_repo.save_item(airlock_request)
     except Exception as e:
@@ -53,7 +53,7 @@ async def save_and_publish_event_airlock_request(airlock_request: AirlockRequest
 async def update_and_publish_event_airlock_request(
         airlock_request: AirlockRequest,
         airlock_request_repo: AirlockRequestRepository,
-        user: User,
+        updated_by: User,
         workspace: Workspace,
         new_status: AirlockRequestStatus = None,
         request_files: List[AirlockFile] = None,
@@ -64,7 +64,7 @@ async def update_and_publish_event_airlock_request(
         logging.debug(f"Updating airlock request item: {airlock_request.id}")
         updated_airlock_request = airlock_request_repo.update_airlock_request(
             original_request=airlock_request,
-            user=user,
+            updated_by=updated_by,
             new_status=new_status,
             request_files=request_files,
             status_message=status_message,
@@ -109,7 +109,7 @@ def check_email_exists(role_assignment_details: defaultdict(list)):
 def get_airlock_requests_by_user_and_workspace(user: User, workspace: Workspace, airlock_request_repo: AirlockRequestRepository,
                                                creator_user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None,
                                                order_by: str = None, order_ascending=True) -> List[AirlockRequest]:
-    return airlock_request_repo.get_airlock_requests(workspace_id=workspace.id, user_id=creator_user_id, type=type, status=status,
+    return airlock_request_repo.get_airlock_requests(workspace_id=workspace.id, creator_user_id=creator_user_id, type=type, status=status,
                                                      order_by=order_by, order_ascending=order_ascending)
 
 
@@ -136,7 +136,7 @@ def enrich_requests_with_allowed_actions(requests: List[AirlockRequest], user: U
     enriched_requests = []
     for request in requests:
         allowed_actions = get_allowed_actions(request, user, airlock_request_repo)
-        enriched_requests.append(AirlockRequestWithAllowedUserActions(airlockRequest=request, allowed_user_actions=allowed_actions))
+        enriched_requests.append(AirlockRequestWithAllowedUserActions(airlockRequest=request, allowedUserActions=allowed_actions))
     return enriched_requests
 
 

api_app/api/routes/migrations.py

@@ -1,6 +1,7 @@
 import logging
 
 from fastapi import APIRouter, Depends, HTTPException, status
+from db.migrations.airlock import AirlockMigration
 from db.migrations.resources import ResourceMigration
 from db.repositories.operations import OperationRepository
 from services.authentication import get_current_admin_user
@@ -24,7 +25,8 @@ async def migrate_database(resources_repo=Depends(get_repository(ResourceReposit
                            operations_repo=Depends(get_repository(OperationRepository)),
                            shared_services_migration=Depends(get_repository(SharedServiceMigration)),
                            workspace_migration=Depends(get_repository(WorkspaceMigration)),
-                           resource_migration=Depends(get_repository(ResourceMigration))):
+                           resource_migration=Depends(get_repository(ResourceMigration)),
+                           airlock_migration=Depends(get_repository(AirlockMigration))):
     try:
         migrations = list()
         logging.info("PR 1030")
@@ -55,6 +57,14 @@ async def migrate_database(resources_repo=Depends(get_repository(ResourceReposit
         shared_services_migration.checkMinFirewallVersion()
         migrations.append(Migration(issueNumber="2371", status='Firewall version meets requirement'))
 
+        logging.info("PR 2779 - Restructure Airlock requests & add createdBy field")
+        airlock_migration.rename_field_name('requestType', 'type')
+        airlock_migration.rename_field_name('requestTitle', 'title')
+        airlock_migration.rename_field_name('user', 'updatedBy')
+        airlock_migration.rename_field_name('creationTime', 'createdWhen')
+        num_updated = airlock_migration.add_created_by_and_rename_in_history()
+        migrations.append(Migration(issueNumber="2779", status=f'Renamed fields & updated {num_updated} airlock requests with createdBy'))
+
         return MigrationOutList(migrations=migrations)
     except Exception as e:
         logging.error("Failed to migrate database: %s", e, exc_info=True)

api_app/db/migrations/airlock.py

@@ -0,0 +1,33 @@
+from azure.cosmos import CosmosClient
+from db.repositories.airlock_requests import AirlockRequestRepository
+
+
+class AirlockMigration(AirlockRequestRepository):
+    def __init__(self, client: CosmosClient):
+        super().__init__(client)
+
+    def add_created_by_and_rename_in_history(self) -> int:
+        num_updated = 0
+        for request in self.container.read_all_items():
+            # Only migrate if createdBy isn't present
+            if 'createdBy' in request:
+                break
+
+            # For each request, check if it has history
+            if len(request['history']) > 0:
+                # createdBy value will be first user in history
+                request['createdBy'] = request['history'][0]['user']
+
+                # Also rename user to updatedBy in each history item
+                for item in request['history']:
+                    if 'user' in item:
+                        item['updatedBy'] = item['user']
+                        del item['user']
+            else:
+                # If not, the createdBy user will be the same as the updatedBy value
+                request['createdBy'] = request['updatedBy']
+
+            self.update_item_dict(request)
+            num_updated = num_updated + 1
+
+        return num_updated

api_app/db/repositories/airlock_requests.py

@@ -31,18 +31,18 @@ def get_resource_base_spec_params():
     def get_timestamp(self) -> float:
         return datetime.utcnow().timestamp()
 
-    def update_airlock_request_item(self, original_request: AirlockRequest, new_request: AirlockRequest, user: User, request_properties: dict) -> AirlockRequest:
+    def update_airlock_request_item(self, original_request: AirlockRequest, new_request: AirlockRequest, updated_by: User, request_properties: dict) -> AirlockRequest:
         history_item = AirlockRequestHistoryItem(
             resourceVersion=original_request.resourceVersion,
             updatedWhen=original_request.updatedWhen,
-            user=original_request.user,
+            updatedBy=original_request.updatedBy,
             properties=request_properties
         )
         new_request.history.append(history_item)
 
         # now update the request props
         new_request.resourceVersion = new_request.resourceVersion + 1
-        new_request.user = user
+        new_request.updatedBy = updated_by
         new_request.updatedWhen = self.get_timestamp()
 
         self.upsert_item_with_etag(new_request, new_request.etag)
@@ -85,42 +85,43 @@ def validate_status_update(self, current_status: AirlockRequestStatus, new_statu
 
         return approved_condition and rejected_condition and blocked_condition and (approved_in_progress_condition or rejected_in_progress_condition or blocking_in_progress_condition or draft_condition or submit_condition or in_review_condition or cancel_condition or failed_condition)
 
-    def create_airlock_request_item(self, airlock_request_input: AirlockRequestInCreate, workspace_id: str) -> AirlockRequest:
+    def create_airlock_request_item(self, airlock_request_input: AirlockRequestInCreate, workspace_id: str, user) -> AirlockRequest:
         full_airlock_request_id = str(uuid.uuid4())
 
         resource_spec_parameters = {**self.get_airlock_request_spec_params()}
 
         airlock_request = AirlockRequest(
             id=full_airlock_request_id,
             workspaceId=workspace_id,
-            requestTitle=airlock_request_input.requestTitle,
+            title=airlock_request_input.title,
             businessJustification=airlock_request_input.businessJustification,
-            requestType=airlock_request_input.requestType,
-            creationTime=datetime.utcnow().timestamp(),
+            type=airlock_request_input.type,
+            createdBy=user,
+            createdWhen=datetime.utcnow().timestamp(),
             properties=resource_spec_parameters,
             reviews=[]
         )
 
         return airlock_request
 
-    def get_airlock_requests(self, workspace_id: str, user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None, order_by: str = None, order_ascending=True) -> List[AirlockRequest]:
-        query = self.airlock_requests_query() + f' where c.workspaceId = "{workspace_id}"'
+    def get_airlock_requests(self, workspace_id: str, creator_user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None, order_by: str = None, order_ascending=True) -> List[AirlockRequest]:
+        query = self.airlock_requests_query() + f' WHERE c.workspaceId = "{workspace_id}"'
 
         # optional filters
-        if user_id:
-            query += ' AND c.user.id=@user_id'
+        if creator_user_id:
+            query += ' AND c.createdBy.id=@user_id'
         if status:
             query += ' AND c.status=@status'
         if type:
-            query += ' AND c.requestType=@type'
+            query += ' AND c.type=@type'
 
         # optional sorting
         if order_by:
             query += ' ORDER BY c.' + order_by
             query += ' ASC' if order_ascending else ' DESC'
 
         parameters = [
-            {"name": "@user_id", "value": user_id},
+            {"name": "@user_id", "value": creator_user_id},
             {"name": "@status", "value": status},
             {"name": "@type", "value": type},
         ]
@@ -137,7 +138,7 @@ def get_airlock_request_by_id(self, airlock_request_id: UUID4) -> AirlockRequest
     def update_airlock_request(
             self,
             original_request: AirlockRequest,
-            user: User,
+            updated_by: User,
             new_status: AirlockRequestStatus = None,
             request_files: List[AirlockFile] = None,
             status_message: str = None,
@@ -151,12 +152,12 @@ def update_airlock_request(
             airlock_review=airlock_review,
             review_user_resource=review_user_resource)
         try:
-            db_response = self.update_airlock_request_item(original_request, updated_request, user, {"previousStatus": original_request.status})
+            db_response = self.update_airlock_request_item(original_request, updated_request, updated_by, {"previousStatus": original_request.status})
         except CosmosAccessConditionFailedError:
             logging.warning(f"ETag mismatch for request ID: '{original_request.id}'. Retrying.")
             original_request = self.get_airlock_request_by_id(original_request.id)
             updated_request = self._build_updated_request(original_request=original_request, new_status=new_status, request_files=request_files, status_message=status_message, airlock_review=airlock_review)
-            db_response = self.update_airlock_request_item(original_request, updated_request, user, {"previousStatus": original_request.status})
+            db_response = self.update_airlock_request_item(original_request, updated_request, updated_by, {"previousStatus": original_request.status})
 
         return db_response
 

api_app/event_grid/event_sender.py

@@ -12,7 +12,7 @@ async def send_status_changed_event(airlock_request: AirlockRequest, previous_st
     request_id = airlock_request.id
     new_status = airlock_request.status.value
     previous_status = previous_status.value if previous_status else None
-    request_type = airlock_request.requestType.value
+    request_type = airlock_request.type.value
     short_workspace_id = airlock_request.workspaceId[-4:]
 
     status_changed_event = EventGridEvent(

api_app/models/domain/airlock_request.py

@@ -62,7 +62,7 @@ class AirlockRequestHistoryItem(AzureTREModel):
     """
     resourceVersion: int
     updatedWhen: float
-    user: dict = {}
+    updatedBy: dict = {}
     properties: dict = {}
 
 
@@ -81,16 +81,17 @@ class AirlockRequest(AzureTREModel):
     """
     id: str = Field(title="Id", description="GUID identifying the resource")
     resourceVersion: int = 0
-    user: dict = {}
+    createdBy: dict = {}
+    createdWhen: float = Field(None, title="Creation time of the request")
+    updatedBy: dict = {}
     updatedWhen: float = 0
     history: List[AirlockRequestHistoryItem] = []
     workspaceId: str = Field("", title="Workspace ID", description="Service target Workspace id")
-    requestType: AirlockRequestType = Field("", title="Airlock request type")
+    type: AirlockRequestType = Field("", title="Airlock request type")
     files: List[AirlockFile] = Field([], title="Files of the request")
-    requestTitle: str = Field("Airlock Request", title="Brief title for the request")
-    businessJustification: str = Field("Business Justifications", title="Explanation that will be provided to the request reviewer")
+    title: str = Field("Airlock Request", title="Brief title for the request")
+    businessJustification: str = Field("Business Justification", title="Explanation that will be provided to the request reviewer")
     status = AirlockRequestStatus.Draft
-    creationTime: float = Field(None, title="Creation time of the request")
     statusMessage: Optional[str] = Field(title="Optional - contains additional information about the current status.")
     reviews: Optional[List[AirlockReview]]
     etag: Optional[str] = Field(title="_etag", alias="_etag")

api_app/models/schemas/airlock_request.py

@@ -20,11 +20,15 @@ def get_sample_airlock_request(workspace_id: str, airlock_request_id: str) -> di
         "requestId": airlock_request_id,
         "workspaceId": workspace_id,
         "status": "draft",
-        "requestType": "import",
+        "type": "import",
         "files": [],
-        "requestTitle": "a request title",
+        "title": "a request title",
         "businessJustification": "some business justification",
-        "creationTime": datetime.utcnow().timestamp(),
+        "createdBy": {
+            "id": "a user id",
+            "name": "a user name"
+        },
+        "createdWhen": datetime.utcnow().timestamp(),
         "reviews": [
             get_sample_airlock_review("29990431-5451-40e7-a58a-02e2b7c3d7c8"),
             get_sample_airlock_review("02dc0f29-351a-43ec-87e7-3dd2b5177b7f")]
@@ -34,7 +38,7 @@ def get_sample_airlock_request(workspace_id: str, airlock_request_id: str) -> di
 def get_sample_airlock_request_with_allowed_user_actions(workspace_id: str) -> dict:
     return {
         "airlockRequest": get_sample_airlock_request(workspace_id, str(uuid.uuid4())),
-        "allowed_user_actions": [AirlockActions.Cancel, AirlockActions.Review, AirlockActions.Submit],
+        "allowedUserActions": [AirlockActions.Cancel, AirlockActions.Review, AirlockActions.Submit],
     }
 
 
@@ -64,7 +68,7 @@ class Config:
 
 class AirlockRequestWithAllowedUserActions(BaseModel):
     airlockRequest: AirlockRequest = Field([], title="Airlock Request")
-    allowed_user_actions: List[str] = Field([], title="actions that the requesting user can do on the request")
+    allowedUserActions: List[str] = Field([], title="actions that the requesting user can do on the request")
 
     class Config:
         schema_extra = {
@@ -78,7 +82,7 @@ class AirlockRequestWithAllowedUserActionsInList(BaseModel):
     class Config:
         schema_extra = {
             "example": {
-                "airlock_requests": [
+                "airlockRequests": [
                     get_sample_airlock_request_with_allowed_user_actions("933ad738-7265-4b5f-9eae-a1a62928772e"),
                     get_sample_airlock_request_with_allowed_user_actions("933ad738-7265-4b5f-9eae-a1a62928772e")
                 ]
@@ -87,16 +91,16 @@ class Config:
 
 
 class AirlockRequestInCreate(BaseModel):
-    requestType: AirlockRequestType = Field("", title="Airlock request type", description="Specifies if this is an import or an export request")
-    requestTitle: str = Field("Airlock Request", title="Brief title for the request")
+    type: AirlockRequestType = Field("", title="Airlock request type", description="Specifies if this is an import or an export request")
+    title: str = Field("Airlock Request", title="Brief title for the request")
     businessJustification: str = Field("Business Justifications", title="Explanation that will be provided to the request reviewer")
     properties: dict = Field({}, title="Airlock request parameters", description="Values for the parameters required by the Airlock request specification")
 
     class Config:
         schema_extra = {
             "example": {
-                "requestType": "import",
-                "requestTitle": "a request title",
+                "type": "import",
+                "title": "a request title",
                 "businessJustification": "some business justification"
             }
         }

api_app/service_bus/airlock_request_status_update.py

@@ -65,7 +65,7 @@ async def update_status_in_database(airlock_request_repo: AirlockRequestReposito
         if airlock_request.status == current_status:
             workspace = workspace_repo.get_workspace_by_id(airlock_request.workspaceId)
             # update to new status and send to event grid
-            await update_and_publish_event_airlock_request(airlock_request=airlock_request, airlock_request_repo=airlock_request_repo, user=airlock_request.user, workspace=workspace, new_status=new_status, request_files=request_files, status_message=status_message)
+            await update_and_publish_event_airlock_request(airlock_request=airlock_request, airlock_request_repo=airlock_request_repo, updated_by=airlock_request.updatedBy, workspace=workspace, new_status=new_status, request_files=request_files, status_message=status_message)
             result = True
         else:
             error_string = strings.STEP_RESULT_MESSAGE_STATUS_DOES_NOT_MATCH.format(airlock_request_id, current_status, airlock_request.status)

api_app/services/airlock.py

@@ -13,7 +13,7 @@
 def get_account_by_request(airlock_request: AirlockRequest, workspace: Workspace) -> str:
     tre_id = config.TRE_ID
     short_workspace_id = workspace.id[-4:]
-    if airlock_request.requestType == constants.IMPORT_TYPE:
+    if airlock_request.type == constants.IMPORT_TYPE:
         if airlock_request.status == AirlockRequestStatus.Draft:
             return constants.STORAGE_ACCOUNT_NAME_IMPORT_EXTERNAL.format(tre_id)
         elif airlock_request.status == AirlockRequestStatus.Submitted: