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

fix: LEAP-640: Don't save empty draft on View All #1689

Merged
merged 5 commits into from
Feb 9, 2024

Conversation

hlomzik
Copy link
Collaborator

@hlomzik hlomzik commented Feb 2, 2024

We should save draft when user clicks on View All, but we should save it only if there are changes made to annotation.

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

Draft is created/updated every time user click on View All, even if that's fresh new task, if there are no changes, if draft was saved a second ago.

What alternative approaches were there?

We could add checks for actual changes to saveDraftImmediatelyWithResults(), but the whole approach should be revisited. So current fix is a quick patch for the actual problem.
Also the main reason for draft not being saved is the check for editable in saveDraft() — at this moment View All is already displayed and all annotations are not editable. Most probably it will be enough just to rewrite this check to check for annotation explicitly.

This change affects (describe how if yes)

  • Performance
  • Security
  • UX — toast about "Draft saved" disappeared but it was only introduced a month ago; we'll return it later

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)

We should save draft when user clicks on View All,
but we should save it only if there are changes or unsubmitted comment.
@hlomzik
Copy link
Collaborator Author

hlomzik commented Feb 8, 2024

/git merge master

Workflow run
Already up-to-date. Nothing to commit.

@yyassi-heartex yyassi-heartex merged commit 776fa04 into master Feb 9, 2024
14 checks passed
@yyassi-heartex yyassi-heartex deleted the fb-leap-640/no-empty-drafts-on-view-all branch February 9, 2024 15:14
MasherJames pushed a commit to HelloPareto/label-studio-frontend that referenced this pull request Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants