fix: now metrics in search will be aggregated across applications #6915
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
| .leftJoin('metrics', (qb) => { | ||
| qb.on( | ||
| 'metrics.environment', | ||
| '=', | ||
| 'ranked_features.environment', | ||
| ).andOn( | ||
| 'metrics.feature_name', | ||
| '=', | ||
| 'ranked_features.feature_name', | ||
| ); | ||
| }) |
There was a problem hiding this comment.
❌ Getting worse: Complex Method
FeatureSearchStore.searchFeatures already has high cyclomatic complexity, and now it increases in Lines of Code from 211 to 227
| @@ -212,23 +210,6 @@ class FeatureSearchStore implements IFeatureSearchStore { | |||
| '=', | |||
| 'features.name', | |||
| ); | |||
| }) | |||
There was a problem hiding this comment.
Now not joining in into main query, because not neede for filtering, paging
| .select('*') | ||
| .from('ranked_features') | ||
| .innerJoin( | ||
| 'final_ranks', | ||
| 'ranked_features.feature_name', | ||
| 'final_ranks.feature_name', | ||
| ) | ||
| .leftJoin('metrics', (qb) => { |
There was a problem hiding this comment.
Join it later, just to get the data.
There was a problem hiding this comment.
I think this change might have introduced a bug -- when you join here, fields of metrics are also in the final result, duplicating the column names feature_name and environment. When there are no metrics, the fields can be null here (leftJoin, after all), causing problems with the aggregation of the results.
We found a bug indicating that the search was not aggregating metrics properly. The issue was that if there were multiple applications posting metrics, only the metrics from the first one were used.
Fixes implemented in this PR:
Do not join metrics into the main query, as they are not needed for filtering and paging.
Determine 'yes' and 'no' counts outside of the main query, within a CTE.