feat: added used preset names to query settings#2504
Conversation
WalkthroughThe PR adds preset labeling metadata and category preset tracking throughout the system. Three new optional fields are added to the CaseQuery schema with corresponding JSON schema definitions. Frontend UI components track selected quick preset labels and category preset labels, propagated through store state and submitted with queries. Export functionality in PDF and DOCX formats now displays preset labels alongside existing metadata. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Areas requiring extra attention during review:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
deps-report 🔍Commit scanned: e46dd0e Vulnerable dependencies4 dependencies have vulnerabilities 😱
Outdated dependencies49 outdated dependencies found (including 17 outdated major versions)😢
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/variants/stores/variantQuery.js (1)
635-678: New preset label state is not reset in$reset(state leak across cases)
$resetclears many store fields but leavesselectedQuickPresetLabel,selectedQuickPresetLabelVersion, andcategoryPresetLabelsuntouched. This can leak preset label metadata from a previous case/query into subsequent ones, affecting both UI and exported metadata.Recommend resetting these alongside the other query‑related fields:
queryLogs.value = null quickPresets.value = null + selectedQuickPresetLabel.value = null + selectedQuickPresetLabelVersion.value = 1 + categoryPresetLabels.value = null categoryPresets.value = { inheritance: null, frequency: null, impact: null, quality: null, chromosomes: null, flagsetc: null, }
🧹 Nitpick comments (2)
backend/variants/query_schemas.py (1)
486-491: Consider coupling version with label presence.The
_quick_preset_label_versionis written independently of_quick_preset_label. Since the version is meaningless without a label, consider conditionally writing it only when the label is also present:# Preserve the quick preset label and version if present if query._quick_preset_label is not None: result["_quick_preset_label"] = query._quick_preset_label - if query._quick_preset_label_version is not None: - result["_quick_preset_label_version"] = query._quick_preset_label_version + if query._quick_preset_label_version is not None: + result["_quick_preset_label_version"] = query._quick_preset_label_version if query._category_preset_labels is not None: result["_category_preset_labels"] = query._category_preset_labelsbackend/variants/views/api/export.py (1)
484-499: Consider extracting duplicated preset display mappings.The
preset_orderandpreset_display_namesare duplicated between PDF and DOCX export sections. Consider extracting these to module-level constants alongside existing ones likeFREQUENCY_DATABASES:# Near line 1271 with other constants CATEGORY_PRESET_ORDER = [ "inheritance", "frequency", "impact", "quality", "chromosomes", "flagsetc", ] CATEGORY_PRESET_DISPLAY_NAMES = { "inheritance": "Inheritance", "frequency": "Frequency", "impact": "Impact", "quality": "Quality", "chromosomes": "Chromosomes", "flagsetc": "Flags etc.", }Also applies to: 2445-2453
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/variants/query_schemas.py(2 hunks)backend/variants/schemas/case-query-v1.json(1 hunks)backend/variants/views/api/export.py(2 hunks)frontend/src/variants/components/FilterForm/QuickPresets.vue(7 hunks)frontend/src/variants/components/FilterResultsTable.vue(2 hunks)frontend/src/variants/stores/variantQuery.js(3 hunks)frontend/src/variants/stores/variantResultSet.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-10-09T06:48:09.413Z
Learnt from: holtgrewe
Repo: varfish-org/varfish-server PR: 1957
File: frontend/src/seqvars/queries/seqvarResultRow.ts:110-112
Timestamp: 2024-10-09T06:48:09.413Z
Learning: In `frontend/src/seqvars/queries/seqvarResultRow.ts`, within the `useSeqvarResultRowRetrieveQueries` function, using `toValue(resultRowUuids)?.length` in the `enabled` function is necessary for transparent access into refs or getters.
Applied to files:
frontend/src/variants/stores/variantResultSet.ts
📚 Learning: 2024-10-10T10:50:55.833Z
Learnt from: holtgrewe
Repo: varfish-org/varfish-server PR: 1957
File: frontend/src/seqvars/views/SeqvarsQuery/SeqvarsQuery.vue:74-87
Timestamp: 2024-10-10T10:50:55.833Z
Learning: Currently, it's not necessary to implement removal logic for `openQueryUuids` in `frontend/src/seqvars/views/SeqvarsQuery/SeqvarsQuery.vue`.
Applied to files:
frontend/src/variants/stores/variantResultSet.tsfrontend/src/variants/components/FilterResultsTable.vue
📚 Learning: 2024-10-22T09:14:05.541Z
Learnt from: holtgrewe
Repo: varfish-org/varfish-server PR: 2046
File: frontend/src/seqvars/components/QueryEditor/QueryEditor.vue:481-496
Timestamp: 2024-10-22T09:14:05.541Z
Learning: In `frontend/src/seqvars/components/QueryEditor/QueryEditor.vue`, when comparing column settings in the `columnsMatchPreset` function, the order of columns is significant and should not be altered. Do not sort the column arrays before comparison because the order matters.
Applied to files:
frontend/src/variants/components/FilterForm/QuickPresets.vuefrontend/src/variants/components/FilterResultsTable.vue
📚 Learning: 2024-10-22T09:13:36.978Z
Learnt from: holtgrewe
Repo: varfish-org/varfish-server PR: 2046
File: frontend/src/seqvars/components/PresetsEditor/CategoryPresetsColumnsEditor.vue:304-304
Timestamp: 2024-10-22T09:13:36.978Z
Learning: In the `varfish-server` project using Vue.js version 3, the non-null assertion operator `!` is allowed in template expressions. Therefore, using `data.column_settings!.length` in `CategoryPresetsColumnsEditor.vue` is acceptable.
Applied to files:
frontend/src/variants/components/FilterResultsTable.vue
🧬 Code graph analysis (3)
frontend/src/variants/stores/variantResultSet.ts (2)
frontend/src/variants/stores/variantQuery.js (4)
variantClient(24-24)variantClient(121-121)variantClient(157-157)variantClient(169-169)backend/seqvars/protos/output_pb2.pyi (1)
query(343-344)
backend/variants/views/api/export.py (1)
backend/variants/views/api/__init__.py (5)
get(690-691)get(710-718)get(737-745)get(781-821)get(863-883)
frontend/src/variants/stores/variantQuery.js (1)
frontend/src/varfish/helpers.ts (1)
copy(249-251)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build-and-push-image
- GitHub Check: Python-Lint
- GitHub Check: Python-Test
- GitHub Check: Analyze (python)
- GitHub Check: Python-Test
- GitHub Check: Python-Lint
🔇 Additional comments (14)
frontend/src/variants/stores/variantResultSet.ts (1)
134-136: LGTM - Query loading for preset metadata access.The addition correctly fetches the query to access preset label settings. The implementation follows the existing pattern in
fetchResultSetViaRow(lines 178-182) where query retrieval is already performed.backend/variants/query_schemas.py (1)
330-333: New metadata fields for preset tracking look good.The underscore prefix convention appropriately marks these as internal metadata fields. The implementation correctly uses
Optionaltypes and reasonable defaults.frontend/src/variants/components/FilterResultsTable.vue (3)
586-608: Computed properties for preset display are well-implemented.Good fallback handling: returns the stored label, defaults to 'custom' for queries without preset metadata, and shows '-' when no query is loaded yet. The version property correctly returns
nullwhen unavailable.
620-719: Export functionality is robust with appropriate fallbacks.The implementation includes:
- Proper source detection (applied query vs current settings)
- Correct CSRF token handling
- Content-Disposition header parsing for filename
- Graceful JSON fallback when backend export fails
One minor observation: the endpoint URL is hardcoded. Consider extracting to a constant or using a route configuration if this pattern is used elsewhere.
787-805: Quick Preset badge UI is well-integrated.The badge displays the preset label with a dynamic tooltip that conditionally includes the version number. The styling is consistent with the existing UI patterns.
frontend/src/variants/components/FilterForm/QuickPresets.vue (3)
419-458: Category preset labels function is well-structured.The implementation correctly:
- Checks each category preset for non-custom values
- Falls back to the preset name if no label is available
- Cleans up by setting to
nullwhen no labels are present- Uses optional chaining for safe property access
346-354: Quick preset label synchronization is correct.The store's
selectedQuickPresetLabelis properly updated both when a matching preset is found and when falling back to 'custom'.
373-382: Handle custom preset selection correctly.Good handling of the custom case by clearing
categoryPresetLabelstonullwhen the user explicitly selects custom mode.backend/variants/views/api/export.py (2)
446-533: PDF export correctly renders preset metadata.The implementation properly:
- Defaults to "custom" when no quick preset label is set
- Formats version conditionally (only when present)
- Renders category presets in a consistent order with user-friendly names
- Only creates the category table when data exists
2427-2485: DOCX export preset additions are consistent with PDF implementation.The logic mirrors the PDF export changes, maintaining consistency across export formats. Header styling for the category presets table correctly applies bold formatting.
frontend/src/variants/stores/variantQuery.js (3)
296-301: State fields for preset labels look reasonableThe three new refs for
selectedQuickPresetLabel,selectedQuickPresetLabelVersion, andcategoryPresetLabelsare well‑documented and fit the existing store pattern. No functional issues here.
361-371: Query payload enrichment with preset metadata is correctUsing a deep copy via
copy(querySettings.value)and then attaching_quick_preset_label,_quick_preset_label_version, and_category_preset_labelsbefore submission is a clean approach and avoids mutatingquerySettings.value. This aligns with the new backend schema.
703-706: Exposing new preset label fields from the store is consistentExporting
selectedQuickPresetLabel,selectedQuickPresetLabelVersion, andcategoryPresetLabelsin the returned object is consistent with how other reactive state is surfaced and should integrate cleanly with the updated components.backend/variants/schemas/case-query-v1.json (1)
1504-1535: New preset label schema fields are well‑typed and consistentThe additions for
_quick_preset_label,_quick_preset_label_version, and_category_preset_labelsare JSON‑Schema‑correct, optional, and match the intended semantics (string/integer or null, with sensible defaults). Restricting_category_preset_labelsto the known category keys withadditionalProperties: falseis a good choice for keeping the public surface precise.
1 participant
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.