Show label hierarchy paths in label previews#379
Conversation
…path-improvements
Add label path support to SearchField-selected label pickers and enable path-aware label payloads across review, import, and search filter surfaces.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
level09
left a comment
There was a problem hiding this comment.
Nice work, the new picker and the path chips are a good direction. Two things break real pages, and one ordering note:
- The cards (bulletin / incident / actor profiles) are also used on the export dashboard, deduplication and import log pages. Those pages don't load
LabelPath.jsor registerLabelPathList, so labels will show empty there. - Same problem on the activity page with
label-search-field(details inline). - Merge order: the Arabic data (
title_ar/path_arin mode 2) comes from PR #369, so that one should go in first, then re-test Arabic here.
| {{ label.title }} | ||
| </v-chip> | ||
| </div> | ||
| <label-path-list :labels="bulletin.labels"></label-path-list> |
There was a problem hiding this comment.
This card is also used on pages not touched by this PR (export dashboard, deduplication, import log). They don't register LabelPathList, so labels there render empty. Registering it inside the card itself (components: { LabelPathList }) protects every page, then those 3 pages just need the script tag.
| app.component('ActivitySearchBox', ActivitySearchBox); | ||
|
|
||
| app.component('SearchField', SearchField); | ||
| app.component('LabelPathList', LabelPathList); |
There was a problem hiding this comment.
This page also renders the bulletin/actor/incident search boxes, which now use label-search-field, but it's not loaded or registered here, so the label filters on this page will be empty.
| } | ||
| return window.LabelPathUtils.chipParts(item, this.selectedDuplicateLeaves); | ||
| }, | ||
| debouncedSearch: debounce(function (search) { |
There was a problem hiding this comment.
I think you can delete this override and labelQueryParams: every caller already sends mode: 2, so a default value for the api prop is enough and the base SearchField does the rest.
| return this.parentSegments(label, segments, 'ar'); | ||
| }, | ||
|
|
||
| parentSegments(label, segments = [], lang = this.primaryLang()) { |
There was a problem hiding this comment.
The backend path never includes the label itself, so this trimming never triggers on real data. Except one bad case: a child label with the same title as its parent (that's allowed) would lose its path. Safer to remove it.
| const english = this.splitPath(label.path); | ||
| if (lang !== 'ar') return this.parentSegments(label, english, 'en'); | ||
|
|
||
| const arabic = this.splitPath(label.path_ar || label.path_tr); |
There was a problem hiding this comment.
title_tr / path_tr don't exist on labels (that's the lookups model), these fallbacks can go.
| @@ -0,0 +1,343 @@ | |||
| const LabelPathUtils = { | |||
| genericLeaves: new Set(['other', 'unknown', 'أخرى', 'آخر', 'غير معروف', 'غير معروفة']), | |||
There was a problem hiding this comment.
This word list will drift from real data ("misc", "N/A", other Arabic words...). The duplicate-leaf check already covers the ambiguous case, I'd drop the list.
| > | ||
| <v-card-text class="pa-3"> | ||
| <div class="text-overline font-weight-bold text-medium-emphasis mb-2"> | ||
| LABEL PATH |
There was a problem hiding this comment.
Hardcoded English, same for "+N more" below. Should go through translations.
Summary
Labels can be nested, but pickers and chips only showed the leaf title, so same-named or generic labels (e.g. "Other") under different parents were indistinguishable. This adds path-aware label display:
LabelSearchField(extendsSearchField) — labels-only autocomplete using the compactmode=2payload, showing parent path as a subtitle and a collapsed path on selected chips when the leaf is ambiguous.LabelPath.js— path utils +LabelPathChip/LabelPathTrail/LabelPathList, replacing flat label chips on bulletin/incident/actor cards and profiles. RTL-aware.search-field→label-search-fieldeverywhere labels/ver_labels are picked (bulletins, actors, incidents, review dialog, media import).Note: card/profile views need a
pathfield added to their serializers to actually show hierarchy — landing in a separate follow-up PR. Until then those views keep showing flat leaf chips.Also: closes the incident drawer when editing an incident, to match bulletins/actors.
How to Test