Pi migration #1892
Pi migration #1892
Conversation
WalkthroughWalkthroughThese changes introduce functionalities for checking migration eligibility and migrating process instances to a new version of their process model. New API endpoints, error handling, and event types are added to support these operations. Modifications also include new methods in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as API Client
participant Server as Server
participant Processor as ProcessInstanceProcessor
participant Service as ProcessInstanceService
Client->>Server: GET /process-instances/{model_id}/{instance_id}/check-can-migrate
Server->>Service: Check process instance can migrate
Service->>Processor: Validate migration
Processor-->>Service: Migration valid/invalid
Service-->>Server: Migration valid/invalid
Server-->>Client: Migration valid/invalid response
Note over Client,Server: Client checks if migration is possible
Client->>Server: POST /process-instance-migrate/{model_id}/{instance_id}
Server->>Service: Migrate process instance
Service->>Processor: Perform migration
Processor-->>Service: Migration result
Service-->>Server: Migration successful/failed
Server-->>Client: Migration result response
Note over Client,Server: Client initiates the migration process
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
spiffworkflow-backend/poetry.lockis excluded by!**/*.lock
Files selected for processing (14)
- spiffworkflow-backend/src/spiffworkflow_backend/api.yml (2 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/exceptions/error.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance_event.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py (2 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py (2 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py (6 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py (5 hunks)
- spiffworkflow-backend/tests/data/migration-test-with-subprocess/migration-initial.bpmn (1 hunks)
- spiffworkflow-backend/tests/data/migration-test-with-subprocess/migration-new.bpmn (1 hunks)
- spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py (1 hunks)
- spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.py (2 hunks)
- spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py (3 hunks)
- spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py (1 hunks)
- spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_service.py (2 hunks)
Files skipped from review due to trivial changes (2)
- spiffworkflow-backend/src/spiffworkflow_backend/exceptions/error.py
- spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance_event.py
Additional comments not posted (35)
spiffworkflow-backend/tests/data/migration-test-with-subprocess/migration-initial.bpmn (5)
1-2: Metadata and namespaces look good.The
definitionselement correctly includes necessary namespaces and metadata.
3-3: Process definition looks good.The
processelement is correctly defined with a valid ID and is marked as executable.
4-11: Start and end events look good.The
startEventandendEventelements are correctly defined and connected via sequence flows.
12-27: Subprocess looks good.The
subProcesselement is correctly defined and contains valid BPMN elements such asstartEvent,manualTask, andendEvent.
28-71: BPMN diagram elements look good.The BPMN diagram elements correctly represent the process and subprocess, including shapes and edges.
spiffworkflow-backend/tests/data/migration-test-with-subprocess/migration-new.bpmn (5)
1-2: Metadata and namespaces look good.The
definitionselement correctly includes necessary namespaces and metadata.
3-3: Process definition looks good.The
processelement is correctly defined with a valid ID and is marked as executable.
4-11: Start and end events look good.The
startEventandendEventelements are correctly defined and connected via sequence flows.
12-32: Subprocess looks good.The
subProcesselement is correctly defined and contains valid BPMN elements such asstartEvent,manualTask, andendEvent.
33-83: BPMN diagram elements look good.The BPMN diagram elements correctly represent the process and subprocess, including shapes and edges.
spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.py (3)
59-116: Test method for process instance migration looks good.The
test_process_instance_migratemethod correctly covers the migration process and verifies the correctness of the migration.
117-163: Test method for checking process instance migration eligibility looks good.The
test_process_instance_check_can_migratemethod correctly covers the check for migration eligibility and verifies the correctness of the check.
Line range hint
10-58: Test method for finding process instance by ID looks good.The
test_find_by_idmethod correctly covers the retrieval process and verifies the correctness of the retrieval.spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_service.py (4)
157-214: Test method for process instance migration looks good.The
test_it_can_migrate_a_process_instancemethod correctly covers the migration process and verifies the correctness of the migration.
215-251: Test method for checking process instance migration eligibility looks good.The
test_it_can_check_if_a_process_instance_can_be_migratedmethod correctly covers the check for migration eligibility and verifies the correctness of the check.
252-293: Test method for handling unsafe migrations looks good.The
test_it_raises_if_a_process_instance_cannot_be_migrated_to_new_process_model_versionmethod correctly covers the scenario where a process instance cannot be migrated and verifies the handling of unsafe migrations.
Line range hint
21-56: Test method for saving file data looks good.The
test_save_file_data_v0method correctly covers the saving of file data with digest references and verifies the correctness of the process.spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py (1)
103-134: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
create_and_run_process_instanceare correctly implemented.Verification successful
Function usage verified.
The function
create_and_run_process_instanceis correctly implemented in the codebase.
spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_message_service.pyspiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.pyspiffworkflow-backend/src/spiffworkflow_backend/routes/webhooks_controller.pyspiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.pyspiffworkflow-backend/src/spiffworkflow_backend/routes/extensions_controller.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `create_and_run_process_instance` are correctly implemented. # Test: Search for the function usage. Expect: Only occurances of the correct usage. rg --type python $'create_and_run_process_instance'Length of output: 81
Script:
#!/bin/bash # Description: Verify all function calls to `create_and_run_process_instance` are correctly implemented. # Test: Search for the function usage. Expect: Only occurrences of the correct usage. rg 'create_and_run_process_instance'Length of output: 1083
Script:
#!/bin/bash # Description: Extract lines around each occurrence of `create_and_run_process_instance` to review the context. # Extract lines around the occurrences in the test_message_service.py rg 'create_and_run_process_instance' -A 5 -B 5 spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_message_service.py # Extract lines around the occurrences in the test_process_instances_controller.py rg 'create_and_run_process_instance' -A 5 -B 5 spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.py # Extract lines around the occurrences in the base_test.py rg 'create_and_run_process_instance' -A 5 -B 5 spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py # Extract lines around the occurrences in the webhooks_controller.py rg 'create_and_run_process_instance' -A 5 -B 5 spiffworkflow-backend/src/spiffworkflow_backend/routes/webhooks_controller.py # Extract lines around the occurrences in the process_instance_service.py rg 'create_and_run_process_instance' -A 5 -B 5 spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py # Extract lines around the occurrences in the extensions_controller.py rg 'create_and_run_process_instance' -A 5 -B 5 spiffworkflow-backend/src/spiffworkflow_backend/routes/extensions_controller.pyLength of output: 3550
spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py (2)
567-574: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
process_instance_migrateare correctly implemented.
550-565: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
process_instance_check_can_migrateare correctly implemented.Verification successful
Function
process_instance_check_can_migrateis correctly implemented and tested.
- The function is correctly referenced in the
api.ymlfile as part of the API specification.- There is a corresponding test function
test_process_instance_check_can_migratein thetest_process_instances_controller.pyfile.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `process_instance_check_can_migrate` are correctly implemented. # Test: Search for the function usage. Expect: Only occurances of the correct usage. rg --type python $'process_instance_check_can_migrate'Length of output: 84
Script:
#!/bin/bash # Description: Verify all function calls to `process_instance_check_can_migrate` are correctly implemented. # Test: Search for the function usage. Expect: Only occurrences of the correct usage. rg 'process_instance_check_can_migrate'Length of output: 482
Script:
#!/bin/bash # Description: Verify the function usage in the api.yml file and the test file. # Check the relevant section in the api.yml file. echo "Checking api.yml for function usage..." rg -A 5 'operationId: spiffworkflow_backend.routes.process_instances_controller.process_instance_check_can_migrate' spiffworkflow-backend/src/spiffworkflow_backend/api.yml # Check the relevant section in the test file. echo "Checking test_process_instances_controller.py for function usage..." rg -A 10 'def test_process_instance_check_can_migrate' spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.pyLength of output: 1295
spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py (2)
111-114: LGTM! But verify the test case implementation in the codebase.The code changes are approved.
However, ensure that the test case for
process-instance-migratepermission creation is correctly implemented.Verification successful
LGTM! But verify the test case implementation in the codebase.
The code changes are approved.
The test case for
process-instance-migratepermission creation is correctly implemented inspiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the test case implementation for `process-instance-migrate` permission creation. # Test: Search for the test case usage. Expect: Only occurances of the correct usage. rg --type python $'process-instance-migrate'Length of output: 74
Script:
#!/bin/bash # Description: Verify the test case implementation for `process-instance-migrate` permission creation. # Test: Search for the test case usage. Expect: Only occurrences of the correct usage. rg 'process-instance-migrate'Length of output: 1084
199-202: LGTM! But verify the test case implementation in the codebase.The code changes are approved.
However, ensure that the test case for
process-instance-migratepermission creation is correctly implemented.Verification successful
The test case for
process-instance-migratepermission creation is correctly implemented.The search results indicate that
process-instance-migrateis being tested in the relevant files, includingtest_authorization_service.py.
spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.pyspiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.pyspiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the test case implementation for `process-instance-migrate` permission creation. # Test: Search for the test case usage. Expect: Only occurances of the correct usage. rg --type python $'process-instance-migrate'Length of output: 74
Script:
#!/bin/bash # Description: Verify the test case implementation for `process-instance-migrate` permission creation. # Test: Search for the test case usage. Expect: Only occurrences of the correct usage. rg --type py 'process-instance-migrate'Length of output: 951
spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py (4)
20-23: LGTM! Imports are necessary and correctly used.The imports from
SpiffWorkflow.bpmn.util.diffare required for the migration functionalities, and the import fromspiffworkflow_backend.exceptions.erroris necessary for handling migration-related errors.
148-175: LGTM! The method correctly checks migration safety.The
check_process_instance_can_be_migratedmethod correctly usesdiff_workflowto get the differences and checks if the migration is safe. It raises an appropriate exception if the migration is not safe.
178-221: LGTM! The method correctly performs the migration.The
migrate_process_instance_to_newest_model_versionmethod correctly performs the migration by callingcheck_process_instance_can_be_migratedand usingmigrate_workflow. It handles errors appropriately and has a TODO comment for missing tests.
733-742: LGTM! The method correctly checks migration safety.The
can_migratemethod correctly checks if the migration is safe by ensuring that no tasks in the workflow differences are in completed or started states.spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py (2)
73-73: Addition of Migration Path Segment ApprovedThe new path segment
/process-instance-migratewith the relevant permissioncreatehas been correctly added.
641-643: Addition of Migration Action ApprovedThe new process instance action
migratehas been correctly added to the permission loop inset_support_permissions.spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py (5)
1080-1085: Good test setup and teardown.The test setup and teardown are well-structured, ensuring a clean environment for each test run.
1086-1093: Comprehensive test coverage.The test covers the initialization and execution of process instances, ensuring the GUIDs are updated correctly.
1099-1100: Good use of ProcessInstanceService for task completion.The test correctly uses
ProcessInstanceService.complete_form_taskfor task completion.
1112-1117: Correctly verifies GUID updates.The test correctly verifies that the GUIDs for tasks and subprocesses are updated.
1122-1125: Ensures consistency after GUID updates.The test ensures that the task names remain consistent after GUID updates, which is crucial for process integrity.
spiffworkflow-backend/src/spiffworkflow_backend/api.yml (2)
1333-1365: Endpoint for checking process instance migration eligibility looks good.The GET method and required path parameters are well-defined. The response schema is clear and consistent with other endpoints.
1498-1524: Endpoint for migrating process instances looks good.The POST method and required path parameters are well-defined. The response schema is clear and consistent with other endpoints.
spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
spiffworkflow-backend/poetry.lockis excluded by!**/*.lock
Files selected for processing (3)
- spiffworkflow-backend/src/spiffworkflow_backend/exceptions/error.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py (2 hunks)
- spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.py (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- spiffworkflow-backend/src/spiffworkflow_backend/exceptions/error.py
- spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py
- spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.py
Supports #1671
Adds an api to migrate a process instance to the new version of the api.
New APIs:
/process-instances/{modified_process_model_identifier}/{process_instance_id}/check-can-migrate/process-instance-migrate/{modified_process_model_identifier}/{process_instance_id}Summary by CodeRabbit
New Features
process_instance_migrated.ProcessInstanceMigrationNotSafeError.Bug Fixes
Tests