From b4e8bad502acd63b6168c67d465a8fdc31d35ff6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dami=C3=A1n=20Pumar?= Date: Wed, 24 Jul 2024 12:55:47 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Add=20individual=20update=20(#5298)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paco Aranda Co-authored-by: Leire Aguirre --- .../annotation/settings/SettingsInfo.vue | 82 +++++++-- .../settings/SettingsInfoReadOnly.vue | 17 ++ .../annotation/settings/SettingsMetadata.vue | 6 +- .../settings/useSettingInfoViewModel.ts | 12 +- .../settings/useSettingsMetadataViewModel.ts | 2 +- .../entities/__mocks__/dataset/mocks.ts | 20 +++ .../domain/entities/dataset/Dataset.test.ts | 169 ++++++++++++++++++ .../v1/domain/entities/dataset/Dataset.ts | 79 +++++++- .../v1/domain/services/IDatasetRepository.ts | 2 +- .../update-dataset-setting-use-case.ts | 32 +++- .../repositories/DatasetRepository.ts | 26 ++- .../repositories/RecordRepository.ts | 62 +++---- .../v1/infrastructure/types/record.ts | 2 +- 13 files changed, 424 insertions(+), 87 deletions(-) create mode 100644 argilla-frontend/v1/domain/entities/__mocks__/dataset/mocks.ts create mode 100644 argilla-frontend/v1/domain/entities/dataset/Dataset.test.ts diff --git a/argilla-frontend/components/features/annotation/settings/SettingsInfo.vue b/argilla-frontend/components/features/annotation/settings/SettingsInfo.vue index 64cdfe4c35..4176a3e404 100644 --- a/argilla-frontend/components/features/annotation/settings/SettingsInfo.vue +++ b/argilla-frontend/components/features/annotation/settings/SettingsInfo.vue @@ -23,15 +23,15 @@
-
-

+

+
+ + +
+ + + + + +
+ +

- +
+
+ + +
@@ -97,8 +130,11 @@ export default { }, }, methods: { - onSubmit() { - this.update(this.settings.dataset); + onSubmitDatasetGuidelines() { + this.update(this.settings.dataset, "guidelines"); + }, + onSubmitDatasetTaskMinimumResponse() { + this.update(this.settings.dataset, "distribution"); }, }, setup() { @@ -169,11 +205,12 @@ export default { color: $black-87; } - & input { + & input, + &__input--read-only { display: flex; flex-direction: row; align-items: center; - width: auto; + width: 80px; height: 24px; padding: $base-space * 2; background: palette(white); @@ -184,9 +221,16 @@ export default { border: 1px solid $primary-color; } } + &__input { + &--read-only { + background: $black-4; + border: 1px solid $black-20; + opacity: 0.6; + } + } } .info-icon { - color: $black-87; + color: $black-37; margin-right: $base-space * 2; &[data-title] { position: relative; diff --git a/argilla-frontend/components/features/annotation/settings/SettingsInfoReadOnly.vue b/argilla-frontend/components/features/annotation/settings/SettingsInfoReadOnly.vue index 221ed19648..0b74ba23d9 100644 --- a/argilla-frontend/components/features/annotation/settings/SettingsInfoReadOnly.vue +++ b/argilla-frontend/components/features/annotation/settings/SettingsInfoReadOnly.vue @@ -20,6 +20,23 @@
+
+

+ +
+
+

diff --git a/argilla-frontend/components/features/annotation/settings/useSettingInfoViewModel.ts b/argilla-frontend/components/features/annotation/settings/useSettingInfoViewModel.ts index a97c24b115..cbfefb6a90 100644 --- a/argilla-frontend/components/features/annotation/settings/useSettingInfoViewModel.ts +++ b/argilla-frontend/components/features/annotation/settings/useSettingInfoViewModel.ts @@ -5,20 +5,18 @@ import { Dataset } from "~/v1/domain/entities/dataset/Dataset"; export const useSettingInfoViewModel = () => { const updateDatasetSettingUseCase = useResolve(UpdateDatasetSettingUseCase); - const restore = (dataset: Dataset) => { - dataset.restore(); - }; - - const update = async (dataset: Dataset) => { + const update = async ( + dataset: Dataset, + part: "guidelines" | "metadata" | "distribution" + ) => { try { - await updateDatasetSettingUseCase.execute(dataset); + await updateDatasetSettingUseCase.execute(dataset, part); } catch (error) { // TODO } }; return { - restore, update, }; }; diff --git a/argilla-frontend/components/features/annotation/settings/useSettingsMetadataViewModel.ts b/argilla-frontend/components/features/annotation/settings/useSettingsMetadataViewModel.ts index da5ee37dc9..2cafd6ec7f 100644 --- a/argilla-frontend/components/features/annotation/settings/useSettingsMetadataViewModel.ts +++ b/argilla-frontend/components/features/annotation/settings/useSettingsMetadataViewModel.ts @@ -17,7 +17,7 @@ export const useSettingsMetadataViewModel = () => { }; const updateDataset = async (dataset: Dataset) => { - await updateDatasetSetting.execute(dataset); + await updateDatasetSetting.execute(dataset, "metadata"); }; return { diff --git a/argilla-frontend/v1/domain/entities/__mocks__/dataset/mocks.ts b/argilla-frontend/v1/domain/entities/__mocks__/dataset/mocks.ts new file mode 100644 index 0000000000..b3a19fe9f2 --- /dev/null +++ b/argilla-frontend/v1/domain/entities/__mocks__/dataset/mocks.ts @@ -0,0 +1,20 @@ +import { Dataset } from "../../dataset/Dataset"; + +export const createEmptyDataset = () => { + return new Dataset( + "FAKE_ID", + "FAKE_NAME", + "FAKE_GUIDELINES", + "ready", + "FAKE_WORKSPACE_ID", + "FAKE_WORKSPACE_NAME", + false, + { + strategy: "FAKE", + minSubmitted: 1, + }, + "", + "", + "" + ); +}; diff --git a/argilla-frontend/v1/domain/entities/dataset/Dataset.test.ts b/argilla-frontend/v1/domain/entities/dataset/Dataset.test.ts new file mode 100644 index 0000000000..1228cdb77b --- /dev/null +++ b/argilla-frontend/v1/domain/entities/dataset/Dataset.test.ts @@ -0,0 +1,169 @@ +import { createEmptyDataset } from "../__mocks__/dataset/mocks"; + +describe("Dataset", () => { + describe("validate should", () => { + test("return message when the guidelines is empty", () => { + const dataset = createEmptyDataset(); + + dataset.guidelines = ""; + + expect(dataset.validate().guidelines).toEqual([ + "This field is required.", + ]); + }); + + test("return a empty array when the guidelines is empty", () => { + const dataset = createEmptyDataset(); + + dataset.guidelines = "FAKE"; + + expect(dataset.validate().guidelines).toHaveLength(0); + }); + }); + + describe("isValidGuidelines should", () => { + test("be false when the guidelines is empty", () => { + const dataset = createEmptyDataset(); + + dataset.guidelines = ""; + + expect(dataset.isValidGuidelines).toBeFalsy(); + }); + + test("be true when the guidelines is empty", () => { + const dataset = createEmptyDataset(); + + dataset.guidelines = "FAKE"; + + expect(dataset.isValidGuidelines).toBeTruthy(); + }); + }); + + describe("isValidDistribution should", () => { + test("be false when the min submitted is empty", () => { + const dataset = createEmptyDataset(); + + dataset.distribution.minSubmitted = undefined; + + expect(dataset.isValidDistribution).toBeFalsy(); + }); + + test("be true when the min submitted is lower than 0", () => { + const dataset = createEmptyDataset(); + + dataset.distribution.minSubmitted = -1; + + expect(dataset.isValidDistribution).toBeFalsy(); + }); + + test("be true when the min submitted is lower than 0", () => { + const dataset = createEmptyDataset(); + + dataset.distribution.minSubmitted = 20; + + expect(dataset.isValidDistribution).toBeTruthy(); + }); + }); + + describe("restore", () => { + describe("restoreDistribution should", () => { + test("restore only the distribution based on original values", () => { + const dataset = createEmptyDataset(); + dataset.distribution.minSubmitted = 20; + + expect(dataset.distribution).not.toEqual(dataset.original.distribution); + + dataset.restoreDistribution(); + + expect(dataset.distribution).toEqual(dataset.original.distribution); + }); + }); + + describe("restoreMetadata should", () => { + test("restore only the metadata info based on original values", () => { + const dataset = createEmptyDataset(); + dataset.allowExtraMetadata = true; + + expect(dataset.allowExtraMetadata).not.toEqual( + dataset.original.allowExtraMetadata + ); + + dataset.restoreMetadata(); + + expect(dataset.allowExtraMetadata).toEqual( + dataset.original.allowExtraMetadata + ); + }); + }); + + describe("restoreGuidelines should", () => { + test("restore only the guidelines based on original values", () => { + const dataset = createEmptyDataset(); + dataset.guidelines = "NEW GUIDELINES"; + + expect(dataset.guidelines).not.toEqual(dataset.original.guidelines); + + dataset.restoreGuidelines(); + + expect(dataset.guidelines).toEqual(dataset.original.guidelines); + }); + }); + }); + + describe("update should", () => { + test("update just the guidelines", () => { + const dataset = createEmptyDataset(); + dataset.guidelines = "NEW GUIDELINES"; + + expect(dataset.guidelines).not.toEqual(dataset.original.guidelines); + + dataset.update("TODAY", "guidelines"); + + expect(dataset.guidelines).toEqual(dataset.original.guidelines); + expect(dataset.distribution.minSubmitted).toBe( + dataset.original.distribution.minSubmitted + ); + expect(dataset.allowExtraMetadata).toBe( + dataset.original.allowExtraMetadata + ); + }); + + test("update just the task distribution", () => { + const dataset = createEmptyDataset(); + dataset.distribution.minSubmitted = 20; + + expect(dataset.distribution.minSubmitted).not.toEqual( + dataset.original.distribution.minSubmitted + ); + + dataset.update("TODAY", "distribution"); + + expect(dataset.distribution.minSubmitted).toEqual( + dataset.original.distribution.minSubmitted + ); + expect(dataset.guidelines).toBe(dataset.original.guidelines); + expect(dataset.allowExtraMetadata).toBe( + dataset.original.allowExtraMetadata + ); + }); + + test("update just the metadata", () => { + const dataset = createEmptyDataset(); + dataset.allowExtraMetadata = true; + + expect(dataset.allowExtraMetadata).not.toEqual( + dataset.original.allowExtraMetadata + ); + + dataset.update("TODAY", "metadata"); + + expect(dataset.allowExtraMetadata).toEqual( + dataset.original.allowExtraMetadata + ); + expect(dataset.guidelines).toBe(dataset.original.guidelines); + expect(dataset.distribution.minSubmitted).toBe( + dataset.original.distribution.minSubmitted + ); + }); + }); +}); diff --git a/argilla-frontend/v1/domain/entities/dataset/Dataset.ts b/argilla-frontend/v1/domain/entities/dataset/Dataset.ts index 7c13d376b3..15103f1dd6 100644 --- a/argilla-frontend/v1/domain/entities/dataset/Dataset.ts +++ b/argilla-frontend/v1/domain/entities/dataset/Dataset.ts @@ -35,29 +35,96 @@ export class Dataset { return this.workspaceName; } - public get isModified(): boolean { + get isModified(): boolean { + return ( + this.isModifiedGuidelines || + this.isModifiedExtraMetadata || + this.isModifiedTaskDistribution + ); + } + + get isModifiedGuidelines(): boolean { + return this.guidelines !== this.original.guidelines; + } + + get isModifiedExtraMetadata(): boolean { + return this.allowExtraMetadata !== this.original.allowExtraMetadata; + } + + get isModifiedTaskDistribution(): boolean { return ( - this.guidelines !== this.original.guidelines || - this.allowExtraMetadata !== this.original.allowExtraMetadata || JSON.stringify(this.distribution) !== - JSON.stringify(this.original.distribution) + JSON.stringify(this.original.distribution) ); } restore() { + this.restoreGuidelines(); + this.restoreMetadata(); + this.restoreDistribution(); + } + + restoreGuidelines() { this.guidelines = this.original.guidelines; + } + + restoreMetadata() { this.allowExtraMetadata = this.original.allowExtraMetadata; + } + + restoreDistribution() { this.distribution = { ...this.original.distribution, }; } - update(when: string) { - this.initializeOriginal(); + update(when: string, part: "guidelines" | "metadata" | "distribution") { + this.original = { + ...this.original, + }; + + if (part === "guidelines") { + this.original.guidelines = this.guidelines; + } + + if (part === "metadata") { + this.original.allowExtraMetadata = this.allowExtraMetadata; + } + + if (part === "distribution") { + this.original.distribution = { + ...this.distribution, + }; + } this.updatedAt = when; } + validate(): Record<"guidelines" | "distribution", string[]> { + const validations: Record<"guidelines" | "distribution", string[]> = { + guidelines: [], + distribution: [], + }; + + if (this.guidelines?.trim().length < 1) { + validations.guidelines.push("This field is required."); + } + + if (!this.distribution.minSubmitted || this.distribution.minSubmitted < 1) { + validations.distribution.push("This field is required."); + } + + return validations; + } + + get isValidGuidelines() { + return this.validate().guidelines.length === 0; + } + + get isValidDistribution() { + return this.validate().distribution.length === 0; + } + private initializeOriginal() { this.original = { guidelines: this.guidelines, diff --git a/argilla-frontend/v1/domain/services/IDatasetRepository.ts b/argilla-frontend/v1/domain/services/IDatasetRepository.ts index ec0d2ffb64..7389aefb3d 100644 --- a/argilla-frontend/v1/domain/services/IDatasetRepository.ts +++ b/argilla-frontend/v1/domain/services/IDatasetRepository.ts @@ -5,6 +5,6 @@ export interface IDatasetRepository { getById(id: string): Promise; getAll(): Promise; delete(datasetId: string); - update({ id, allowExtraMetadata, guidelines }: Dataset); + update(dataset: Partial): Promise<{ when: string }>; getProgress(datasetId: string): Promise; } diff --git a/argilla-frontend/v1/domain/usecases/dataset-setting/update-dataset-setting-use-case.ts b/argilla-frontend/v1/domain/usecases/dataset-setting/update-dataset-setting-use-case.ts index b7fe5187df..8f8258123d 100644 --- a/argilla-frontend/v1/domain/usecases/dataset-setting/update-dataset-setting-use-case.ts +++ b/argilla-frontend/v1/domain/usecases/dataset-setting/update-dataset-setting-use-case.ts @@ -4,9 +4,35 @@ import { IDatasetRepository } from "../../services/IDatasetRepository"; export class UpdateDatasetSettingUseCase { constructor(private readonly datasetRepository: IDatasetRepository) {} - async execute(dataset: Dataset) { - const response = await this.datasetRepository.update(dataset); + async execute( + dataset: Dataset, + part: "guidelines" | "metadata" | "distribution" + ) { + const response = await this.update(dataset, part); - dataset.update(response.when); + dataset.update(response.when, part); + } + + private update( + dataset: Dataset, + part: "guidelines" | "metadata" | "distribution" + ) { + if (part === "guidelines") + return this.datasetRepository.update({ + id: dataset.id, + guidelines: dataset.guidelines, + }); + + if (part === "metadata") + return this.datasetRepository.update({ + id: dataset.id, + allowExtraMetadata: dataset.allowExtraMetadata, + }); + + if (part === "distribution") + return this.datasetRepository.update({ + id: dataset.id, + distribution: dataset.distribution, + }); } } diff --git a/argilla-frontend/v1/infrastructure/repositories/DatasetRepository.ts b/argilla-frontend/v1/infrastructure/repositories/DatasetRepository.ts index 833384e227..506578384c 100644 --- a/argilla-frontend/v1/infrastructure/repositories/DatasetRepository.ts +++ b/argilla-frontend/v1/infrastructure/repositories/DatasetRepository.ts @@ -74,15 +74,23 @@ export class DatasetRepository implements IDatasetRepository { return [...feedbackDatasets]; } - async update({ id, allowExtraMetadata, guidelines, distribution }: Dataset) { - const request: BackendUpdateDataset = { - allow_extra_metadata: allowExtraMetadata, - guidelines: guidelines?.trim() !== "" ? guidelines.trim() : null, - distribution: { - strategy: distribution.strategy, - min_submitted: distribution.minSubmitted, - }, - }; + async update({ id, ...dataset }: Partial) { + const request: Partial = {}; + + if ("allowExtraMetadata" in dataset) { + request.allow_extra_metadata = dataset.allowExtraMetadata; + } + + if ("guidelines" in dataset) { + request.guidelines = dataset.guidelines?.trim() ?? null; + } + + if ("distribution" in dataset) { + request.distribution = { + strategy: dataset.distribution.strategy, + min_submitted: dataset.distribution.minSubmitted, + }; + } try { const { data } = diff --git a/argilla-frontend/v1/infrastructure/repositories/RecordRepository.ts b/argilla-frontend/v1/infrastructure/repositories/RecordRepository.ts index 871282d9e7..356d0547ae 100644 --- a/argilla-frontend/v1/infrastructure/repositories/RecordRepository.ts +++ b/argilla-frontend/v1/infrastructure/repositories/RecordRepository.ts @@ -212,8 +212,31 @@ export class RecordRepository { const body: BackendAdvanceSearchQuery = { query: {}, + filters: { + and: [ + { + type: "terms", + scope: { + entity: "response", + property: "status", + }, + values: [status], + }, + ], + }, }; + if (status === "pending") { + body.filters.and.push({ + type: "terms", + scope: { + entity: "record", + property: "status", + }, + values: [status], + }); + } + if (isFilteringBySimilarity) { body.query.vector = { name: similaritySearch.vectorName, @@ -232,40 +255,6 @@ export class RecordRepository { }; } - body.filters = { - and: [ - { - type: "terms", - scope: { - entity: "response", - property: "status", - }, - values: [status], - }, - ], - }; - - if (status === "pending") { - body.filters.and.push({ - type: "terms", - scope: { - entity: "record", - property: "status", - }, - values: ["pending"], - }); - } - - if ( - isFilteringByMetadata || - isFilteringByResponse || - isFilteringBySuggestion - ) { - body.filters = { - and: [], - }; - } - if (isFilteringByMetadata) { metadata.value.forEach((m) => { const range = m.value as RangeValue; @@ -435,7 +424,7 @@ export class RecordRepository { }); } - const params = this.createParams(from, many, status); + const params = this.createParams(from, many); const { data } = await this.axios.post< ResponseWithTotal @@ -500,7 +489,7 @@ export class RecordRepository { }; } - private createParams(fromRecord: number, howMany: number, status: string) { + private createParams(fromRecord: number, howMany: number) { const offset = `${fromRecord - 1}`; const params = new URLSearchParams(); @@ -508,7 +497,6 @@ export class RecordRepository { params.append("include", "suggestions"); params.append("offset", offset); params.append("limit", howMany.toString()); - params.append("response_status", status); return params; } diff --git a/argilla-frontend/v1/infrastructure/types/record.ts b/argilla-frontend/v1/infrastructure/types/record.ts index dab0cb170c..e9465f9617 100644 --- a/argilla-frontend/v1/infrastructure/types/record.ts +++ b/argilla-frontend/v1/infrastructure/types/record.ts @@ -84,7 +84,7 @@ export interface BackendAdvanceSearchQuery { field?: string; }; }; - filters?: { + filters: { and: AndFilterBackendSearchQuery[]; }; sort?: BackendSort[];