Hotfix/flaw labels v2 review fixes - #830
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughFlaw 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. ChangesFlaw label updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 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 -->
|
There was a problem hiding this comment.
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
📒 Files selected for processing (9)
CHANGELOG.mdsrc/components/FlawLabels/FlawLabelsTable.vuesrc/components/FlawLabels/__tests__/FlawLabelsTable.spec.tssrc/components/IssueQueue/IssueQueueItem.vuesrc/composables/__tests__/useFlawLabels.spec.tssrc/composables/useFlawLabels.tssrc/composables/useFlawModel.tssrc/services/LabelsService.tssrc/services/__tests__/LabelsService.spec.ts
| 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; |
There was a problem hiding this comment.
🗄️ 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.
There was a problem hiding this comment.
I think this makes sense to address for consistency
There was a problem hiding this comment.
🧩 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 | sortLength 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.tsLength 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.
There was a problem hiding this comment.
🧩 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.tsLength 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.uuidfor persisted labels. - Use a serialized
contributorandnamepair 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.
| let hasErrors = false; | ||
| settled.forEach((result, index) => { | ||
| if (result.status === 'fulfilled') { | ||
| operations[index].onSuccess(); | ||
| } else { | ||
| hasErrors = true; | ||
| } | ||
| }); | ||
|
|
||
| return { hasErrors }; |
There was a problem hiding this comment.
🗄️ 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 withhasErrors.src/composables/useFlawModel.ts#L331-L340: merge only failed local labels intoupdatedFlaw.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.
There was a problem hiding this comment.
🧩 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.tsLength 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:
- Create label
Asucceeds. - Update label
Bfails. getFlaw()returnsAwith its UUID.- The
hasErrorsbranch replaces all labels with the pre-save local labels. - Local label
Ahas 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
🧩 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.tsLength 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
left a comment
There was a problem hiding this comment.
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>
[OSIDB-ID] Fix flaw label pending-state and key-collision bugs
Checklist:
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:handleUpdateLabelno longer marks a still-unsaved (newLabels) label asupdated, preventing a redundantupdateLabelrequest that was rejected for missing a uuid and showeda spurious error toast.
useFlawLabels.ts:updateLabels()now tracks per-operation success individually and only clearsthe
newLabels/updatedLabels/deletedLabelsentries that actually succeeded, returning{ hasErrors }instead of the rawPromise.allSettledarray.useFlawModel.ts: consumes{ hasErrors }fromupdateLabels()and preserves the localflaw.value.labelsreference across the post-save refetch when there were errors, so failed edits stayqueued for retry instead of being wiped out (mirrors the existing
affectshandling in the samefunction).
IssueQueueItem.vue: badgev-fornow keys onname+contributorinstead ofnamealone,since two labels can share a name with different contributors.
LabelsService.ts: extracted the duplicated missing-uuid check inupdateLabel/deleteLabelinto a shared
assertLabelUuidhelper, dropping the unnecessaryPromise.reject().catch()indirection.LabelsService.spec.ts: replaced the blanketvi.mock('@/composables/service-helpers')(whichmade
createCatchHandlersilently returnundefined, letting rejection tests pass via.catch(undefined)passthrough rather than exercising the real handler) with a factory mock thatmirrors the real throw-by-default behavior, and added assertions that the handler is actually invoked.
FlawLabelsTable.spec.tsanduseFlawLabels.spec.tscovering the fixedscenarios.
Considerations:
/osidb/api/v2/flaws/{uuid}/labelsendpoint used byLabelsService.tsisn't yet reflected inthe 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.
labelsRecordkeying (bynameonly) inuseFlawLabels.ts— only patched thesafe symptom (Vue key) in
IssueQueueItem.vue, since I couldn't confirm same-name/different-contributorlabels are actually possible on a real flaw.