1205093 - "Ready for L10N" column on KB dashboard should reflect only the last normal or major revision

Status: REOPENED

(support.mozilla.org :: Knowledge Base Software, defect, P2)

Description

[Joni Chan]

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"

Comment 1

[Safwan Rahman (:safwan)]

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

Comment 2

[Joni Chan]

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.

Comment 3

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

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)?

Comment 4

[Joni Chan]

Hi Michal. Yes, I think this is still useful to have. Thanks for checking!

Comment 5

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

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.

Comment 6

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

Attachment: GitHub PR

^

Comment 7

[Joni Chan]

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.

Comment 8

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

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.

Comment 9

[Alice Wyman]

Related discussion:
https://support.mozilla.org/en-US/forums/l10n-forum/712016 Changes Not Ready For Localization

Comment 10

[Wayne Mery (:wsmwk)]

(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

Comment 11

[Wayne Mery (:wsmwk)]

does bug 703321 still exist?

Comment 12

[Alice Wyman]

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

Comment 13

[Alice Wyman]

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.

Comment 14

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

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.

Comment 15

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

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)

Comment 16

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

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.

Comment 17

[vesper]

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/ ?

Comment 18

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

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.

Comment 19

[vesper]

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!

Comment 20

[madalina]

Roland, let's discuss this when planning the next sprint, please add it to the backlog.

Comment 21

[Giorgos Logiotatidis [:giorgos]]

(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?

Comment 22

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

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.

Comment 23

[Alice Wyman]

This bug seems to have fallen through the cracks.

Comment 24

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

The patch is still in the PR, which can be reopened later after the platform is ready to accept patches.

Comment 25

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

Attachment: pr 3196: [Bug 1205093] Show Ready for L10n in the KB dashboard based on the latest normal or major revision

Comment 26

[Seburo]

Closed as per https://blog.mozilla.org/sumo/2019/06/07/sumo-platform-roadmap/

Comment 27

[Alice Wyman]

This is still needed. In the meantime, I've been using https://support.mozilla.org/en-US/contributors/unready Changes Not Ready For Localization