Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

fix(steps-details): Required object type's parameters are lost #1605

Merged
merged 1 commit into from
Mar 31, 2023
Merged

fix(steps-details): Required object type's parameters are lost #1605

merged 1 commit into from
Mar 31, 2023

Conversation

lordrip
Copy link
Member

@lordrip lordrip commented Mar 31, 2023

Description

Currently, having a step with a required object type parameter gets removed from the Code Editor upon opening the step's details panel.

This issue occurs because we're instructing uniforms that we have a required object property without a default or initial value.

Once uniforms tries to build the initial form value, it returns an empty object. This happens here.

Changes

Considering that there are no current plans to support object property definitions, the workaround is to provide a default value alongside the schema to feed the initial value of object properties.

Before

Screencast.from.2023-03-31.13-11-26.webm

After

Screencast.from.2023-03-31.13-10-26.webm

Fixes: #1603

Currently, having a step with a required object type parameter
gets removed from the Code Editor upon opening the step's details
panel.

This issue occurs because we're instructing `uniforms` that we have
a required object property without a default or initial value.

Once `uniforms` tries to build the initial form value, it returns an
empty object. [This happens here](https://github.com/vazco/uniforms/blob/ecb1785468bfe2508399a48e13bdb921549ab91a/packages/uniforms-bridge-json-schema/src/JSONSchemaBridge.ts#L267-L276).

Considering that there are no current plans to support object property
definitions, the workaround is to provide a default value alongside
the schema to feed the initial value of object properties.

Fixes: #1603
@lordrip lordrip requested a review from a team March 31, 2023 11:12
@lordrip
Copy link
Member Author

lordrip commented Mar 31, 2023

Hi @unsortedhashsets, I would like to add a test for this issue, could you please give me some pointers to do so? 😃

@sonarcloud
Copy link

sonarcloud bot commented Mar 31, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #1605 (1484087) into main (9405caa) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1605      +/-   ##
==========================================
- Coverage   57.03%   56.98%   -0.05%     
==========================================
  Files          69       69              
  Lines        2055     2053       -2     
  Branches      470      470              
==========================================
- Hits         1172     1170       -2     
  Misses        839      839              
  Partials       44       44              
Impacted Files Coverage Δ
src/components/CustomJsonSchemaBridge.tsx 26.66% <ø> (-8.63%) ⬇️
src/components/VisualizationStepViews.tsx 69.23% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@igarashitm igarashitm merged commit d04a654 into KaotoIO:main Mar 31, 2023
@unsortedhashsets
Copy link
Member

unsortedhashsets commented Apr 17, 2023

Hi @unsortedhashsets, I would like to add a test for this issue, could you please give me some pointers to do so? smiley

Hi @lordrip, sorry for not fast response, was on vacation.

If you want to do it, I recommend to expand section 08 -> code_editor_actions.cy.js

  • Start a new it
  • Use fixture with Marshal step in it -> cy.uploadFixture('EipAction.yaml');
  • openStepConfigurationTab
  • closeStepConfigurationTab
  • assert editor with checkCodeSpanLine

All these extra functions described in /cypress/support/kaoto-ui-commands/

seems that is all for that test (just may be issue with stability between open/close so you can add extra assert after open if some flakiness will take place 🤔)

If you will have any questions just tag me or write in chat 👍

@lordrip lordrip deleted the fix/required-object-parameteres-are-lost branch May 6, 2023 17:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Required object parameters gets removed from the Code Editor when opening Step Details
3 participants