Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

fix: LEAP-580: Display non-string values in Text #1675

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

hlomzik
Copy link
Collaborator

@hlomzik hlomzik commented Jan 22, 2024

During recent incident fix we missed conversion to string for Text tag's value param.
Fixing it back!

PR fulfills these requirements

  • Tests for the changes have been added/updated
  • Docs have been added/updated
  • Best efforts were made to ensure docs/code are concise and coherent (checked for spelling/grammatical errors, commented out code, debug logs etc.)
  • Self-reviewed and ran all changes on a local instance

Describe the reason for change

Non-string values can be very helpful in different cases, for example to display meta info like prices, flags, etc.

What feature flags were used to cover this change?

fflag_fix_leap_466_text_sanitization — but this flag should never be false and will be removed soon.

This change affects (describe how if yes)

  • Performance
  • Security
  • UX

Does this PR introduce a breaking change?

  • Yes, and covered entirely by feature flag(s)
  • Yes, and covered partially by feature flag(s)
  • No
  • Not sure (briefly explain the situation below)

What level of testing was included in the change?

  • e2e (codecept)
  • integration (cypress)
  • unit (jest) — no new tests, it's very hard to test object tags, but unit tests for related parseValue were fixed to replace deprecated function

During recent incident fix we missed conversion to string
for Text tag's `value` param.
@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (c8886fb) 64.60% compared to head (733a30f) 64.60%.

Files Patch % Lines
src/tags/object/RichText/model.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1675      +/-   ##
==========================================
- Coverage   64.60%   64.60%   -0.01%     
==========================================
  Files         443      443              
  Lines       28730    28730              
  Branches     7526     7526              
==========================================
- Hits        18561    18560       -1     
- Misses      10169    10170       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hlomzik
Copy link
Collaborator Author

hlomzik commented Jan 23, 2024

/git merge master

Workflow run
Successfully pushed new changes:
Merge remote-tracking branch 'origin/master' into fb-leap-580/non-string-values (733a30f)

@hlomzik hlomzik changed the title Fix: LEAP-580: Display non-string values in Text аix: LEAP-580: Display non-string values in Text Jan 23, 2024
@hlomzik hlomzik changed the title аix: LEAP-580: Display non-string values in Text fix: LEAP-580: Display non-string values in Text Jan 23, 2024
@hlomzik hlomzik enabled auto-merge (squash) January 23, 2024 15:23
@hlomzik hlomzik merged commit fb446c9 into master Jan 23, 2024
13 of 16 checks passed
@hlomzik hlomzik deleted the fb-leap-580/non-string-values branch January 23, 2024 15:34
hlomzik added a commit that referenced this pull request Jan 23, 2024
* Fix: LEAP-580: Display non-string values in Text

During recent incident fix we missed conversion to string
for Text tag's `value` param.

* Fix deprecated `substr` to `slice` in `parseValue`

* Add cypress test for non-string values

---------

Co-authored-by: hlomzik <[email protected]>
hlomzik added a commit that referenced this pull request Jan 23, 2024
fix: LEAP-580: Display non-string values in Text (#1675)

* Fix: LEAP-580: Display non-string values in Text

During recent incident fix we missed conversion to string
for Text tag's `value` param.

* Fix deprecated `substr` to `slice` in `parseValue`

* Add cypress test for non-string values

---------

Co-authored-by: hlomzik <[email protected]>
MasherJames pushed a commit to HelloPareto/label-studio-frontend that referenced this pull request Feb 29, 2024
* Fix: LEAP-580: Display non-string values in Text

During recent incident fix we missed conversion to string
for Text tag's `value` param.

* Fix deprecated `substr` to `slice` in `parseValue`

* Add cypress test for non-string values

---------

Co-authored-by: hlomzik <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants