Skip to content

Conversation

@olayinkaadelakun
Copy link
Collaborator

@olayinkaadelakun olayinkaadelakun commented Nov 28, 2025

Description

When a user create a global variable, and the type is credential, the value is persisted to the BE. The BE however doesn't send the credential back to the FE to protect the key. On the FE we receive a null for the value field which is why the index is empty. My change simply ensures that when a null is seen we just used a masked string "*****" to simulate that fact that a key was inserted.

Testcase

  1. Add a new variable (credential) under the global variable tab on the setting page
  2. Ensure that we see the masked key when the key as the been saved
  3. Add a new variable (generic) under the global variable tab on the setting page
  4. Ensure that we see the value previously inserted when the key as the been saved

Screenshot
Before
Screenshot 2025-11-28 at 10 30 31 AM

After
Screenshot 2025-11-28 at 10 31 15 AM

Summary by CodeRabbit

  • UI/UX Improvements
    • Global Variables table now masks null or undefined values, displaying them as asterisks for improved clarity and visual consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

@olayinkaadelakun olayinkaadelakun self-assigned this Nov 28, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The pull request adds a value formatter to the Global Variables table's value column to display "*****" for null or undefined values, while keeping actual values visible otherwise. Additionally, formatting adjustments (indentation and spacing) are applied to a unit test file.

Changes

Cohort / File(s) Summary
Global Variables Table Display
src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx
Added valueFormatter function to the "value" column definition to mask null/undefined values as "*****"
Test File Formatting
src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
Applied indentation, spacing, and line-wrapping adjustments; no functional or semantic changes

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • ValueFormatter is a simple, self-contained display function with no business logic
  • Test file changes are purely cosmetic formatting with no behavioral alterations

Suggested labels

lgtm

Suggested reviewers

  • edwinjosechittilappilly
  • Cristhianzl
  • erichare

Pre-merge checks and finishing touches

❌ Failed checks (1 error, 2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Test Coverage For New Implementations ❌ Error PR adds valueFormatter to mask null credentials in GlobalVariablesPage but includes no new unit tests to verify this functionality. Add test file GlobalVariablesPage.spec.ts with unit tests verifying valueFormatter returns '*****' for null values and actual values for non-null values.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Quality And Coverage ⚠️ Warning No new unit tests were added for the valueFormatter functionality that masks null credential values. Only formatting changes were made to the test file. Add comprehensive unit tests validating that the valueFormatter displays '*****' for null values and correctly displays non-null values, including edge cases.
Test File Naming And Structure ❓ Inconclusive PR modifies only formatting in codeAreaModalComponent.spec.ts without adding new tests for the valueFormatter functionality in GlobalVariablesPage. Add automated test file (e.g., GlobalVariablesPage.spec.ts) covering valueFormatter behavior for null/non-null credential values with descriptive test names and positive/negative scenarios.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: masking null values in the global variables table to hide credential fields.
Excessive Mock Usage Warning ✅ Passed E2E test uses real Playwright interactions without excessive mocking; feature change adds simple UI formatter with no problematic test patterns.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Nov 28, 2025
@github-actions github-actions bot added the bug Something isn't working label Nov 28, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 28, 2025

Frontend Unit Test Coverage Report

Coverage Summary

Lines Statements Branches Functions
Coverage: 15%
15.42% (4240/27485) 8.6% (1807/21009) 9.68% (586/6051)

Unit Test Results

Tests Skipped Failures Errors Time
1652 0 💤 0 ❌ 0 🔥 19.929s ⏱️

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Nov 28, 2025
@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.47%. Comparing base (bf5d979) to head (68aec3c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...s/SettingsPage/pages/GlobalVariablesPage/index.tsx 0.00% 4 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (40.05%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #10778      +/-   ##
==========================================
- Coverage   32.52%   32.47%   -0.06%     
==========================================
  Files        1368     1368              
  Lines       63440    63418      -22     
  Branches     9377     9379       +2     
==========================================
- Hits        20631    20592      -39     
- Misses      41770    41787      +17     
  Partials     1039     1039              
Flag Coverage Δ
frontend 14.28% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/SettingsPage/pages/GlobalVariablesPage/index.tsx 0.00% <0.00%> (ø)

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx (1)

65-67: Consider adding tests for the new value masking behavior.

The coding guidelines emphasize that tests should cover the main functionality being implemented. This new valueFormatter changes how null values are displayed, but no tests were added to verify this behavior.

Consider adding a test that:

  1. Verifies credential-type variables with null values display "*****"
  2. Verifies generic-type variables display their actual values
  3. Covers the edge case of generic variables with null/undefined values

Based on coding guidelines for frontend tests.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 271c7ff and 73381cc.

📒 Files selected for processing (2)
  • src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx (1 hunks)
  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/frontend/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)

src/frontend/src/**/*.{ts,tsx}: Use React 18 with TypeScript for frontend development
Use Zustand for state management

Files:

  • src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx
src/frontend/src/**/*.{tsx,jsx,css,scss}

📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)

Use Tailwind CSS for styling

Files:

  • src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx
src/frontend/src/**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)

src/frontend/src/**/*.{tsx,jsx}: Implement dark mode support using the useDarkMode hook and dark store
Use Lucide React for icon components in the application

Files:

  • src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx
src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx}: Use @pytest.mark.asyncio decorator for async frontend tests; structure tests to verify component behavior, state updates, and proper cleanup
Frontend tests should validate component rendering, user interactions, state management, and async operations using appropriate testing libraries (React Testing Library, Vitest, Jest, etc.)
Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes

Files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
**/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py}

📄 CodeRabbit inference engine (Custom checks)

**/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py}: Review test files for excessive use of mocks that may indicate poor test design - check if tests have too many mock objects that obscure what's actually being tested
Warn when mocks are used instead of testing real behavior and interactions
Suggest using real objects or test doubles when mocks become excessive
Ensure mocks are used appropriately for external dependencies, not core logic
Test files should have descriptive test function names that explain what is being tested
Tests should be organized logically with proper setup and teardown
Consider including edge cases and error conditions for comprehensive coverage
Verify tests cover both positive and negative scenarios where appropriate
Tests should cover the main functionality being implemented
Check that tests are not just smoke tests but actually validate behavior
Ensure tests follow the project's testing patterns (pytest for backend, Playwright for frontend)
For API endpoints, verify both success and error response testing

Files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
🧠 Learnings (9)
📚 Learning: 2025-07-11T22:12:46.255Z
Learnt from: namastex888
Repo: langflow-ai/langflow PR: 9018
File: src/frontend/src/modals/apiModal/codeTabs/code-tabs.tsx:244-244
Timestamp: 2025-07-11T22:12:46.255Z
Learning: In src/frontend/src/modals/apiModal/codeTabs/code-tabs.tsx, the inconsistent showLineNumbers setting between Step 1 (false) and Step 2 (true) in the API modal is intentional to prevent breaking the modal height. Step 1 uses showLineNumbers={false} to save vertical space while Step 2 uses showLineNumbers={true} for better readability of longer code.

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx} : Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Ensure mocks are used appropriately for external dependencies, not core logic

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Ensure tests follow the project's testing patterns (pytest for backend, Playwright for frontend)

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx} : Frontend tests should validate component rendering, user interactions, state management, and async operations using appropriate testing libraries (React Testing Library, Vitest, Jest, etc.)

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:46:45.790Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:45.790Z
Learning: Applies to src/frontend/**/*.test.{ts,tsx} : Write component tests using React Testing Library with render(), screen, and fireEvent utilities

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Consider including edge cases and error conditions for comprehensive coverage

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Tests should cover the main functionality being implemented

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-06-23T12:46:42.048Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-06-23T12:46:42.048Z
Learning: Frontend tests should be written using testing-library/react, with both component and integration tests verifying rendering, user interaction, and data loading.

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
🧬 Code graph analysis (1)
src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts (1)
src/frontend/tests/utils/await-bootstrap-test.ts (1)
  • awaitBootstrapTest (4-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 15/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 3/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 14/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 1/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 11/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 12/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 16/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 9/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 6/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 5/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 2/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 8/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 13/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 7/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 10/16
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 4/16
  • GitHub Check: Test Docker Images / Test docker images
  • GitHub Check: Test Starter Templates
🔇 Additional comments (2)
src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts (1)

1-103: LGTM - Formatting changes only.

All changes in this file are formatting adjustments (indentation and spacing) with no impact on test logic, assertions, or behavior.

src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx (1)

65-67: Current implementation correctly masks credentials and displays generic values—no changes needed.

The valueFormatter logic is correct because the backend explicitly handles masking:

  1. Backend behavior (service.py get_all() method, lines 7-26):

    • Credentials: value = None (never decrypted)
    • Generics: value = decrypted_value (always have actual values)
  2. Frontend valueFormatter correctly interprets this:

    • params.value == null → credentials return "*****"
    • params.value → generics return their decrypted value
  3. No edge case with null generic variables: Generic variables are decrypted only when variable.type == GENERIC_TYPE (service.py line 15), so they never return null unless the backend encryption fails, in which case plaintext is used (line 22).

The suggested type-checking refactor is unnecessary because the backend already guarantees only credentials have null values. The current implementation is sound.

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Nov 28, 2025
Copy link
Member

@Cristhianzl Cristhianzl left a comment

Choose a reason for hiding this comment

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

lgtm

@github-actions github-actions bot added the lgtm This PR has been approved by a maintainer label Nov 28, 2025
Copy link
Collaborator

@Adam-Aghili Adam-Aghili left a comment

Choose a reason for hiding this comment

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

LGTM!

@olayinkaadelakun olayinkaadelakun added this pull request to the merge queue Dec 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 1, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 1, 2025
@carlosrcoelho carlosrcoelho added this pull request to the merge queue Dec 1, 2025
@olayinkaadelakun olayinkaadelakun removed this pull request from the merge queue due to a manual request Dec 1, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 1, 2025
@olayinkaadelakun olayinkaadelakun added this pull request to the merge queue Dec 1, 2025
Merged via the queue into main with commit 056faea Dec 1, 2025
26 of 28 checks passed
@olayinkaadelakun olayinkaadelakun deleted the global-variable-mask-value-column branch December 1, 2025 18:49
@Adam-Aghili
Copy link
Collaborator

I have this listed as needed for 1.7.0. if that is still true please cherrypick these changes into release-1.7.0

Example: #10738

olayinkaadelakun added a commit that referenced this pull request Dec 1, 2025
* fix: mask value to hide null field being returned

* [autofix.ci] apply automated fixes

* fix: added testcase and updated functionality

---------

Co-authored-by: Olayinka Adelakun <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Carlos Coelho <[email protected]>
Co-authored-by: Olayinka Adelakun <[email protected]>
olayinkaadelakun added a commit that referenced this pull request Dec 1, 2025
* fix: mask value to hide null field being returned

* [autofix.ci] apply automated fixes

* fix: added testcase and updated functionality

---------

Co-authored-by: Olayinka Adelakun <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Carlos Coelho <[email protected]>
Co-authored-by: Olayinka Adelakun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants