Skip to content
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

feat: delete suggestion from record on search engine #4336

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

jfcalvo
Copy link
Member

@jfcalvo jfcalvo commented Nov 28, 2023

Description

Delete a suggestion from the record document on search engine when the suggestion is deleted using it's deletion endpoint.

Ref #4230

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested

  • Running tests locally.
  • Checking manually that deleting a suggestion is affecting filters later on the UI.

Checklist

  • I added relevant documentation
  • follows the style guidelines of this project
  • I did a self-review of my code
  • I made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I filled out the contributor form (see text above)
  • I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. area: api Indicates that an issue or pull request is related to the Fast API server or REST endpoints language: python Pull requests or issues that update Python code team: backend Indicates that the issue or pull request is owned by the backend team type: enhancement Indicates new feature requests labels Nov 28, 2023
@@ -1092,20 +1092,32 @@ async def upsert_suggestion(

async def delete_suggestions(db: "AsyncSession", record: Record, suggestions_ids: List[UUID]) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

this code may not be affected?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's clearly affected yes. I will take a look.

Copy link
Member Author

Choose a reason for hiding this comment

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

We discussed it and we will take a first approach reindexing suggestions individually.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 28, 2023
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6630d7b) 90.13% compared to head (5b4c971) 65.84%.
Report is 496 commits behind head on develop.

❗ Current head 5b4c971 differs from pull request most recent head 8721334. Consider uploading reports for the commit 8721334 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #4336       +/-   ##
============================================
- Coverage    90.13%   65.84%   -24.30%     
============================================
  Files          233      330       +97     
  Lines        12493    19036     +6543     
============================================
+ Hits         11261    12534     +1273     
- Misses        1232     6502     +5270     
Flag Coverage Δ
pytest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link

The URL of the deployed environment for this PR is https://argilla-quickstart-pr-4336-ki24f765kq-no.a.run.app

Co-authored-by: Francisco Aranda <[email protected]>
@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label Nov 28, 2023
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 28, 2023
@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label Nov 28, 2023
@jfcalvo jfcalvo merged commit d42e888 into develop Nov 28, 2023
13 of 14 checks passed
@jfcalvo jfcalvo deleted the feature/search-engine-delete-suggestion branch November 28, 2023 18:15
davidberenstein1957 pushed a commit that referenced this pull request Nov 29, 2023
# Description

Delete a suggestion from the record document on search engine when the
suggestion is deleted using it's deletion endpoint.

Ref #4230 

**Type of change**

- [x] New feature (non-breaking change which adds functionality)

**How Has This Been Tested**

- [x] Running tests locally.
- [x] Checking manually that deleting a suggestion is affecting filters
later on the UI.

**Checklist**

- [ ] I added relevant documentation
- [x] follows the style guidelines of this project
- [x] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [ ] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

---------

Co-authored-by: Francisco Aranda <[email protected]>
leiyre pushed a commit that referenced this pull request Nov 29, 2023
* develop: (30 commits)
  chore: increase dev version release to 1.21.0
  fix: responses and suggestions filter QA (#4337)
  feat: delete suggestion from record on search engine (#4336)
  feat: update suggestion from record on search engine (#4339)
  bug: fix bug and update test (#4341)
  fix: preserve `TextClassificationSettings.label_schema` order (#4332)
  Update issue templates
  feat: 🚀 support for filtering and sorting by responses and suggestions (#4160)
  fix: handling errors for non-existing endpoints (#4325)
  feat: adding utils module and functions (#4121)
  Update labels in github workflows (#4315)
  fix: correct unification implementation for `RankingQuestionStrategy` (#4295)
  fix: update to solve the error of integration tests in CI (#4314)
  docs: revisit install process (#4261)
  feat: increase timeout minutes for python tests (#4307)
  docs: docs export dataset does not apply coloring for code snippets (#4296)
  docs: update final section of the rag haystack blog post (#4294)
  feat: add multi_modal templates and update vector setting (#4283)
  feat: better logging bar for FeedbackDataset (#4267)
  refactor: ArgillaTrainer for unified variable usage (#4214)
  ...

# Conflicts:
#	frontend/v1/infrastructure/repositories/RecordRepository.ts
leiyre pushed a commit that referenced this pull request Dec 5, 2023
* develop: (41 commits)
  chore: update dev version
  chore: update CHANGELOG.md before release v1.20.0 (#4357)
  docs: temporal update to indicate persistent storage (#4355)
  docs: add suggestions and responses filters and sorting (#4345)
  docs: add end2end example on creating a basic text-classification dataset (#4208)
  Fix/responses suggestions filter fine tune (#4356)
  Fix/responses suggestions filter fine tune (#4356)
  fix: Accept draft responses on dataset records creation (#4354)
  Feature/responses operator (#4352)
  Feature/responses operator (#4352)
  chore: increase dev version release to 1.21.0
  chore: remove dev suffix for release branch
  fix: responses and suggestions filter QA (#4337)
  feat: delete suggestion from record on search engine (#4336)
  feat: update suggestion from record on search engine (#4339)
  bug: fix bug and update test (#4341)
  fix: preserve `TextClassificationSettings.label_schema` order (#4332)
  Update issue templates
  feat: 🚀 support for filtering and sorting by responses and suggestions (#4160)
  fix: handling errors for non-existing endpoints (#4325)
  ...

# Conflicts:
#	frontend/v1/domain/entities/question/Question.ts
#	frontend/v1/domain/entities/record/Record.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api Indicates that an issue or pull request is related to the Fast API server or REST endpoints language: python Pull requests or issues that update Python code size:M This PR changes 30-99 lines, ignoring generated files. team: backend Indicates that the issue or pull request is owned by the backend team type: enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants