Open Bug 1205093 Opened 4 years ago Updated 8 days ago
"Ready for L10N" column on KB dashboard should reflect only the last normal or major revision
pr 3196: [Bug 1205093] Show Ready for L10n in the KB dashboard based on the latest normal or major revision
44 bytes, text/x-github-pull-request
|Details | Review|
The "Ready for L10N" column on the KB dashboard shows all articles as "ready for L10N" even if they're not. STR: 1. Visit the KB dashboard at https://support.mozilla.org/en-US/contributors/kb-overview?product=firefox 2. Scroll down to "How to sync your add-ons with another copy of Firefox". Note that it shows up as "ready for L10N" on the dashboard. 3. Click the article title. 4. View the article history. The "R" column says it's not ready for localization. Expected result: The "Ready for L10N" column for articles that are not ready for L10N should say "No" (black text, red background). Actual result: The article's "Ready for L10N" column says "Yes"
From platform side, I understand as following: # If the article has "any revision ready for localization", it will show "Yes" # If the article has "no revision ready for localization", it will show "No" Note: "How to sync your add-ons with another copy of Firefox" article has 3 revision ready for localization.(1) If you look at dashboard, the article "Tracking Protection in Private Browsing" has no revision ready for localization(2) and its showing "No" in the dashboard So, I think the dashboard is working as expected. @Joni, what do you prefer? how it should work? 1. https://support.mozilla.org/en-US/kb/sync-your-add-ons-another-copy-firefox/history 2. https://support.mozilla.org/en-US/kb/tracking-protection-pbm/history
Thanks for pointing this out, Safwan. I wasn't aware that the dashboard was looking at any revision in the article history. I'd like this dashboard to consider only the last normal or major revision (not the minor ones). - If the last normal/major revision is not ready for L10N, the "ready for L10N" column should say "No". - If it was marked ready for L10N, I'd like it to say "Yes". I'll reword the title.
Summary: "Ready for L10N" column on KB dashboard is inaccurate → "Ready for L10N" column on KB dashboard should reflect only the last normal or major revision
This bug is more than half a year old. Is still the desired behavior to show Yes/No depending on the last normal/major revision (instead of saying Yes if any revision has been made ready for l10n)?
Hi Michal. Yes, I think this is still useful to have. Thanks for checking!
Started working on it now. Just to clarify, should I do some magic around new minor typo revisions approved for l10n? Two cases I am not sure what should happen: 1) There is a minor revision (marked ready for l10n) based on the latest normal/major revision (not marked ready for l10n). 2) There is a brand new minor revision, let's say from today (ready for l10n), but not based on the latest normal/major revision. The latest normal/major revision is not ready for l10n.
Assignee: nobody → mstanke
Status: NEW → ASSIGNED
Good question, Michał. Here are my thoughts: 1) Bypass the minor revision and just factor in the last normal/major revision. The minor revision applies only to en-us, even if it's based on a normal/major revision not marked for L10N. 2) Bypass the minor revision again. In these cases, the minor revisions would have to be redone anyway if it's not based on the last normal/major revision. Let me know if that covers it or if you see any other potential issues.
Thank you, Joni. The current patch I have prepared should do right this way. But we are afraid of breaking the dashboard without the dev team. So I need to prepare some tests first before merging it, so it may take some time before this change is applied to production.
Related discussion: https://support.mozilla.org/en-US/forums/l10n-forum/712016 Changes Not Ready For Localization
(In reply to Alice Wyman from comment #9) > Related discussion: > https://support.mozilla.org/en-US/forums/l10n-forum/712016 Changes Not Ready > For Localization and https://support.mozilla.org/en-US/forums/knowledge-base-articles/711969
Severity: normal → major
does bug 703321 still exist?
As a workaround until this bug is fixed, we can view the "Changes Not Ready For Localization" page at https://support.mozilla.org/contributors/unready to find articles with approved content changes that haven't been marked ready to localize. This page was "orphaned" when we switched to the new KB Dashboard but it's currently linked from https://support.mozilla.org/kb/article-review-guidelines
3 years ago
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Reopening. I see no reason not to fix this bug, now that we are back on Kitsune for the foreseeable future. Roland (or Joni), can you set a priority for this bug? This bug makes it difficult for reviewers to find articles with approved content changes that haven't been marked ready to localize (unless they use the workaround in comment 12). The KB Dashboard Overview page at https://support.mozilla.org/en-US/contributors/kb-overview will show Yes under the Ready for L10n column as long as any article revision was ever marked RFL in the past. We want the column to show if the last content revision (normal or major) needs to be made ready to localize. Example: https://support.mozilla.org/en-US/kb/custom-installation-firefox-on-windows shows "Yes" in the Ready for L10n column, even though https://support.mozilla.org/en-US/kb/custom-installation-firefox-on-windows/history shows that the last two content changes are not ready for localization.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
This has been closed due to expected Kitsune decommission. It's still under code freeze, but once it's open for development again, I am ready to reopen the PR with the patch.
setting to P2, perhaps we can help Michal tackle this in the next sprint, do you agree this is critical enough for that :vesper, joni and madalina? (I don't know how we can help other than asking somebody from ben sternthal's team to take a look at Michal's soon-to-be-reopened-PR if they have time in the next sprint)
Priority: -- → P2
3 years ago
The complexity of the change is not high, but the change itself is in very important dashboard - the more the change is needed, the more would hurt if it breaks something else there. So if there is someone willing to review and merge the patch into the codebase, I am happy to reopen the PR and implement any changes requested in the review.
Thanks to everyone looking into this! How about spending at least a few good days reviewing the impact of the PR on a test environment before it's merged? @Michal - do you have a local testing environment for Kitsune or is this something best tested on https://support.allizom.org/ ?
Flags: needinfo?(mdziewonski) → needinfo?(mstanke)
I do not have any infrastructure than would allow you access from outside to test it on my setup. So the two options are: 1) Setup local Kitsune instance according to the documentation and add enough data to test manually yourself. 2) Ask :giorgos if the patch can be deployed to staging, and test there. But I am not sure how staging currently works in the deploy pipeline. There used to be another instance for developers, but AFAIK it's not anymore. In any case, the code needs to be reviewed because of the style, if there is a better way for the change, etc.
Hi, Giorgos - a question about the staging environment for testing Kitsune patches submitted by Michal or other community contributors - are there any changes to how you found things to be pre-migration? Thanks!
Roland, let's discuss this when planning the next sprint, please add it to the backlog.
(In reply to vesper from comment #19) > Hi, Giorgos - a question about the staging environment for testing Kitsune > patches submitted by Michal or other community contributors - are there any > changes to how you found things to be pre-migration? There's no support-dev.allizom.org anymore as Michal states. (In reply to Michal Stanke (Mozilla.cz) [:MikkCZ] (use needinfo) from comment #18) > 2) Ask :giorgos if the patch can be deployed to staging, and test there. But > I am not sure how staging currently works in the deploy pipeline. There used > to be another instance for developers, but AFAIK it's not anymore. I can push that to staging for you and have it live for a couple of days so you can verify. Does that work for you?
Flags: needinfo?(giorgos) → needinfo?(mstanke)
Thank you Giorgios. Yes, that will work. We should coordinate this with Joni too, as she is the reported and can clearly verify everything works. Joni, are you working in the next days, or on PTO? The code review is of course something different.
This bug seems to have fallen through the cracks.
The patch is still in the PR, which can be reopened later after the platform is ready to accept patches.
Status: REOPENED → RESOLVED
Closed: 3 years ago → 1 year ago
Resolution: --- → WONTFIX
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
You need to log in before you can comment on or make changes to this bug.