Insights dashboard: move KEY_PROJECT_SLUGS to env, remove dashes, reformat dashboard css - #4338
Insights dashboard: move KEY_PROJECT_SLUGS to env, remove dashes, reformat dashboard css#4338functionzz wants to merge 3 commits into
Conversation
Could you please describe the intended changes here or file an issue? |
…able.html Co-authored-by: Matjaž Horvat <matjaz.horvat@gmail.com>
mathjazz
changed the title
| "firefox," | ||
| "mozilla-accounts," | ||
| "mozilla-vpn-client", | ||
| ).split(",") |
There was a problem hiding this comment.
- That's not very readable - the order of
,and"is easy to get wrong. - The order of project slugs also appears to be pretty random.
- It also doesn't prevent whitespace and empty entries from causing subtle bugs.
KEY_PROJECT_SLUGS="firefox, firefox-for-ios"yieldsfirefox-for-ios, which matches no project and silently under-counts. - Setting
KEY_PROJECT_SLUGS=""gives[""]rather than[], solen(KEY_PROJECT_SLUGS)is1— which feeds both thebase_thresholdand the divisor inchs.py.
Something like this would be a big improvement:
DEFAULT_KEY_PROJECT_SLUGS = (
"firefox",
"firefox-for-android",
"firefox-for-ios",
"firefox-relay-website",
"mozilla-accounts",
"mozilla-monitor-website",
"mozilla-vpn-client",
)
KEY_PROJECT_SLUGS = [
slug.strip()
for slug in os.environ.get(
"KEY_PROJECT_SLUGS",
",".join(DEFAULT_KEY_PROJECT_SLUGS),
).split(",")
if slug.strip()
]There was a problem hiding this comment.
yeah that makes alot of sense
| "mozilla-accounts", | ||
| "mozilla-monitor-website", | ||
| "mozilla-vpn-client", | ||
| ) |
There was a problem hiding this comment.
Thanks, that looks good now, except... Should we avoid shipping Mozilla-specific project slugs in the codebase? Pontoon runs outside Mozilla too, and those instances would silently compute a CHS against seven slugs that don't exist rather than a visibly unconfigured one.
I suggest the default is an empty list or all priority=HIGHEST projects instead?
There was a problem hiding this comment.
right...probably an empty list makes sense. But does that mean the project slugs need to be manually inputted by the superuser on Mozilla's Pontoon instance?
There was a problem hiding this comment.
Correct. The alternative would be to add a Project.is_key_project field and set it in the admin.
There was a problem hiding this comment.
I kind of like the idea of defining CHS projects (wouldn't call them key projects though) as part of the project object.
There was a problem hiding this comment.
How would you call them (and the corresponding DB field)?
There was a problem hiding this comment.
I thought I left a suggestion, but that was just in my head. is_chs_project should work and avoid confusion?
There was a problem hiding this comment.
Sounds good.
I wonder where does the confusion come from, though. The colum header also reads "Enabled Key Projects".
You mean it might confuse 3rd party users? We could also use help_text for that.
There was a problem hiding this comment.
Key projects out of context sounds too opinionated to me, or at least open to interpretation. The header is literally in the Community Health Score section, so there is context and it's fine.
These are just projects that we chose to use for CHS, they might not even be key (I wouldn't call Monitor/Relay key these days 😓).
There was a problem hiding this comment.
OK. Please comment above if we should also change the "Enabled Key Projects" to something else then, e.g. "Enabled Tracked Projects" or something.
3 participants
This PR contains small requested changes to Insights dashboard and CHS snapshot collection code.
These changes include:
KEY_PROJECT_SLUGSto base.py as an env