hypertrace / hypertrace-ui

UI for Hypertrace
Other
25 stars 12 forks source link

Fix: Request should be cloned when caching table data #2522

Open aneeshsharma opened 1 year ago

aneeshsharma commented 1 year ago

Description

When storing the request object in the cache for the table, only the reference was being assigned to the cache for the column config. So, the new request object and the cached request object would always have the same column config even after it is updated leading to new data not being fetch when the column config changes.

Now, made the change so a deep copy of the request is created when caching.

This fixes the issue where new data was not being fetched when sort state changes in the column config.

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (fd5f5e3) 83.01% compared to head (4f22f5b) 81.93%. Report is 30 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2522 +/- ## ========================================== - Coverage 83.01% 81.93% -1.08% ========================================== Files 925 928 +3 Lines 20636 20901 +265 Branches 3257 3321 +64 ========================================== - Hits 17130 17125 -5 - Misses 3386 3638 +252 - Partials 120 138 +18 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 1 year ago

Test Results

       4 files  ±0     316 suites  ±0   31m 36s :stopwatch: +17s 1 135 tests +4  1 135 :heavy_check_mark: +4  0 :zzz: ±0  0 :x: ±0  1 145 runs  +4  1 145 :heavy_check_mark: +4  0 :zzz: ±0  0 :x: ±0 

Results for commit 4f22f5be. ± Comparison against base commit fd5f5e32.

:recycle: This comment has been updated with latest results.

aneeshsharma commented 11 months ago

The sorting and filter issue was solved with the following PR. https://github.com/hypertrace/hypertrace-ui/pull/2521

After discussing offline. Creating a shallow copy is the intended behavior and we don't want a cache miss if the column config is simply modified in place. If any change in the columns is supposed to trigger a cache miss, that is taken care by the request object.

Also added a comment explaining what haveColumConfigsChanged is working, so that nobody tries to dig into this again.

haveColumConfigsChanged is supposed to return false if the list of columns itself has changed. Not if any of the columns themselves have changed.