Refactor stats, removing aggregate fields from project, projectlocale & locale #3536
Conversation
|
It's likely (but not certain) that this will also provide a solution for #3514. |
|
The branch is deployed to stage for testing. |
Stats for Italian are completely off? |
|
https://mozilla-pontoon-staging.herokuapp.com/sl/ also shows 495 more Missing strings in the heading than the sum of Missing ProjectLocale strings in the table. Are we including private projects? |
No, but we aren't filtering out disabled projects in some cases where we should. I'll apply a fix later today. |
There was a problem hiding this comment.
Looks good!
We tested on stage and not only do stats look fine, but we also didn't notice performance regressions.
We should check New Relic after deploying to prod and monitor any performance impact. We should also verify the Insights are still collected as expected.
Based on my unscientific local testing, the rendering speed benefits from eagerly populating the aggregated stats into Project, ProjectLocale & Locale don't exist, compared to collating the data on the fly from TranslatedResource objects. Therefore, we should not be doing this complex extra work. This significantly simplifies the logic around stats updates, and makes the stats much easier to reason about.
The AggregatedStats class no longer extends Model, and it's mostly a crutch for reducing the risks in this change; hence its move from
base/models/tobase/. With it, fields liketotal_stringsandstrings_with_errorswill continue to work for the Django models from which the corresponding DB fields were removed.