Skip to content

Conversation

@hlomzik
Copy link
Collaborator

@hlomzik hlomzik commented Nov 26, 2024

Meta text is saved in regions by Normalization mixin and it's serialized into every result of this region. When we update the meta text we should get this value again from region, not previously stored one in result.

So the priority of sources for meta value was changed. Plus "control meta" was removed.

normInput is also removed, it was an excess field.

PR fulfills these requirements

  • Tests for the changes have been added/updated (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • 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 (for bug fixes/features)

Change has impacts in these area(s)

  • Product design
  • Backend (Database)
  • Backend (API)
  • Frontend

Describe the reason for change

Meta text was not saved during annotation update, because the old saved value took precedence in serialization.

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)

It changes behaviour of meta data saving, but that's a rare feature with two data fields saved, easy to track.

What level of testing was included in the change?

  • e2e
  • integration
  • unit

Meta text is saved in regions by Normalization mixin and it's
serialized into every result of this region. When we update the meta text
we should get this value again from region, not previously stored one in result.

So the priority of sources for meta value was changed.
Plus "control meta" was removed.
@github-actions github-actions bot added the fix label Nov 26, 2024
@netlify
Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for label-studio-docs-new-theme canceled.

Name Link
🔨 Latest commit 4d961b4
🔍 Latest deploy log https://app.netlify.com/sites/label-studio-docs-new-theme/deploys/6752d4102b8f860008d7f4c9

@netlify
Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for heartex-docs canceled.

Name Link
🔨 Latest commit 4d961b4
🔍 Latest deploy log https://app.netlify.com/sites/heartex-docs/deploys/6752d410a1aa840008dab7c2

It was a model field, so controlled state went through MST update anyway.
And it's saved to actual meta in two places, so we can just get the real value there
and make input uncontrolled.
@MihajloHoma
Copy link
Contributor

MihajloHoma commented Nov 27, 2024

/git merge

Workflow run
Successfully merged: 8 files changed, 67 insertions(+), 13 deletions(-)

Copy link
Contributor

@bmartel bmartel left a comment

Choose a reason for hiding this comment

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

Nice cleanup and context, this will help a lot in furthering our code cleanups!

area.meta is always there, at least as an empty object, so we have to check keys.
@hlomzik hlomzik enabled auto-merge (squash) December 4, 2024 15:00
So it will be saved in any case. Bug was found by tests.
@hlomzik hlomzik merged commit 07f9101 into develop Dec 6, 2024
33 checks passed
@nikitabelonogov nikitabelonogov deleted the fb-leap-1651/meta branch December 6, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants