fix: set eval version for workflows artefact #34197
Conversation
WalkthroughThe changes primarily involve consolidating the Changes
Assessment against linked issues
Poem
Warning Review ran into problemsProblems (1)
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 (
|
github-actions
bot
added
Bug
github-actions
bot
added
Bug
ayushpahwa
changed the title
github-actions
bot
added
Bug
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
app/client/src/ce/constants/WorkflowConstants.ts (1)
1-1: The addition ofevaluationVersionto theWorkflowinterface is well-implemented and aligns with the PR's objectives to handle different evaluation versions.Consider adding a comment above the
evaluationVersionfield to explain its purpose and usage, enhancing maintainability.Also applies to: 19-19
app/client/src/ce/selectors/applicationSelectors.tsx (2)
Line range hint
79-84: Consider removing the unnecessary else clause to simplify the code.- } else if ( + if (
Line range hint
165-167: Consider removing the unnecessary else clause to simplify the code.- } else { + {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/src/ce/constants/WorkflowConstants.ts (2 hunks)
- app/client/src/ce/selectors/applicationSelectors.tsx (1 hunks)
Additional context used
Biome
app/client/src/ce/selectors/applicationSelectors.tsx
[error] 79-84: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 165-167: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Additional comments not posted (2)
app/client/src/ce/selectors/applicationSelectors.tsx (2)
181-181: ExportingDEFAULT_EVALUATION_VERSIONis a good practice as it ensures consistency across the application when referencing the default evaluation version.
181-181: TheselectEvaluationVersionselector is well-implemented, providing a sensible default if theevaluationVersionis not set. This enhances robustness by preventing potential runtime errors.
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/src/ce/constants/WorkflowConstants.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/src/ce/constants/WorkflowConstants.ts
ayushpahwa
added
ok-to-test
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- app/client/src/ce/api/ApplicationApi.tsx (1 hunks)
- app/client/src/ce/constants/ReduxActionConstants.tsx (2 hunks)
- app/client/src/ce/constants/WorkflowConstants.ts (2 hunks)
- app/client/src/ce/selectors/applicationSelectors.tsx (2 hunks)
- app/client/src/ce/workers/Evaluation/Actions.ts (1 hunks)
- app/client/src/constants/EvalConstants.ts (1 hunks)
- app/client/src/ee/configs/index.ts (1 hunks)
- app/client/src/sagas/EvaluationsSaga.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/constants/EvalConstants.ts
Files skipped from review as they are similar to previous changes (2)
- app/client/src/ce/constants/WorkflowConstants.ts
- app/client/src/ce/selectors/applicationSelectors.tsx
Additional comments not posted (5)
app/client/src/ee/configs/index.ts (1)
2-2: Centralizing theEvaluationVersiontype import enhances maintainability and consistency across the codebase.app/client/src/ce/workers/Evaluation/Actions.ts (1)
11-11: ImportingEvaluationVersionfromconstants/EvalConstantsstandardizes its usage across different modules, aligning with best practices for type management.app/client/src/ce/api/ApplicationApi.tsx (1)
17-17: The update to importEvaluationVersionfromconstants/EvalConstantsensures consistency and reduces redundancy in type definitions across the application.app/client/src/sagas/EvaluationsSaga.ts (1)
73-73: ImportingEvaluationVersionfromconstants/EvalConstantsin the saga file aligns with the centralized management of type definitions, facilitating easier updates and maintenance.app/client/src/ce/constants/ReduxActionConstants.tsx (1)
19-19: The import change forEvaluationVersioncentralizes its definition, which is a good practice for maintainability.
ayushpahwa
added
ok-to-test
## Description Eval version 2 is used to properly handle the escape characters like `\n` etc in the eval flow. If not set, it is defaulting to behaviour for eval version 1. This PR introduces the type fixes to allow for eval version to be set via the API. The main change is in the EE PR and this PR is to update the type. Fixes #34171 ## Automation /test sanity ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9478023950> > Commit: c5edfb3 > Cypress dashboard url: <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9478023950&attempt=1" target="_blank">Click here!</a> <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Consolidated `EvaluationVersion` type imports to a single source for improved maintainability. - Promoted `DEFAULT_EVALUATION_VERSION` to be exportable for broader use across the application. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…release-blocker fix: set eval version for workflows artefact (#34197)
Description
Eval version 2 is used to properly handle the escape characters like
\netc in the eval flow. If not set, it is defaulting to behaviour for eval version 1. This PR introduces the type fixes to allow for eval version to be set via the API. The main change is in the EE PR and this PR is to update the type.Fixes #34171
Automation
/test sanity
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9478023950
Commit: c5edfb3
Cypress dashboard url: Click here!
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
EvaluationVersiontype imports to a single source for improved maintainability.DEFAULT_EVALUATION_VERSIONto be exportable for broader use across the application.