-
Notifications
You must be signed in to change notification settings - Fork 37
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
Only show data stores from upsearch in the properties panel #1116
Conversation
WalkthroughWalkthroughThese updates enhance the logging capabilities for backend and frontend containers and expand the data store management system in the SpiffWorkflow backend. The changes now support handling multiple process group identifiers simultaneously, boosting flexibility and efficiency in data store operations. Moreover, the frontend now dynamically integrates the process group identifier into API calls, streamlining communication between frontend and backend components. Changes
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (7)
- Makefile (3 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/data_stores/crud.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/data_stores/json.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/data_stores/kkv.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/data_stores/typeahead.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/routes/data_store_controller.py (2 hunks)
- spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx (1 hunks)
Additional comments: 10
Makefile (3)
- 44-46: The addition of the
be-logs
target is correctly implemented for following backend container logs.- 68-70: The addition of the
fe-logs
target is correctly implemented for following frontend container logs.- 91-92: The update to the
.PHONY
target list to includebe-logs
andfe-logs
is correctly done, ensuring these targets are treated as commands.spiffworkflow-backend/src/spiffworkflow_backend/data_stores/crud.py (1)
- 27-27: The change in the
existing_data_stores
method signature to accept a list of process group identifiers aligns with the PR's objectives and is correctly implemented.spiffworkflow-backend/src/spiffworkflow_backend/data_stores/typeahead.py (1)
- 17-20: The update to the
existing_data_stores
method to accept a list ofprocess_group_identifiers
and the simplified conditional logic are correctly implemented, preparing for future enhancements.spiffworkflow-backend/src/spiffworkflow_backend/routes/data_store_controller.py (2)
- 15-15: The import of
UpsearchService
is correctly added to support the upsearch mechanism for data store visibility.- 27-36: The modifications in the
data_store_list
function to include locations for upsearch when querying data stores are correctly implemented, aligning with the PR's objectives.spiffworkflow-backend/src/spiffworkflow_backend/data_stores/json.py (1)
- 33-38: The update to the
existing_data_stores
method to accept a list ofprocess_group_identifiers
and the updated filtering logic are correctly implemented, aligning with the PR's objectives.spiffworkflow-backend/src/spiffworkflow_backend/data_stores/kkv.py (1)
- 31-36: The update to the
existing_data_stores
method to accept a list ofprocess_group_identifiers
and the updated filtering logic are correctly implemented, aligning with the PR's objectives.spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx (1)
- 383-386: The logic to extract the
processGroupIdentifier
and use it in the API call path within theonDataStoresRequested
function is a significant improvement for fetching relevant data stores based on the parent process group. This change aligns with the PR's objective to refine the visibility of data stores within the properties panel. However, consider handling the case whereprocessModel
orprocessModel.parent_groups
might benull
orundefined
to avoid potential runtime errors. A defensive programming approach could enhance robustness.Additionally, ensure that the backend endpoint
/data-stores
is equipped to handle the query parameterprocess_group_identifier
efficiently, especially considering the potential for a large number of data stores or complex relationships between process groups and data stores.However, it's recommended to verify the robustness of the implementation in edge cases, such as when
processModel
orprocessModel.parent_groups
might not be as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (3)
- spiffworkflow-backend/src/spiffworkflow_backend/api.yml (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/routes/data_store_controller.py (2 hunks)
- spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- spiffworkflow-backend/src/spiffworkflow_backend/routes/data_store_controller.py
- spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx
Additional comments: 1
spiffworkflow-backend/src/spiffworkflow_backend/api.yml (1)
- 2788-2793: The addition of the
upsearch
query parameter to the/data-stores
endpoint is clear and follows the OpenAPI specification standards. It's optional and well-documented, indicating its purpose for performing an upsearch. This change should enhance the API's functionality by providing more flexible querying capabilities for data stores. Ensure that the backend implementation correctly handles this parameter to perform the intended upsearch operation.
Work on #1041 - previous all data stores in the system were displayed in the properties panel. This fix passes in the location of the parent process group for upsearching. Also added a couple make targets for easier access to the front/backend logs when running via the dev docker containers.
Summary by CodeRabbit
New Features
Refactor
Chores