Skip to content

ITEP-66910 - Fix adding a label on a subgroup #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jpggvilaca
Copy link
Contributor

πŸ“ Description

  • Fix adding a label on a subgroup for hierarchical classification projects

✨ Type of Change

Select the type of change your PR introduces:

  • 🐞 Bug fix – Non-breaking change which fixes an issue
  • πŸš€ New feature – Non-breaking change which adds functionality
  • πŸ”¨ Refactor – Non-breaking change which refactors the code base
  • πŸ’₯ Breaking change – Changes that break existing functionality
  • πŸ“š Documentation update
  • πŸ”’ Security update
  • πŸ§ͺ Tests

πŸ§ͺ Testing Scenarios

Describe how the changes were tested and how reviewers can test them too:

  • βœ… Tested manually
  • πŸ€– Run automated end-to-end tests

βœ… Checklist

Before submitting the PR, ensure the following:

  • πŸ” PR title is clear and descriptive
  • πŸ“ For internal contributors: If applicable, include the JIRA ticket number (e.g., ITEP-123456) in the PR title. Do not include full URLs
  • πŸ’¬ I have commented my code, especially in hard-to-understand areas
  • πŸ“„ I have made corresponding changes to the documentation
  • βœ… I have added tests that prove my fix is effective or my feature works

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the payload for adding a label under a subgroup by using the element’s id instead of its name.

  • Updated the label payload to reference parentElement.id for subgroup relationships.
  • Adjusted the unit test to assert the new parentLabelId value.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
web_ui/src/core/projects/hooks/utils.ts Switched parentLabelId from parentElement.name to parentElement.id.
web_ui/src/core/projects/hooks/utils.test.ts Updated the expected parentLabelId in the test to match the new UUID.

@dwesolow
Copy link
Contributor

Could you add a unit test for this?

@jpggvilaca
Copy link
Contributor Author

Could you add a unit test for this?

it exists already. i updated it. it failed when i made the change

return getNewLabelPayloadNew({ ...label, parentLabelId: parentElement.name }, shouldRevisit);
return getNewLabelPayloadNew({ ...label, parentLabelId: parentElement.id }, shouldRevisit);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this also work when creating a new group hiearchy? I.e. when the parentElement is also a new label (so therefore not having an id)

Copy link
Contributor

Choose a reason for hiding this comment

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

I also thought about that case - the problem is that during creation we do not have id, only the name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does : )

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.

4 participants