-
Notifications
You must be signed in to change notification settings - Fork 330
Bug 1876823 - Add action to fetch list of sites that should not be tr… #5463
Conversation
359308f
to
e887898
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on translations!
Just a few minor comments about things that recently changed and a suggested pattern for the MiddlewareTest
.
...e/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt
Outdated
Show resolved
Hide resolved
...ser/state/src/main/java/mozilla/components/browser/state/reducer/TranslationsStateReducer.kt
Outdated
Show resolved
Hide resolved
@@ -203,6 +203,24 @@ class TranslationsActionTest { | |||
assertEquals(supportedLanguages, tabState().translationsState.supportedLanguages) | |||
} | |||
|
|||
@Test | |||
fun `WHEN a SetNeverTranslateSitesAction is dispatched AND successful THEN update neverTranslateSites`() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I figured out what was going on with the TranslationsMiddlewareTest.kt pattern over in #5389. That test now seems to be verifying as expected and I think could be used as a pattern for an additional test on this patch too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! I added the setup and tested it out, and will rebase when you have that merged in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks and thanks for the review! #5389 just merged!
...ponents/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt
Show resolved
Hide resolved
...e/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt
Outdated
Show resolved
Hide resolved
...ents/browser/state/src/main/java/mozilla/components/browser/state/state/TranslationsState.kt
Outdated
Show resolved
Hide resolved
e887898
to
1a2a6b0
Compare
1a2a6b0
to
79133e7
Compare
…anslated
Pull Request checklist
After merge
To download an APK when reviewing a PR (after all CI tasks finished running):
Checks
at the top of the PR page.firefoxci-taskcluster
group on the left to expand all tasks.build-apk-{fenix,focus,klar}-debug
task you're interested in.View task in Taskcluster
in the newDETAILS
section.GitHub Automation
https://bugzilla.mozilla.org/show_bug.cgi?id=1876823