Skip to content

Commit 9f6a4e5

Browse files
Fix errors to be able to fetch soft deleted workflows that only exist in the DB (#1162)
* Add unit tests for fetching process with a soft deleted workflow * Change get_auth_callbacks to work with optional workflow param * Convert values to boolean for retryAllowed and resumeAllowed for graphql process user_permissions * Bumpversion to 4.6.2 --------- Co-authored-by: Peter Boers <[email protected]>
1 parent 08451d1 commit 9f6a4e5

File tree

7 files changed

+59
-9
lines changed

7 files changed

+59
-9
lines changed

.bumpversion.cfg

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[bumpversion]
2-
current_version = 4.6.1
2+
current_version = 4.6.2
33
commit = False
44
tag = False
55
parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)(rc(?P<build>\d+))?

orchestrator/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
"""This is the orchestrator workflow engine."""
1515

16-
__version__ = "4.6.1"
16+
__version__ = "4.6.2"
1717

1818

1919
from structlog import get_logger

orchestrator/api/api_v1/endpoints/processes.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ def get_steps_to_evaluate_for_rbac(pstat: ProcessStat) -> StepList:
101101
return StepList(past_steps >> first(remaining_steps))
102102

103103

104-
def get_auth_callbacks(steps: StepList, workflow: Workflow) -> tuple[Authorizer | None, Authorizer | None]:
104+
def get_auth_callbacks(steps: StepList, workflow: Workflow | None) -> tuple[Authorizer | None, Authorizer | None]:
105105
"""Iterate over workflow and prior steps to determine correct authorization callbacks for the current step.
106106
107107
It's safest to always iterate through the steps. We could track these callbacks statefully
@@ -112,6 +112,9 @@ def get_auth_callbacks(steps: StepList, workflow: Workflow) -> tuple[Authorizer
112112
- RESUME callback is explicit RESUME callback, else previous START/RESUME callback
113113
- RETRY callback is explicit RETRY, else explicit RESUME, else previous RETRY
114114
"""
115+
if not workflow:
116+
return None, None
117+
115118
# Default to workflow start callbacks
116119
auth_resume = workflow.authorize_callback
117120
# auth_retry defaults to the workflow start callback if not otherwise specified.

orchestrator/graphql/schemas/process.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,11 @@ async def user_permissions(self, info: OrchestratorInfo) -> FormUserPermissionsT
8686
oidc_user = await info.context.get_current_user
8787
workflow = get_workflow(self.workflow_name)
8888
process = load_process(db.session.get(ProcessTable, self.process_id)) # type: ignore[arg-type]
89-
auth_resume, auth_retry = get_auth_callbacks(get_steps_to_evaluate_for_rbac(process), workflow) # type: ignore[arg-type]
89+
auth_resume, auth_retry = get_auth_callbacks(get_steps_to_evaluate_for_rbac(process), workflow)
9090

9191
return FormUserPermissionsType(
92-
retryAllowed=auth_retry and auth_retry(oidc_user), # type: ignore[arg-type]
93-
resumeAllowed=auth_resume and auth_resume(oidc_user), # type: ignore[arg-type]
92+
retryAllowed=bool(auth_retry and auth_retry(oidc_user)),
93+
resumeAllowed=bool(auth_resume and auth_resume(oidc_user)),
9494
)
9595

9696
@authenticated_field(description="Returns list of subscriptions of the process") # type: ignore

test/unit_tests/conftest.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@
4242
from orchestrator.utils.json import json_dumps
4343
from orchestrator.utils.redis_client import create_redis_client
4444
from pydantic_forms.core import FormPage
45-
from test.unit_tests.fixtures.processes import mocked_processes, mocked_processes_resumeall, test_workflow # noqa: F401
45+
from test.unit_tests.fixtures.processes import ( # noqa: F401
46+
mocked_processes,
47+
mocked_processes_resumeall,
48+
test_workflow,
49+
test_workflow_soft_deleted,
50+
)
4651
from test.unit_tests.fixtures.products.product_blocks.product_block_list_nested import ( # noqa: F401
4752
test_product_block_list_nested,
4853
test_product_block_list_nested_db_in_use_by_block,

test/unit_tests/fixtures/processes.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55

66
import pytest
77
import pytz
8-
from sqlalchemy import delete
8+
from sqlalchemy import delete, select
99

1010
from orchestrator.config.assignee import Assignee
1111
from orchestrator.db import ProcessStepTable, ProcessSubscriptionTable, ProcessTable, db
12+
from orchestrator.db.models import WorkflowTable
1213
from orchestrator.workflow import done, init, inputstep, step, workflow
1314
from pydantic_forms.core import FormPage
1415
from pydantic_forms.types import FormGenerator, UUIDstr
@@ -53,6 +54,18 @@ def workflow_for_testing_processes_py():
5354
yield wf
5455

5556

57+
@pytest.fixture
58+
def test_workflow_soft_deleted(test_workflow) -> Generator:
59+
stmt = select(WorkflowTable).where(WorkflowTable.workflow_id == test_workflow.workflow_id)
60+
workflow = db.session.scalar(stmt)
61+
assert workflow
62+
workflow.deleted_at = datetime.now().tzinfo
63+
db.session.add(workflow)
64+
db.session.commit()
65+
66+
yield test_workflow
67+
68+
5669
@pytest.fixture
5770
def mocked_processes(test_workflow, generic_subscription_1, generic_subscription_2):
5871
first_datetime = datetime(2020, 1, 14, 9, 30, tzinfo=pytz.utc)

test/unit_tests/graphql/test_process.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,18 @@
1212
# limitations under the License.
1313
import json
1414
from http import HTTPStatus
15+
from unittest import mock
1516

1617

1718
def build_simple_query(process_id):
1819
q = """
1920
query ProcessQuery($processId: UUID!) {
2021
process(processId: $processId) {
2122
processId
23+
userPermissions {
24+
retryAllowed
25+
resumeAllowed
26+
}
2227
}
2328
}
2429
"""
@@ -39,4 +44,28 @@ def test_process(test_client, mocked_processes):
3944

4045
response = test_client.post("/api/graphql", content=test_query, headers={"Content-Type": "application/json"})
4146
assert response.status_code == HTTPStatus.OK
42-
assert response.json() == {"data": {"process": {"processId": str(process_id)}}}
47+
assert response.json() == {
48+
"data": {
49+
"process": {"processId": str(process_id), "userPermissions": {"resumeAllowed": True, "retryAllowed": True}}
50+
}
51+
}
52+
53+
54+
@mock.patch("orchestrator.graphql.schemas.process.get_workflow")
55+
def test_process_is_allowed_with_historic_workflow_only_left_in_db(
56+
mock_get_workflow, test_client, mocked_processes, test_workflow_soft_deleted
57+
):
58+
mock_get_workflow.return_value = None
59+
process_id = mocked_processes[0]
60+
test_query = build_simple_query(process_id)
61+
62+
response = test_client.post("/api/graphql", content=test_query, headers={"Content-Type": "application/json"})
63+
assert response.status_code == HTTPStatus.OK
64+
assert response.json() == {
65+
"data": {
66+
"process": {
67+
"processId": str(process_id),
68+
"userPermissions": {"resumeAllowed": False, "retryAllowed": False},
69+
}
70+
}
71+
}

0 commit comments

Comments
 (0)