Skip to content

Commit

Permalink
Refactor endpoint update case status (#128)
Browse files Browse the repository at this point in the history
* Refactored analysis/mark_ready endpoint to handle all event types and update case status. Paired with Angelina on this.

* Removed console logs.

* Updated unit tests

* Paired with Angelina to fix failing unit test

* Linted backend files

* Formatted backend files

* Updated frontend to show success notification for active status update

* Updated frontend tests to pass
  • Loading branch information
fatimarabab authored Aug 21, 2023
1 parent ae3dde9 commit 42d9408
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 68 deletions.
2 changes: 1 addition & 1 deletion backend/src/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class EventType(str, Enum):


class StatusType(str, Enum):
"""Enumeration of the diferent statuses for an analyses"""
"""Enumeration of the different statuses for an analyses"""

ANNOTATION = "Annotation"
READY = "Ready"
Expand Down
10 changes: 4 additions & 6 deletions backend/src/repository/analysis_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from pymongo import ReturnDocument
from ..models.event import Event
from ..enums import EventType

# pylint: disable=too-few-public-methods
# Disabling too few public metods due to utilizing Pydantic/FastAPI BaseSettings class
Expand Down Expand Up @@ -358,15 +359,12 @@ def attach_third_party_link(self, analysis_name: str, third_party_enum: str, lin

return updated_document

def mark_ready(self, analysis_name: str, username: str):
""" Marks an analysis as ready """
def update_event(self, analysis_name: str, username: str, event_type: EventType):
""" Updates analysis status """
analysis = self.collection.find_one({"name": analysis_name})
if not analysis:
raise ValueError(f"Analysis with name {analysis_name} does not exist.")
for event in analysis['timeline']:
if event['event'] == 'ready':
raise ValueError(f"Analysis {analysis_name} is already marked as ready!")
analysis['timeline'].append(Event.timestamp_ready_event(username).dict())
analysis['timeline'].append(Event.timestamp_event(username, event_type).dict())

updated_document = self.collection.find_one_and_update(
{"name": analysis_name},
Expand Down
14 changes: 8 additions & 6 deletions backend/src/routers/analysis_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from ..dependencies import database, annotation_queue
from ..models.analysis import Analysis, AnalysisSummary
from ..models.event import Event
from ..enums import ThirdPartyLinkType
from ..enums import ThirdPartyLinkType, EventType
from ..models.phenotips_json import BasePhenotips
from ..models.user import VerifyUser
from ..security.security import get_authorization, get_current_user
Expand Down Expand Up @@ -91,16 +91,18 @@ async def create_file(
return new_analysis


@router.put("/{analysis_name}/mark_ready", response_model=Analysis)
def mark_ready(
@router.put("/{analysis_name}/event/{event_type}", response_model=Analysis)
def update_event(
analysis_name: str,
event_type: EventType,
repositories=Depends(database),
username: VerifyUser = Security(get_current_user),
authorized=Security(get_authorization, scopes=["write"]) #pylint: disable=unused-argument
authorized=Security(get_authorization, scopes=["write"]), #pylint: disable=unused-argument
):
""" Marks an analysis as ready for review """
""" Updates analysis status """

try:
return repositories["analysis"].mark_ready(analysis_name, username)
return repositories["analysis"].update_event(analysis_name, username, event_type)
except ValueError as exception:
raise HTTPException(status_code=409, detail=str(exception)) from exception

Expand Down
30 changes: 3 additions & 27 deletions backend/tests/integration/test_analysis_routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ def test_attach_third_party_link_invalid_enum(client, mock_access_token, mock_re


def test_mark_ready(client, mock_access_token, mock_repositories):
""" Testing the mark ready endpoint """
""" Testing the update analysis event endpoint """
staging_analysis_timeline = read_test_fixture("analysis-CPAM0002.json")
staging_analysis_timeline["timeline"] = [{
'event': 'create',
Expand All @@ -376,7 +376,7 @@ def test_mark_ready(client, mock_access_token, mock_repositories):

mock_repositories["analysis"].collection.find_one_and_update.return_value = expected

response = client.put("/analysis/CPAM0002/mark_ready", headers={"Authorization": "Bearer " + mock_access_token})
response = client.put("/analysis/CPAM0002/event/ready", headers={"Authorization": "Bearer " + mock_access_token})

assert response.status_code == 200

Expand All @@ -385,36 +385,12 @@ def test_mark_ready_analysis_does_not_exist(client, mock_access_token, mock_repo
""" Testing the mark ready endpoint """
mock_repositories["analysis"].collection.find_one.return_value = None

response = client.put("/analysis/CPAM2222/mark_ready", headers={"Authorization": "Bearer " + mock_access_token})
response = client.put("/analysis/CPAM2222/event/ready", headers={"Authorization": "Bearer " + mock_access_token})

assert response.status_code == 409
assert response.json() == {'detail': 'Analysis with name CPAM2222 does not exist.'}


def test_mark_ready_analysis_already_ready(client, mock_access_token, mock_repositories):
""" Testing the mark ready endpoint """
staging_analysis_timeline = read_test_fixture("analysis-CPAM0002.json")
staging_analysis_timeline["timeline"] = [
{
'event': 'create',
'timestamp': '2022-11-10T16:52:43.910000',
'username': 'johndoe',
},
{
'event': 'ready',
'timestamp': '2022-11-10T16:52:52.301003',
'username': 'johndoe',
},
]

mock_repositories["analysis"].collection.find_one.return_value = staging_analysis_timeline

response = client.put("/analysis/CPAM0002/mark_ready", headers={"Authorization": "Bearer " + mock_access_token})

assert response.status_code == 409
assert response.json() == {'detail': 'Analysis CPAM0002 is already marked as ready!'}


@pytest.fixture(name="analysis_updates_json")
def fixture_analysis_updates_json():
"""The JSON that is being sent from a client to the endpoint with updates in it"""
Expand Down
24 changes: 4 additions & 20 deletions backend/tests/unit/repository/test_analysis_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ def test_mark_ready(analysis_collection, create_timestamp, ready_timestamp):
}]
analysis_collection.collection.find_one.return_value = staging_analysis_timeline
with patch(
"src.models.event.Event.timestamp_ready_event",
"src.models.event.Event.timestamp_event",
return_value=Event(
**{
'event': EventType.READY,
Expand All @@ -298,9 +298,8 @@ def test_mark_ready(analysis_collection, create_timestamp, ready_timestamp):
}
)
):

analysis_collection.mark_ready("CPAM0002", "user01")

analysis_collection.update_event("CPAM0002", "user01", EventType.READY)
print(ready_timestamp)
analysis_collection.collection.find_one_and_update.assert_called_with(
{"name": "CPAM0002"},
{
Expand All @@ -319,27 +318,12 @@ def test_mark_ready_analysis_does_not_exist(analysis_collection):
"""Tests the mark_ready function returns an error if the analysis does not exist"""
analysis_collection.collection.find_one.return_value = None
try:
analysis_collection.mark_ready("CPAM2222", "user01")
analysis_collection.update_event("CPAM2222", "user01", EventType.READY)
except ValueError as error:
assert isinstance(error, ValueError)
assert str(error) == "Analysis with name CPAM2222 does not exist."


def test_mark_ready_already_marked_ready(analysis_collection, create_timestamp, ready_timestamp):
""""tests the mark_ready function returns an error if the analysis is already marked ready"""
staging_analysis_timeline = read_test_fixture("analysis-CPAM0002.json")
staging_analysis_timeline["timeline"] = [
{'event': 'create', 'timestamp': create_timestamp, 'username': 'user01'},
{'event': 'ready', 'timestamp': ready_timestamp, 'username': 'user01'},
]
analysis_collection.collection.find_one.return_value = staging_analysis_timeline
try:
analysis_collection.mark_ready("CPAM0002", "user01")
except ValueError as error:
assert isinstance(error, ValueError)
assert str(error) == "Analysis CPAM0002 is already marked as ready!"


@pytest.fixture(name="analysis_with_no_p_dot")
def fixture_analysis_with_no_p_dot():
"""Returns an analysis with no p. in the genomic unit"""
Expand Down
8 changes: 3 additions & 5 deletions frontend/src/models/analyses.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,13 @@ export default {
},

async markAnalysisReady(analysisName) {
const url = `/rosalution/api/analysis/${analysisName}/mark_ready`;
const url = `/rosalution/api/analysis/${analysisName}/event/ready`;
return await Requests.put(url);
},

async markAnalysisActive(analysisName) {
// const url = `/rosalution/api/analysis/${analysisName}/mark_active`;
// return await Requests.put(url);
console.warn('Mark analysis active does not exist yet');
// ** Not implemented yet **
const url = `/rosalution/api/analysis/${analysisName}/event/opened`;
return await Requests.put(url);
},

async getAnnotationConfiguration(analysisName) {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/AnalysisView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export default {
text: 'Mark Active',
operation: () => {
Analyses.markAnalysisActive(this.analysis_name);
toast.info('The Mark Ready feature is not yet implemented.');
toast.success('Analysis marked as Active.');
},
});
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/test/views/AnalysisView.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ describe('AnalysisView', () => {
await triggerAction(wrapper, 'Mark Active');

expect(toast.state.active).to.be.true;
expect(toast.state.type).to.equal('info');
expect(toast.state.message).to.equal('The Mark Ready feature is not yet implemented.');
expect(toast.state.type).to.equal('success');
expect(toast.state.message).to.equal('Analysis marked as Active.');
});

it('should display info toast with correct message when entering edit mode', async () => {
Expand Down

0 comments on commit 42d9408

Please sign in to comment.