Skip to content
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

Change workflow queries to use API to get global workflows #1688

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Jan 31, 2025

Important

Refactor to use API-based global workflow queries with useGlobalWorkflowsQuery, removing reliance on environment variables.

  • Behavior:
    • Replaces environment variable globalWorkflowIds with API-based query using useGlobalWorkflowsQuery.
    • Updates WorkflowPage, WorkflowTitle, WorkflowHeader, and other components to use useGlobalWorkflowsQuery for global workflow checks.
  • Hooks:
    • Adds useGlobalWorkflowsQuery in useGlobalWorkflowsQuery.ts to fetch global workflows with template=true.
    • Refactors useWorkflowQuery, useWorkflowRunQuery, useWorkflowRunTimelineQuery, and useWorkflowRunsQuery to use useGlobalWorkflowsQuery for determining global workflows.
  • Misc:
    • Removes globalWorkflowIds from env.ts.

This description was created by Ellipsis for e378525. It will automatically update as commits are pushed.

…src/'

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Refactor workflow queries to use `useGlobalWorkflowsQuery` for fetching global workflows, removing reliance on environment variables.
>
>   - **Behavior**:
>     - Replaces direct API calls with `useGlobalWorkflowsQuery` in `WorkflowPage.tsx`, `WorkflowTitle.tsx`, and `WorkflowHeader.tsx` to fetch global workflows.
>     - Removes reliance on `globalWorkflowIds` from `env.ts`.
>   - **Hooks**:
>     - Adds `useGlobalWorkflowsQuery` to fetch global workflows with `template=true`.
>     - Updates `useWorkflowQuery`, `useWorkflowRunQuery`, `useWorkflowRunTimelineQuery`, and `useWorkflowRunsQuery` to use `useGlobalWorkflowsQuery` for determining global workflows.
>   - **Misc**:
>     - Removes `getGlobalWorkflowIds` and `globalWorkflowIds` from `env.ts`.
>
> <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 42937efb83d5c50403e12958a798fc23708591ff. It will automatically update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on e378525 in 37 seconds

More details
  • Looked at 404 lines of code in 9 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/hooks/useGlobalWorkflowsQuery.ts:21
  • Draft comment:
    Consider adding a loading state check for globalWorkflows to ensure it's fully loaded before being used in other components or hooks. This can prevent potential null or undefined errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of globalWorkflows in multiple hooks and components without checking if it's loaded can lead to potential null or undefined errors. It's better to handle the loading state explicitly.
2. skyvern-frontend/src/routes/workflows/WorkflowTitle.tsx:15
  • Draft comment:
    Ensure that globalWorkflows is fully loaded before using it. Consider handling the loading state explicitly to avoid potential null or undefined errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The useGlobalWorkflowsQuery is used in multiple places, and its loading state is not consistently checked. This can lead to potential issues if the data is not yet available.
3. skyvern-frontend/src/routes/workflows/editor/WorkflowHeader.tsx:34
  • Draft comment:
    Ensure that globalWorkflows is fully loaded before using it. Consider handling the loading state explicitly to avoid potential null or undefined errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The useGlobalWorkflowsQuery is used in multiple places, and its loading state is not consistently checked. This can lead to potential issues if the data is not yet available.
4. skyvern-frontend/src/routes/workflows/hooks/useWorkflowQuery.ts:11
  • Draft comment:
    Ensure that globalWorkflows is fully loaded before using it. Consider handling the loading state explicitly to avoid potential null or undefined errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The useGlobalWorkflowsQuery is used in multiple places, and its loading state is not consistently checked. This can lead to potential issues if the data is not yet available.
5. skyvern-frontend/src/routes/workflows/hooks/useWorkflowRunQuery.ts:15
  • Draft comment:
    Ensure that globalWorkflows is fully loaded before using it. Consider handling the loading state explicitly to avoid potential null or undefined errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The useGlobalWorkflowsQuery is used in multiple places, and its loading state is not consistently checked. This can lead to potential issues if the data is not yet available.
6. skyvern-frontend/src/routes/workflows/hooks/useWorkflowRunTimelineQuery.ts:13
  • Draft comment:
    Ensure that globalWorkflows is fully loaded before using it. Consider handling the loading state explicitly to avoid potential null or undefined errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The useGlobalWorkflowsQuery is used in multiple places, and its loading state is not consistently checked. This can lead to potential issues if the data is not yet available.
7. skyvern-frontend/src/routes/workflows/hooks/useWorkflowRunsQuery.ts:25
  • Draft comment:
    Ensure that globalWorkflows is fully loaded before using it. Consider handling the loading state explicitly to avoid potential null or undefined errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The useGlobalWorkflowsQuery is used in multiple places, and its loading state is not consistently checked. This can lead to potential issues if the data is not yet available.

Workflow ID: wflow_Hs0fIoYmOlcnyhKP


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to e378525 in 1 minute and 19 seconds

More details
  • Looked at 404 lines of code in 9 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/WorkflowTitle.tsx:15
  • Draft comment:
    Consider using a context or a higher-level component to provide globalWorkflows to avoid multiple components independently fetching the same data. This can improve performance by reducing redundant API calls.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of useGlobalWorkflowsQuery in multiple components and hooks is consistent and seems to replace the previous method of checking global workflow IDs. However, there is a potential performance issue due to multiple components independently fetching the same data.
2. skyvern-frontend/src/routes/workflows/editor/WorkflowHeader.tsx:34
  • Draft comment:
    Consider using a context or a higher-level component to provide globalWorkflows to avoid multiple components independently fetching the same data. This can improve performance by reducing redundant API calls.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of useGlobalWorkflowsQuery in multiple components and hooks is consistent and seems to replace the previous method of checking global workflow IDs. However, there is a potential performance issue due to multiple components independently fetching the same data.
3. skyvern-frontend/src/routes/workflows/hooks/useWorkflowQuery.ts:11
  • Draft comment:
    Consider using a context or a higher-level component to provide globalWorkflows to avoid multiple components independently fetching the same data. This can improve performance by reducing redundant API calls.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of useGlobalWorkflowsQuery in multiple components and hooks is consistent and seems to replace the previous method of checking global workflow IDs. However, there is a potential performance issue due to multiple components independently fetching the same data.
4. skyvern-frontend/src/routes/workflows/hooks/useWorkflowRunQuery.ts:15
  • Draft comment:
    Consider using a context or a higher-level component to provide globalWorkflows to avoid multiple components independently fetching the same data. This can improve performance by reducing redundant API calls.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of useGlobalWorkflowsQuery in multiple components and hooks is consistent and seems to replace the previous method of checking global workflow IDs. However, there is a potential performance issue due to multiple components independently fetching the same data.
5. skyvern-frontend/src/routes/workflows/hooks/useWorkflowRunTimelineQuery.ts:13
  • Draft comment:
    Consider using a context or a higher-level component to provide globalWorkflows to avoid multiple components independently fetching the same data. This can improve performance by reducing redundant API calls.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of useGlobalWorkflowsQuery in multiple components and hooks is consistent and seems to replace the previous method of checking global workflow IDs. However, there is a potential performance issue due to multiple components independently fetching the same data.
6. skyvern-frontend/src/routes/workflows/hooks/useWorkflowRunsQuery.ts:25
  • Draft comment:
    Consider using a context or a higher-level component to provide globalWorkflows to avoid multiple components independently fetching the same data. This can improve performance by reducing redundant API calls.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of useGlobalWorkflowsQuery in multiple components and hooks is consistent and seems to replace the previous method of checking global workflow IDs. However, there is a potential performance issue due to multiple components independently fetching the same data.

Workflow ID: wflow_7oyLcxmMnGA9J6za


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@msalihaltun msalihaltun merged commit f38ba59 into main Jan 31, 2025
7 checks passed
@msalihaltun msalihaltun deleted the salih/fetch-global-workflows-from-api branch January 31, 2025 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants