Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature App-defined Language implementation backported to 3.X.X version #1350

Merged
merged 29 commits into from
Jun 3, 2021

Conversation

tanaynigam
Copy link
Contributor

@tanaynigam tanaynigam commented May 28, 2021

Description

Add feature app-defined language implementation. A user can define their own language setting to be used by OneSignal-SDK version 3.X.X. Cherry picked from OneSignal-SDK version 4.4.0 PR #1334

Details

  • Add LanguageContext, LanguageProviderAppDefined and LanguageProviderDevice class and LanguageProvider interface
  • Add OneSignal.setLanguage implementation
  • Add runSetLanguage runnable to OneSignal.setLanguage for network call implementation. Add runnable to addTaskToQueue

Test

  • Add MainOneSignalClassRunner testSetLanguageOnPlayerCreate, testSetLanguagePUTRequest, testSetLanguageOnSession unit tests
  • Test device using button click to call OneSignal.setLanguage to set it to language different from language defined by the device. OneSignal app dashboard reflects the language set using OneSignal.setLanguage

This change is Reviewable

* Added Language Context Interface for getter and setter for Language Provider
* Added classes Language Provider, LanguageProviderAppDefined and LanguageProviderDevice
* The interface retrieves the default language set by device if a language override is not set
* Fixed the null check in the LanguageContext class
* Added getLanguage method to retrieve the language from strategy
* added network call to setLanguage method in OneSignal class
* Updated device info to retrieve language from LanguageContext class
* Added SET_LANGUAGE to OSTaskController String Array to execute task delays
* Added a Unit test to test setLanguage method executing during the initialization of OneSignal library
* Added a Unit test to test setLanguage method executing after the OneSignal library has been initialized
* Added a Unit test to test setLanguage method executing after a an app has been restarted and new session has been initialized
* Moved the implementation of OSUtils.getCorrectedLanguage to LanguageProviderDevice.getLanguage
* Made OneSignal.preferences public for accessibility in Language classes
* Moved callbacks for OneSignalPrefs from LanguageContext and LanguageProviderAppDefined classes to OneSignal.preferences
* Fix OSInAppMessageController variantIdForMessage retrieves languageIndentifier from OneSignal languageContext
* Fix NotificationChannelManager CreateChannel retrieves deviceLanguage from OneSignal languageContext
* Add Singleton pattern to LanguageContext class
* Add OSSharedPreference dependency to LanguageProviderAppDefined constructor
* Add OSSharedPreference dependency to LanguageContext constructor
* Add LanguageContext dependency to OSInAppMessageController constructor
* Add LanguageContext dependency to OSInAppMessageControllerFactory constructor
* Add LanguageContext dependency to OSInAppMessageDummyController
* Rename languageIdentifier to language in OSInAppMessageController variantIdentifier
* Update NotificationChannelManager createChannel to use LanguageContext instance
* Rename deviceLanguage variable to language for consistency
* Convert OneSignal preferences and OneSignal languageContext from public to private since they are being called through dependency injection
* Fix format space LanguageContext class
* Fix format space LanguageProviderAppDefined class
* Fix format space LanguageProviderDevice class
* Remove unnecessary null check at OneSignal.setLanguage
* Remove error logging at JSONException catch at OneSignal.setLanguage
…Language

* Add OneSignal.initWithContext to NotificationChannelManagerRunner.processChannelListWithMultiLanguage to allow languageContext to initialize before using languageContext.getLanguage
* Add NonNull annotation to getLanguage implementations in LanguageProviderDevice, LanguageProviderAppDefined and LanguageContext classes
* Move "en" string to constant in LanguageProviderAppDefined class
* Move Corrected Language Strings to constant in LanguageProviderDevice class
* Add OSTaskRemoteController.SET_LANGUAGE in place of setLanguage() string in OneSignal.setLanguage
* Move preferences.getString to variable instead of if statement for clarity in LanguageContext constructor
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove .ds_store file

Reviewed 13 of 13 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @emawby, @Jeasmine, and @tanaynigam)


OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java, line 745 at r1 (raw file):

      // Initialize languageContext
      languageContext = new LanguageContext(preferences);

We are already creating an instance of setAppContext so we should remove this one.

Copy link
Contributor

@emawby emawby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 13 of 13 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Jeasmine and @tanaynigam)

Copy link
Contributor

@Jeasmine Jeasmine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after josh's comments are done

Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @tanaynigam)

* Replace androidx imports to android.support imports in LanguageContext, LanguageProviderDevice, LanguageAppDefined classes and LanguageProvider interface
* Move order of class initializations of classes LanguageProviderAppDefined, LanguageContext
* Move order of run command of runnable runSetLanguage
* Fix OSInAppMessageController.htmlPathForMessage to non-static
* Fix NotificationChannelManagerRunner.processChannelListWithMultiLanguage to use OneSignal.startInit
* Remove time variable from MainOneSignalClassRunner.testSetLanguageOnSession function restartAppAndElapseTimeToNextSession
* Add languageContext initialization to OneSignal.setAppContext to allow languageContext initialization to run when app is initialized via context.
* Add simulated delay to FCM/GCM providing a push token which delays the /player CREATE network call in MainOneSignalClassRunner.testSetLanguageOnPlayerCreate and MainOneSignalClassRunner.testSetLanguageOnSession unit tests
* Remove unnecessary languageContext initialization from OneSignal.init
* Replace startInit with setAppContext in NotificationChannelManagerRunner.processChannelWithMultiLanguage
@tanaynigam tanaynigam force-pushed the feat/set_language_for_3.X.X branch from 0c0e704 to 678ff8b Compare June 3, 2021 20:01
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @tanaynigam)

@jkasten2 jkasten2 merged commit 6fdfb55 into sdk_3.X.X Jun 3, 2021
@jkasten2 jkasten2 deleted the feat/set_language_for_3.X.X branch June 3, 2021 20:37
@jkasten2 jkasten2 mentioned this pull request Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants