Add country rank filter#247
Conversation
| # Normalize incoming domains to match the domain_normalized field | ||
| domains = list({nd for d in domains if (nd := normalize_domain(d))}) | ||
| # Only import for buckets that don't already have rank data. | ||
| already_ranked = set( |
There was a problem hiding this comment.
I think if triage creates a new cluster bucket for a domain that already has rank rows on another bucket, already_ranked contains that domain, so this new bucket gets excluded and doesn't receive rank? (until there is a full run)
So something like this would make sure these buckets get a rank:
buckets = list(
Bucket.objects.filter(
domain_normalized__in=domains,
country_ranks__isnull=True,
).only("id", "domain_normalized")
)
Though if a bucket already has some rank rows (e.g. us_rank) but is missing a new country (e.g. poland_rank), it will still be excluded on partial runs with this. But it might be fine? Not sure how often that will happen. We can run the full command manually/ or run it once a month on a schedule
| if not domains: | ||
| LOG.info("No buckets with a normalized domain — nothing to import") | ||
| return | ||
|
|
There was a problem hiding this comment.
The two identical domains = [b.domain_normalized for b in buckets] lines can be simplified with a check for buckets:
if not buckets:
LOG.info("No buckets with a normalized domain — nothing to import")
return
domains = [b.domain_normalized for b in buckets]
| name='updated_at', | ||
| field=models.DateTimeField(default=django.utils.timezone.now), | ||
| ), | ||
| ] |
There was a problem hiding this comment.
Would it be possible to squash the 2 migrations into one? Since we haven't deployed this yet.
| .then((columns) => registerCountryRankFilters(columns)) | ||
| .catch((e) => console.debug("Failed to load country rank columns:", e)) | ||
| .finally(() => app.mount("#app")); | ||
| }); |
There was a problem hiding this comment.
Maybe instead of fetching it main.js, this can be in the bucket_filter_config.js
let countryRankReady = null;
export function ensureCountryRankFilters() {
if (!countryRankReady) {
countryRankReady = listCountryRankColumns()
.then((columns) => registerCountryRankFilters(columns))
.catch((e) => console.debug("Failed to load country rank columns:", e));
}
return countryRankReady;
}
And in List.vue it can trigger the call before doing the fetch (so filter and going directly to links like #q=poland_rank:<=1000 work):
fetch: _throttle(
async function () {
await ensureCountryRankFilters();
if (!this.advancedMode) {
....
…d lint - Fix a bug where a bucket sharing a domain with an already-ranked bucket (e.g. a newly triaged cluster) was excluded from partial imports even without rank rows of its own. - Squash the two BucketCountryRank migrations into one. - Fix ruff-format and eslint/prettier violations flagged by CI.
|
@ksy36 thanks for the review! I think I addressed all the feedback. Let me know if there's anything else I should check :) |
2 participants
No description provided.