Skip to content

Hotfix/flaw labels v2 review fixes - #830

Open
superbuggy wants to merge 5 commits into
mainfrom
hotfix/flaw-labels-v2-review-fixes
Open

Hotfix/flaw labels v2 review fixes#830
superbuggy wants to merge 5 commits into
mainfrom
hotfix/flaw-labels-v2-review-fixes

Conversation

@superbuggy

@superbuggy superbuggy commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

[OSIDB-ID] Fix flaw label pending-state and key-collision bugs

Checklist:

  • Commits consolidated
  • Changelog updated
  • Test cases added/updated
  • Integration tests updated

Summary:

Follow-up fixes for the flaw labels v2 API migration (PR #828): editing an unsaved label queued a
redundant/erroring update, failed label writes silently lost their pending state instead of staying
queued for retry, and the issue queue could hit duplicate Vue keys for same-name labels with different
contributors.

Changes:

  • FlawLabelsTable.vue: handleUpdateLabel no longer marks a still-unsaved (newLabels) label as
    updated, preventing a redundant updateLabel request that was rejected for missing a uuid and showed
    a spurious error toast.
  • useFlawLabels.ts: updateLabels() now tracks per-operation success individually and only clears
    the newLabels/updatedLabels/deletedLabels entries that actually succeeded, returning { hasErrors } instead of the raw Promise.allSettled array.
  • useFlawModel.ts: consumes { hasErrors } from updateLabels() and preserves the local
    flaw.value.labels reference across the post-save refetch when there were errors, so failed edits stay
    queued for retry instead of being wiped out (mirrors the existing affects handling in the same
    function).
  • IssueQueueItem.vue: badge v-for now keys on name + contributor instead of name alone,
    since two labels can share a name with different contributors.
  • LabelsService.ts: extracted the duplicated missing-uuid check in updateLabel/deleteLabel
    into a shared assertLabelUuid helper, dropping the unnecessary Promise.reject().catch() indirection.
  • LabelsService.spec.ts: replaced the blanket vi.mock('@/composables/service-helpers') (which
    made createCatchHandler silently return undefined, letting rejection tests pass via
    .catch(undefined) passthrough rather than exercising the real handler) with a factory mock that
    mirrors the real throw-by-default behavior, and added assertions that the handler is actually invoked.
  • Added regression tests in FlawLabelsTable.spec.ts and useFlawLabels.spec.ts covering the fixed
    scenarios.

Considerations:

  • The v2 /osidb/api/v2/flaws/{uuid}/labels endpoint used by LabelsService.ts isn't yet reflected in
    the checked-in openapi-osidb.yml/generated client — only the mock server was hand-patched for it.
    Worth confirming the spec gets regenerated once the backend change is confirmed live, or CI could mask a
    real 404.
  • Did not change the labels Record keying (by name only) in useFlawLabels.ts — only patched the
    safe symptom (Vue key) in IssueQueueItem.vue, since I couldn't confirm same-name/different-contributor
    labels are actually possible on a real flaw.

Editing a still-unsaved new label queued both a create and an update
for it, and the update was rejected for lacking a uuid, producing a
spurious error toast. Separately, updateLabels() always triggered a
full flaw refetch and reset regardless of per-request outcome, so any
failed label create/update/delete silently lost its pending state
instead of staying queued for retry.
Two flaw labels can share a name while differing by contributor, so
keying the badge v-for on name alone let Vue reuse the wrong DOM node
between them on re-render.
Extract the duplicated missing-uuid check in updateLabel/deleteLabel
into a shared assertion helper, dropping the unnecessary
Promise.reject().catch() indirection. The spec's blanket
vi.mock('@/composables/service-helpers') made createCatchHandler
return undefined, so its rejection tests passed via .catch(undefined)
passthrough rather than exercising the real handler; mock it with a
factory that mirrors the real throw-by-default behavior instead.
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6c728292-e3f5-4b2e-acdb-5368d2a50edf

📥 Commits

Reviewing files that changed from the base of the PR and between ac406c6 and 2f1c0d6.

📒 Files selected for processing (3)
  • src/components/FlawLabels/FlawLabelsContributor.vue
  • src/components/FlawLabels/__tests__/FlawLabelsContributor.spec.ts
  • src/components/FlawLabels/__tests__/FlawLabelsTableEditingRow.spec.ts

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Fixed flaw label rendering when labels share the same name.
    • Prevented unsaved labels from triggering invalid update requests.
    • Improved label update and deletion error handling, including missing identifiers.
    • Preserved pending edits after partial failures so they can be retried.
    • Refreshed flaw details reliably after label changes while retaining local updates.
    • Corrected label edit persistence, contributor input handling, and duplicate badge handling.
    • Ensured successfully completed label changes are removed from pending edits.

Walkthrough

Flaw label operations now validate UUIDs, preserve failed edits after partial failures, and report update errors. Label editing persists contributor input and keeps new labels classified correctly. Issue-queue keys now distinguish labels with the same name.

Changes

Flaw label updates

Layer / File(s) Summary
Label service UUID validation
src/services/LabelsService.ts, src/services/__tests__/LabelsService.spec.ts
deleteLabel and updateLabel use shared UUID validation. Tests verify missing-UUID error handling.
Partial label update state
src/composables/useFlawLabels.ts, src/composables/useFlawModel.ts, src/composables/__tests__/useFlawLabels.spec.ts
updateLabels returns hasErrors, clears only successful operations, and preserves failed edits after the flaw refresh.
Label editing state
src/components/FlawLabels/FlawLabelsTable.vue, src/components/FlawLabels/__tests__/FlawLabelsTable.spec.ts, src/components/FlawLabels/FlawLabelsContributor.vue, src/components/FlawLabels/__tests__/*
New labels remain in newLabels during editing. Contributor input persists through blur and save.
Label rendering state
src/components/IssueQueue/IssueQueueItem.vue, CHANGELOG.md
Issue-queue keys include the contributor. The changelog records the label fixes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: c-valen

Sequence Diagram(s)

sequenceDiagram
  participant FlawModel
  participant FlawLabels
  participant LabelsService
  FlawModel->>FlawLabels: updateLabels()
  FlawLabels->>LabelsService: createLabel/updateLabel/deleteLabel
  LabelsService-->>FlawLabels: success or error
  FlawLabels-->>FlawModel: hasErrors and pending state
  FlawModel->>FlawModel: re-fetch flaw and preserve failed edits
``

</details>

<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->

<details>
<summary>🚥 Pre-merge checks | ✅ 4 | ❌ 1</summary>

### ❌ Failed checks (1 warning)

|     Check name     | Status     | Explanation                                                                           | Resolution                                                                         |
| :----------------: | :--------- | :------------------------------------------------------------------------------------ | :--------------------------------------------------------------------------------- |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |

<details>
<summary>✅ Passed checks (4 passed)</summary>

|         Check name         | Status   | Explanation                                                                                                                                          |
| :------------------------: | :------- | :--------------------------------------------------------------------------------------------------------------------------------------------------- |
|         Title check        | ✅ Passed | The title clearly identifies the flaw labels v2 review fixes and matches the primary changes in the pull request.                                    |
|      Description check     | ✅ Passed | The description includes the required checklist, summary, changes, and considerations, and it accurately notes the incomplete integration-test work. |
|     Linked Issues check    | ✅ Passed | Check skipped because no linked issues were found for this pull request.                                                                             |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request.                                                                             |

</details>

</details>

<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->

<details>
<summary>✨ Finishing Touches 💡 1</summary>

<!-- finishing_touch_suggestion:docstrings -->
<details>
<summary>📝 Generate docstrings 💡</summary>

- [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> Create stacked PR
- [ ] <!-- {"checkboxId": "3e1879ae-f29b-4d0d-8e06-d12b7ba33d98"} --> Commit on current branch

</details>
<details>
<summary>🧪 Generate unit tests (beta)</summary>

- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} -->   Create PR with unit tests
- [ ] <!-- {"checkboxId": "6ba7b810-9dad-11d1-80b4-00c04fd430c8", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} -->   Commit unit tests in branch `hotfix/flaw-labels-v2-review-fixes`

</details>

</details>

<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->

---

Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=RedHatProductSecurity/osim&utm_content=830)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

<details>
<summary>❤️ Share</summary>

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)
- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)
- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)
- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

</details>


<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>

<!-- tips_end -->
Loading

@superbuggy
superbuggy marked this pull request as ready for review August 11, 2026 15:46

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/composables/useFlawLabels.ts`:
- Around line 74-83: Update updateLabels in src/composables/useFlawLabels.ts
(lines 74-83) to return failed label identities or per-operation outcomes
alongside hasErrors. In useFlawModel.ts (lines 331-340), use that result to
merge only failed local labels into updatedFlaw.labels while preserving
refreshed server labels from fulfilled operations, including server-assigned
UUIDs.
- Around line 47-79: Use a stable label identity based on contributor plus name
for unsaved labels and UUID for persisted labels throughout the label state and
request preparation, replacing name-only indexing in the surrounding composable
flow. Before building operations, remove any label from updatedLabels when it is
marked for deletion so deletion takes precedence, and ensure successful deletion
clears the conflicting pending markers to prevent stale update retries.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 59e52f73-5d73-42c9-8f9f-8c4bca03351e

📥 Commits

Reviewing files that changed from the base of the PR and between 6b006c3 and ac406c6.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • src/components/FlawLabels/FlawLabelsTable.vue
  • src/components/FlawLabels/__tests__/FlawLabelsTable.spec.ts
  • src/components/IssueQueue/IssueQueueItem.vue
  • src/composables/__tests__/useFlawLabels.spec.ts
  • src/composables/useFlawLabels.ts
  • src/composables/useFlawModel.ts
  • src/services/LabelsService.ts
  • src/services/__tests__/LabelsService.spec.ts

Comment on lines +47 to +79
const operations: { onSuccess: () => void; request: Promise<unknown> }[] = [];
for (const newLabel of newLabels.value) {
requests.push(createLabel(flaw.value.uuid, labels.value[newLabel]));
operations.push({
request: createLabel(flaw.value.uuid, labels.value[newLabel]),
onSuccess: () => newLabels.value.delete(newLabel),
});
}

for (const updatedLabel of updatedLabels.value) {
requests.push(updateLabel(flaw.value.uuid, labels.value[updatedLabel]));
operations.push({
request: updateLabel(flaw.value.uuid, labels.value[updatedLabel]),
onSuccess: () => updatedLabels.value.delete(updatedLabel),
});
}

for (const deletedLabel of deletedLabels.value) {
requests.push(deleteLabel(flaw.value.uuid, labels.value[deletedLabel]));
operations.push({
request: deleteLabel(flaw.value.uuid, labels.value[deletedLabel]),
onSuccess: () => {
deletedLabels.value.delete(deletedLabel);
delete labels.value[deletedLabel];
},
});
}

return await Promise.allSettled(requests);
const settled = await Promise.allSettled(operations.map(({ request }) => request));

let hasErrors = false;
settled.forEach((result, index) => {
if (result.status === 'fulfilled') {
operations[index].onSuccess();
} else {
hasErrors = true;

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Use a stable label identity and resolve conflicting operations before requests.

src/components/IssueQueue/IssueQueueItem.vue line 111 shows that label.name is not unique. This code still indexes local labels and pending operations by name. A same-name label with another contributor can overwrite the first label or send an operation for the wrong label.

A persisted label can also remain in updatedLabels when the user deletes it. This code then sends PUT and DELETE requests concurrently. If DELETE succeeds and PUT fails, retrying the stale update passes undefined to updateLabel.

Use a stable key that includes the contributor for unsaved labels and the UUID for persisted labels. Give deletion precedence over updates and clear conflicting pending markers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/composables/useFlawLabels.ts` around lines 47 - 79, Use a stable label
identity based on contributor plus name for unsaved labels and UUID for
persisted labels throughout the label state and request preparation, replacing
name-only indexing in the surrounding composable flow. Before building
operations, remove any label from updatedLabels when it is marked for deletion
so deletion takes precedence, and ensure successful deletion clears the
conflicting pending markers to prevent stale update retries.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this makes sense to address for consistency

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/composables/useFlawLabels.ts --view expanded
rg -n -C 3 '\b(labels|newLabels|updatedLabels|deletedLabels|isNewLabel|isUpdatedLabel|isDeletedLabel|updateLabels)\b' src
rg -n -C 3 'ZodFlawLabelType|contributor|uuid' src/types src/components src/composables src/services
fd -t f -a 'FlawLabels|useFlawLabels|zodFlaw' src | sort

Length of output: 195334


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,180p' src/components/FlawLabels/FlawLabelsTable.vue
sed -n '1,180p' src/composables/__tests__/useFlawLabels.spec.ts
sed -n '1,180p' src/components/FlawLabels/__tests__/FlawLabelsTable.spec.ts
sed -n '200,240p' src/types/zodFlaw.ts

Length of output: 18143


@C-Valen, the focused review confirms the two failure paths. The table also uses label.name as its Vue key and edit selector.

  • Add one stable label identity helper.
  • Replace name-only state keys in the composable and table.
  • Move an unsaved label marker when an edit changes its name or contributor.
  • Make deletion remove conflicting update state.
  • Add regression tests for duplicate names and update-then-delete.

🧬 Code Graph Analysis Results

src/types/zodFlaw.ts:157

export type ZodFlawLabelType = NonNullable<ZodFlawType['labels']>[number];

src/services/LabelsService.ts:8-20

export async function fetchLabels() {
  try {
    const { data }: { data: PaginatedFlawLabelList } = await osidbFetch({
      method: 'get',
      url: '/osidb/api/v1/labels',
    });

    return data.results;
  } catch (error) {
    console.error('LabelService::fetchLabels() Error fetching labels', error);
    return [];
  }
}

src/services/LabelsService.ts:31-39

export async function createLabel(flawUUID: string, label: ZodFlawLabelType) {
  return osidbFetch({
    method: 'post',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels`,
    data: label,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} created.` }))
    .catch(createCatchHandler(`Error creating label ${label.name}`));
}

src/services/LabelsService.ts:41-50

export async function deleteLabel(flawUUID: string, label: ZodFlawLabelType) {
  assertLabelUuid(label, 'deleting');

  return osidbFetch({
    method: 'delete',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels/${label.uuid}`,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} deleted.` }))
    .catch(createCatchHandler(`Error deleting label ${label.name}`));
}

src/services/LabelsService.ts:52-62

export async function updateLabel(flawUUID: string, label: ZodFlawLabelType) {
  assertLabelUuid(label, 'updating');

  return osidbFetch({
    method: 'put',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels/${label.uuid}`,
    data: label,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} updated.` }))
    .catch(createCatchHandler(`Error updating label ${label.name}`));
}

src/composables/useFlaw.ts:38-46

export function useFlaw() {
  return {
    flaw,
    initialFlaw: readonly(initialFlaw),
    isFlawUpdated,
    resetFlaw,
    setFlaw,
  };
}

  • 📌 Create a pull request with these changes

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/components/FlawLabels src/composables --items all --type function --match 'Label|label'
rg -n -C 3 '\b(labels|newLabels|updatedLabels|deletedLabels)\b' src/components/FlawLabels src/composables --glob '*.ts' --glob '*.vue'

Length of output: 37462


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- label schema ---'
rg -n -C 12 'labels\s*:' src/types/zodFlaw.ts

printf '%s\n' '--- table handlers and template ---'
sed -n '1,180p' src/components/FlawLabels/FlawLabelsTable.vue

printf '%s\n' '--- composable test setup and affected tests ---'
sed -n '1,115p' src/composables/__tests__/useFlawLabels.spec.ts

printf '%s\n' '--- table test setup and affected tests ---'
sed -n '1,145p' src/components/FlawLabels/__tests__/FlawLabelsTable.spec.ts

printf '%s\n' '--- label service request contracts ---'
sed -n '1,90p' src/services/LabelsService.ts

Length of output: 19217


@C-Valen, the inspection confirms the finding.

uuid and contributor are optional. The current code uses label.name for the label record, pending sets, Vue keys, and edit state. handleDeleteLabel also leaves updatedLabels unchanged. This allows concurrent update and delete requests for one persisted label.

  • Add a collision-safe label-key helper.
  • Use label.uuid for persisted labels.
  • Use a serialized contributor and name pair for unsaved labels.
  • Replace all name-based label-state accesses in the composable and table.
  • Preserve the original key while editing an unsaved label. If its name or contributor changes, move its local state to the new key.
  • Clear the update marker when deletion starts. Also filter update operations against deleted keys before requests are built.
  • Clear both pending markers after a successful deletion.
  • Update existing key assertions and add targeted regressions.

🧬 Code Graph Analysis Results

src/types/zodFlaw.ts:157

export type ZodFlawLabelType = NonNullable<ZodFlawType['labels']>[number];

src/services/LabelsService.ts:8-20

fetchLabels() requests /osidb/api/v1/labels, returns data.results, and logs errors before returning an empty array.

src/services/LabelsService.ts:31-39

createLabel(flawUUID, label) posts the label to the flaw labels endpoint and applies success/error handlers.

src/services/LabelsService.ts:41-50

deleteLabel(flawUUID, label) validates the label UUID, deletes the label by UUID, and applies success/error handlers.

src/services/LabelsService.ts:52-62

updateLabel(flawUUID, label) validates the label UUID, updates the label by UUID, and applies success/error handlers.

src/composables/useFlaw.ts:38-46

useFlaw() returns the reactive flaw state along with read-only initial state and flaw mutation/reset helpers.


Failed to handle agent chat message. Please try again.

You are interacting with an AI system.

Comment on lines +74 to +83
let hasErrors = false;
settled.forEach((result, index) => {
if (result.status === 'fulfilled') {
operations[index].onSuccess();
} else {
hasErrors = true;
}
});

return { hasErrors };

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Preserve only failed local labels after a mixed result.

updateLabels removes pending markers for fulfilled requests but returns only hasErrors. useFlawModel then replaces all refreshed labels with the pre-save local collection when one request fails. A successful create can therefore disappear or remain without its server UUID when another label operation fails.

  • src/composables/useFlawLabels.ts#L74-L83: return the failed label identities or per-operation outcomes with hasErrors.
  • src/composables/useFlawModel.ts#L331-L340: merge only failed local labels into updatedFlaw.labels; retain refreshed server labels for fulfilled operations.
📍 Affects 2 files
  • src/composables/useFlawLabels.ts#L74-L83 (this comment)
  • src/composables/useFlawModel.ts#L331-L340
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/composables/useFlawLabels.ts` around lines 74 - 83, Update updateLabels
in src/composables/useFlawLabels.ts (lines 74-83) to return failed label
identities or per-operation outcomes alongside hasErrors. In useFlawModel.ts
(lines 331-340), use that result to merge only failed local labels into
updatedFlaw.labels while preserving refreshed server labels from fulfilled
operations, including server-assigned UUIDs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/composables/useFlawModel.ts --items all --type function --match 'update|label|Label' || true
rg -n -C 10 'updateLabels\s*\(|updatedFlaw\.labels|labels\.value|initialLabels' \
  src/composables/useFlawModel.ts src/composables/useFlawLabels.ts

Length of output: 9451


@C-Valen, confirmed. The value fields are usually not the main problem.

The UUID concern is valid. If one operation fails, useFlawModel.ts replaces all refetched labels with flaw.value.labels. If a different create operation succeeded, this replacement discards the server label and its server-assigned uuid.

For example:

  1. Create label A succeeds.
  2. Update label B fails.
  3. getFlaw() returns A with its UUID.
  4. The hasErrors branch replaces all labels with the pre-save local labels.
  5. Local label A has no UUID.

updateLabels() must return the failed label identities or operation outcomes. useFlawModel.ts must then merge only failed local labels into updatedFlaw.labels.

🔍🐇


🧬 Code Graph Analysis Results

src/types/zodFlaw.ts:157

export type ZodFlawLabelType = NonNullable<ZodFlawType['labels']>[number];

src/services/LabelsService.ts:31-39

export async function createLabel(flawUUID: string, label: ZodFlawLabelType) {
  return osidbFetch({
    method: 'post',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels`,
    data: label,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} created.` }))
    .catch(createCatchHandler(`Error creating label ${label.name}`));
}

src/services/LabelsService.ts:41-50

export async function deleteLabel(flawUUID: string, label: ZodFlawLabelType) {
  assertLabelUuid(label, 'deleting');

  return osidbFetch({
    method: 'delete',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels/${label.uuid}`,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} deleted.` }))
    .catch(createCatchHandler(`Error deleting label ${label.name}`));
}

src/services/LabelsService.ts:52-62

export async function updateLabel(flawUUID: string, label: ZodFlawLabelType) {
  assertLabelUuid(label, 'updating');

  return osidbFetch({
    method: 'put',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels/${label.uuid}`,
    data: label,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} updated.` }))
    .catch(createCatchHandler(`Error updating label ${label.name}`));
}

src/services/LabelsService.ts:8-20

export async function fetchLabels() {
  try {
    const { data }: { data: PaginatedFlawLabelList } = await osidbFetch({
      method: 'get',
      url: '/osidb/api/v1/labels',
    });

    return data.results;
  } catch (error) {
    console.error('LabelService::fetchLabels() Error fetching labels', error);
    return [];
  }
}

You are interacting with an AI system.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure about this, if presaved edits are correctly tracked, the save operation should, in theory, return back same values. However the UUID is an exception and sounds like a real concern.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/composables/useFlawModel.ts --items all --type function --match 'update|save|flaw|Label'
rg -n -C 12 'updateLabels\s*\(|hasErrors|updatedFlaw\.labels|flaw\.value\.labels' \
  src/composables/useFlawModel.ts

Length of output: 6760


@C-Valen, the current code confirms the UUID issue.

At Line 331, updateLabels() returns only hasErrors. At Line 334, the code refetches the flaw. At Line 339, if any operation fails, the code replaces all updatedFlaw.labels with flaw.value.labels.

This replacement affects successful operations too. A successful createLabel can exist in updatedFlaw.labels with its server UUID. The local label has no server UUID. The replacement removes that UUID.

The pending sets correctly retain failed operations for retry. The problem is that useFlawModel.ts cannot identify which local labels failed. updateLabels() must return the failed label names or per-operation outcomes. useFlawModel.ts must then merge only those failed local labels into the refetched labels.


🧬 Code Graph Analysis Results

src/types/zodFlaw.ts:157

export type ZodFlawLabelType = NonNullable<ZodFlawType['labels']>[number];

src/services/LabelsService.ts:31-39

export async function createLabel(flawUUID: string, label: ZodFlawLabelType) {
  return osidbFetch({
    method: 'post',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels`,
    data: label,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} created.` }))
    .catch(createCatchHandler(`Error creating label ${label.name}`));
}

src/services/LabelsService.ts:41-50

export async function deleteLabel(flawUUID: string, label: ZodFlawLabelType) {
  assertLabelUuid(label, 'deleting');

  return osidbFetch({
    method: 'delete',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels/${label.uuid}`,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} deleted.` }))
    .catch(createCatchHandler(`Error deleting label ${label.name}`));
}

src/services/LabelsService.ts:52-62

export async function updateLabel(flawUUID: string, label: ZodFlawLabelType) {
  assertLabelUuid(label, 'updating');

  return osidbFetch({
    method: 'put',
    url: `/osidb/api/v2/flaws/${flawUUID}/labels/${label.uuid}`,
    data: label,
  })
    .then(createSuccessHandler({ title: 'Success!', body: `Label ${label.name} updated.` }))
    .catch(createCatchHandler(`Error updating label ${label.name}`));
}

You are interacting with an AI system.

@C-Valen C-Valen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work on those fixes.
Holding the approval for considering the couple of coderabbit points + the additional bug discussed off thread.

FlawLabelsContributor only synced its local input back to the parent's
v-model when the field was cleared to empty (from the OSIDB-4014 fix),
never when arbitrary text was typed and not picked from the Jira user
suggestions. Editing just the contributor of an already-added label and
clicking the row's Save button then saw no field as changed and
silently emitted cancel, reverting the whole row to its previous value.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants