feat: add data correction section to settings page#3039
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Hello! Thank you for opening your first PR to npmx, @Hui-Hwoo! 🚀 Here’s what will happen next:
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChart-filter defaults are centralised and reset support is exposed in settings. A new data-correction section displays filter values, provides a conditional reset button, adds localisation and schema keys, and includes composable tests. ChangesChart filter reset
Sequence Diagram(s)sequenceDiagram
participant SettingsPage
participant useChartFilter
participant SettingsState
SettingsPage->>useChartFilter: read filter state
useChartFilter->>SettingsState: compare chartFilter with DEFAULT_CHART_FILTER
SettingsPage->>useChartFilter: click reset
useChartFilter->>SettingsState: assign default chart-filter copy
SettingsState-->>SettingsPage: update values and button state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
coderabbitai
Bot
added
the
007
gameroman
left a comment
gameroman
left a comment
There was a problem hiding this comment.
Please use the Pull Request template and resolve the conflicts
447a187 to
07f0dd5
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
gameroman
removed
the
007
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
i18n/locales/es-419.json (1)
15-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the regional overlay limited to genuine differences.
After
i18n/locales/es.jsonhas final translations, omit these keys fromes-419.jsonunless the Latin-American wording intentionally differs, so the variant inherits the base Spanish values.🤖 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 `@i18n/locales/es-419.json` around lines 15 - 16, Remove data_correction_reset and data_correction_description from the es-419 locale unless their Latin-American translations intentionally differ from the finalized es.json values, allowing the regional locale to inherit the base Spanish translations.Source: Learnings
🤖 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 `@i18n/locales/ar-EG.json`:
- Line 50: Replace every literal “EN TEXT TO REPLACE:” placeholder with accurate
localized translations for the relevant Settings strings. Update
i18n/locales/ar-EG.json at 50 and 74-75, ar.json at 93 and 122-123, az-AZ.json
at 140 and 173-174, bg-BG.json at 111 and 142-143, sr-Latn-RS.json at 144 and
191-192, ta-IN.json at 92 and 121-122, and te-IN.json at 69 and 89-90; preserve
the existing locale structure and ensure no English placeholders remain.
In `@i18n/locales/en-GB.json`:
- Around line 6-7: Replace every “EN TEXT TO REPLACE:” marker with finalized
localized copy: add English text for data_correction_reset and
data_correction_description in i18n/locales/en-GB.json; add Latin-American
Spanish or inherit the base locale in i18n/locales/es-419.json; add Austrian
German in i18n/locales/de-AT.json; and translate the section label plus reset
action and description at the specified locations in i18n/locales/de.json,
i18n/locales/es.json, i18n/locales/fr-FR.json, and i18n/locales/nl.json. Ensure
no unfinished marker remains.
---
Nitpick comments:
In `@i18n/locales/es-419.json`:
- Around line 15-16: Remove data_correction_reset and
data_correction_description from the es-419 locale unless their Latin-American
translations intentionally differ from the finalized es.json values, allowing
the regional locale to inherit the base Spanish translations.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 60af86c3-126b-408c-a940-db5c39f485f9
📒 Files selected for processing (38)
app/composables/useSettings.tsapp/pages/settings.vuei18n/locales/ar-EG.jsoni18n/locales/ar.jsoni18n/locales/az-AZ.jsoni18n/locales/bg-BG.jsoni18n/locales/bn-IN.jsoni18n/locales/cs-CZ.jsoni18n/locales/de-AT.jsoni18n/locales/de.jsoni18n/locales/en-GB.jsoni18n/locales/en.jsoni18n/locales/es-419.jsoni18n/locales/es.jsoni18n/locales/fr-FR.jsoni18n/locales/hi-IN.jsoni18n/locales/hu-HU.jsoni18n/locales/id-ID.jsoni18n/locales/it-IT.jsoni18n/locales/ja-JP.jsoni18n/locales/kn-IN.jsoni18n/locales/mr-IN.jsoni18n/locales/nb-NO.jsoni18n/locales/ne-NP.jsoni18n/locales/nl.jsoni18n/locales/pl-PL.jsoni18n/locales/pt-BR.jsoni18n/locales/pt-PT.jsoni18n/locales/ro-RO.jsoni18n/locales/ru-RU.jsoni18n/locales/sr-Latn-RS.jsoni18n/locales/ta-IN.jsoni18n/locales/te-IN.jsoni18n/locales/tr-TR.jsoni18n/locales/uk-UA.jsoni18n/locales/vi-VN.jsoni18n/locales/zh-CN.jsoni18n/locales/zh-TW.json
🚧 Files skipped from review as they are similar to previous changes (3)
- i18n/locales/en.json
- app/pages/settings.vue
- app/composables/useSettings.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/pages/settings.vue (1)
353-356: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalise the
anomaliesFixedlabel.{{ settings.chartFilter.anomaliesFixed }}will rendertrue/falsedirectly; use a translated label here instead of the raw boolean.🤖 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 `@app/pages/settings.vue` around lines 353 - 356, Update the anomaliesFixed display in the settings template to render a localized true/false label rather than the raw boolean value, using the existing translation helper and established translation keys.
🧹 Nitpick comments (1)
app/composables/useSettings.ts (1)
263-269: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider iterating over
DEFAULT_CHART_FILTERkeys forisDefault.The manual field-by-field comparison works correctly today, but if a new field is added to
chartFilterin the future,isDefaultmust be updated in lockstep — otherwise the reset button could be incorrectly enabled or disabled. Iterating overDEFAULT_CHART_FILTERkeys auto-includes future fields.♻️ Optional refactor
const isDefault = computed( - () => - settings.value.chartFilter.averageWindow === DEFAULT_CHART_FILTER.averageWindow && - settings.value.chartFilter.smoothingTau === DEFAULT_CHART_FILTER.smoothingTau && - settings.value.chartFilter.anomaliesFixed === DEFAULT_CHART_FILTER.anomaliesFixed && - settings.value.chartFilter.predictionPoints === DEFAULT_CHART_FILTER.predictionPoints, + () => + (Object.keys(DEFAULT_CHART_FILTER) as (keyof typeof DEFAULT_CHART_FILTER)[]).every( + key => settings.value.chartFilter[key] === DEFAULT_CHART_FILTER[key] + ), )🤖 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 `@app/composables/useSettings.ts` around lines 263 - 269, Update the isDefault computed property to iterate over the keys of DEFAULT_CHART_FILTER and compare each corresponding value in settings.value.chartFilter, replacing the manual field-by-field checks while preserving the current boolean result.
🤖 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.
Outside diff comments:
In `@app/pages/settings.vue`:
- Around line 353-356: Update the anomaliesFixed display in the settings
template to render a localized true/false label rather than the raw boolean
value, using the existing translation helper and established translation keys.
---
Nitpick comments:
In `@app/composables/useSettings.ts`:
- Around line 263-269: Update the isDefault computed property to iterate over
the keys of DEFAULT_CHART_FILTER and compare each corresponding value in
settings.value.chartFilter, replacing the manual field-by-field checks while
preserving the current boolean result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d24ecf0-9d7c-40d0-a6a9-bfa5d3252021
📒 Files selected for processing (3)
app/composables/useSettings.tsapp/pages/settings.vuetest/nuxt/composables/use-settings.spec.ts
There was a problem hiding this comment.
@Hui-Hwoo thanks for your PR :)
A few remarks:
- there is no chart modal anymore. Download charts are visible on the stats and compare pages
- for each correction variable, a tooltip should be used to explain what the variables are about (tooltip visible on hover of a i icon)
- values should have units when applicable (for example: average window is in days)
- I would replace "known anomalies true" with "Known anomalies correction enabled/disabled)
- The styles of the reset button need to match the application standards
- Update description to reference stats/compare pages instead of chart modal - Add info icon tooltips explaining each correction variable - Show units (points) for average window and prediction values - Display "Enabled"/"Disabled" instead of raw boolean for anomalies - Use size="sm" on reset button to match application standards
Fixes HTML validation error where div elements (from TooltipApp) were nested inside span elements, which is not permitted per HTML spec.
graphieros
added
the
007
3 participants
🔗 Linked issue
Closes #2536
🧭 Context
The download chart's data correction parameters (
averageWindow,smoothingTau,anomaliesFixed,predictionPoints) persist inlocalStoragebut are only accessible from the chart modal on package pages. Users who modified these settings may forget they did so, leading to confusing download number discrepancies (see #2510).📚 Description
Adds a new Data Correction section to the Settings page (
/settings) that displays the current values of all fourchartFilterparameters and provides a "Reset to defaults" button.Changes:
app/composables/useSettings.ts— Exports aDEFAULT_CHART_FILTERconstant (single source of truth for defaults), referenced byDEFAULT_SETTINGSapp/pages/settings.vue— Adds the Data Correction section with a computedisDefaultChartFiltercheck and aresetChartFilter()function. The reset button is disabled when all values are already at defaultsi18n/locales/en.json— Addssettings.sections.data_correction,settings.data_correction_reset, andsettings.data_correction_descriptioni18n/locales/*.json— Propagates the three new keys to all locale files with English placeholders