fix: Add fetching and caching of unknown grade weight and set TeltMee…#93
fix: Add fetching and caching of unknown grade weight and set TeltMee…#93Tureluurtje wants to merge 1 commit intoQkeleQ10:mainfrom
Conversation
… to false for grades with weight being 0
There was a problem hiding this comment.
Pull request overview
This PR updates the grade statistics pane to ensure grades with column weight 0 are excluded (TeltMee = false), including cases where the weight is initially unknown by fetching and caching grade-column info from the API.
Changes:
- Added a
yeargetter plus in-memory caching forgradesColumnInfolookups. - Introduced
#applyColumnWeightToGrade()to fetch missing column weights and setTeltMee = falsewhen weight is0. - Integrated the new weight application step into
toggleGrade()(plus substantial formatting changes).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const existingWeight = Number(grade.CijferKolom.Weging); | ||
| if (!Number.isNaN(existingWeight) && existingWeight === 0) { | ||
| grade.TeltMee = false; | ||
| return; | ||
| } | ||
|
|
||
| if (grade.CijferKolom.Weging != null) return; |
There was a problem hiding this comment.
Number(null) evaluates to 0, so when CijferKolom.Weging is null (unknown) this branch will incorrectly treat it as weight 0 and set grade.TeltMee = false. Guard the “weight is 0” logic with a non-null/undefined check (e.g., only consider it 0 when Weging != null), and apply the same guard after merging gradeColumnInfo to avoid null being interpreted as 0 there too.
| await this.#applyColumnWeightToGrade(grade); | ||
|
|
||
| if ( | ||
| (this.selectedGrades.some((g) => g.id === grade.CijferId) || | ||
| force === false) && | ||
| force !== true | ||
| ) { | ||
| this.selectedGrades = this.selectedGrades.filter( | ||
| (g) => g.id !== grade.CijferId, | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
toggleGrade awaits #applyColumnWeightToGrade before determining whether this call is actually removing an already-selected grade. This can add unnecessary latency (and potentially network calls) to deselection/removal flows; consider checking the remove-condition first and only fetching column info when the grade is being added/validated.
| await this.#applyColumnWeightToGrade(grade); | |
| if ( | |
| (this.selectedGrades.some((g) => g.id === grade.CijferId) || | |
| force === false) && | |
| force !== true | |
| ) { | |
| this.selectedGrades = this.selectedGrades.filter( | |
| (g) => g.id !== grade.CijferId, | |
| ); | |
| } else { | |
| const isSelected = this.selectedGrades.some( | |
| (g) => g.id === grade.CijferId, | |
| ); | |
| const shouldRemove = | |
| (isSelected || force === false) && force !== true; | |
| if (shouldRemove) { | |
| this.selectedGrades = this.selectedGrades.filter( | |
| (g) => g.id !== grade.CijferId, | |
| ); | |
| } else { | |
| await this.#applyColumnWeightToGrade(grade); |
| if (!this.#gradeColumnInfoCache.has(cacheKey)) { | ||
| this.#gradeColumnInfoCache.set( | ||
| cacheKey, | ||
| magisterApi.gradesColumnInfo(year, columnId).catch(() => { | ||
| this.#gradeColumnInfoFailed.add(cacheKey); | ||
| return null; | ||
| }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Because #applyColumnWeightToGrade can call magisterApi.gradesColumnInfo, and toggleGrade() is invoked for every grade during the pane’s first redraw() initialization, opening this pane may now result in many column-info requests (one per unique column with unknown weight). This can be slow and may risk API rate limiting; consider adding throttling/backoff (see the delays used when fetching column info in grades/backup.js) or deferring these lookups until the user explicitly selects grades.
… to false for grades with weight being 0