iOS - Update web view cookie with in-app selected language #1333

mumer92 · 2020-01-28T11:24:26Z

Description

Create and set web view cookie with the selected language locale.
Cookie name should be "prod-edx-language-preference" and its value should be selected language locale.

How to test this PR

Web view language should be updated as device selected language.

codecov · 2020-01-28T11:59:08Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d680f2f). Click here to learn what that means.
The diff coverage is 0%.

@@           Coverage Diff            @@
##             master   #1333   +/-   ##
========================================
  Coverage          ?     52%           
========================================
  Files             ?     432           
  Lines             ?   27317           
  Branches          ?       0           
========================================
  Hits              ?   14473           
  Misses            ?   12844           
  Partials          ?       0

Impacted Files	Coverage Δ
Source/WKWebView+LanguageCookie.swift	`0% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d680f2f...afcf622. Read the comment docs.

Source/AuthenticatedWebViewController.swift

salman2013 · 2020-01-31T11:02:42Z

Source/AuthenticatedWebViewController.swift

+            }
+
+            getCookie(with: languageCookieName) { cookie in
+                if cookie == nil {


Can you confirm that either the cookie will persist with each session start? If it is then we don't need to set cookie every time by check if the cookie already set.

Printing out cookie value gives:

Optional(<NSHTTPCookie version:1 name:prod-edx-language-preference value:en expiresDate:'2020-03-13 03:30:32 +0000' created:'2020-01-31 11:30:32 +0000' sessionOnly:FALSE domain:.edx.org partition:none sameSite:none path:/ isSecure:FALSE path:"/" isSecure:FALSE>)

Execution time for this function:

Time elapsed for cookie found: 1.77224600315094 seconds Time elapsed for cookie not found and setting cookie: 1.77321195602417 seconds Time elapsed for cookie found: 1.9854040145874023 seconds Time elapsed for cookie not found and setting cookie: 1.9858750104904175 seconds Time elapsed for cookie found: 1.8449450731277466 seconds Time elapsed for cookie not found and setting cookie: 1.846182107925415 seconds Time elapsed for cookie found: 1.8192960023880005 seconds Time elapsed for cookie not found and setting cookie: 1.8198130130767822 seconds

…te function name

salman2013 · 2020-02-03T09:36:08Z

Source/WKWebViewExtension.swift

+                        self?.load(request)
+                    }
+                } else {
+                    self?.load(request)


load(request) seems to be redundant in all conditions.

salman2013 · 2020-02-03T09:47:19Z

Source/WKWebViewExtension.swift

+
+extension URL {
+    var rootDomain: String? {
+        guard let hostName = self.host else { return nil }


self can be remove.

salman2013 · 2020-02-03T09:59:15Z

edX.xcodeproj/project.pbxproj

@@ -2882,6 +2884,7 @@
 				77503E8D1B287DBF00C47229 /* KeyboardInsetsSource.swift */,
 				7784C68A1B03BA1E00529C7C /* LoadStateViewController.swift */,
 				9EAB5BE81B564C2F00CA9F3C /* ProgressController.swift */,
+				5F60728023E445E2004DBF07 /* WKWebViewExtension.swift */,


Can you place the extension class along with all other extensions, this will look more organized. The extension name can be more specific as per apple recommendations.

salman2013 · 2020-02-03T10:04:57Z

Source/WKWebViewExtension.swift

+        return "prod-edx-language-preference"
+    }
+
+    private var defaultLanguage: String {


This can be done like

guard let language = NSLocale.preferredLanguages.first, Bundle.main.preferredLocalizations.contains(language) else { return "en" } return language

salman2013 · 2020-02-04T12:43:51Z

Source/WKWebView+LoadRequestExtension.swift

+        } else {
+            request.addValue("\(languageCookieName)=\(defaultLanguage))", forHTTPHeaderField: "Cookie")
+        }
+


remove this extra line.

salman2013 · 2020-02-04T12:43:57Z

Source/WKWebView+LoadRequestExtension.swift

+
+    func loadRequest(_ request: URLRequest) {
+        var request = request
+


remove this extra line.

salman2013 · 2020-02-04T13:09:36Z

edX.xcodeproj/project.pbxproj

@@ -176,7 +176,7 @@
 		5DD0FFCF1B1D225C00837121 /* DiscussionNewPostViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5DD0FFCD1B1D225C00837121 /* DiscussionNewPostViewController.swift */; };
 		5DD0FFD21B1D23DA00837121 /* DiscussionNewPostViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 5DD0FFD11B1D23DA00837121 /* DiscussionNewPostViewController.xib */; };
 		5DEA47D71B62F64300831EC9 /* DiscussionObjectModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5DEA47D61B62F64300831EC9 /* DiscussionObjectModel.swift */; };
-		5F60728123E445E3004DBF07 /* WKWebViewExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5F60728023E445E2004DBF07 /* WKWebViewExtension.swift */; };
+		5F60728123E445E3004DBF07 /* WKWebView+LoadRequestExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5F60728023E445E2004DBF07 /* WKWebView+LoadRequestExtension.swift */; };


The name should be more specific to the functionality. I think "WKWebView+LanguageCookie"

salman2013 · 2020-02-04T13:10:13Z

@mumer92 Please rebase the code with latest master update.

…te function name

…mer/LEARNER-7498

salman2013 · 2020-02-06T07:40:56Z

@mumer92 When i am changing the language to Spanish the discovery webvew is not updating the language. Can you please check whats the reason for it?

mumer92 · 2020-02-06T07:55:22Z

@salman2013

Cookie is being applied to webview, but seems like it is not loading specific language.

mumer92 · 2020-02-06T10:06:13Z

Seems like it is being applied on device.

salman2013 · 2020-02-11T10:46:03Z

Source/WKWebView+LanguageCookie.swift

    private var defaultLanguage: String {
        guard let language = NSLocale.preferredLanguages.first,
            Bundle.main.preferredLocalizations.contains(language) else { return "en" }
        return language
    }

+    private var storedLanguageCookieValue: String {
+        set {
+            UserDefaults.standard.set(newValue, forKey: SelectedLanguageCookieValue)


We can not read the language code from the existing cookie? My thinking is that there are two cases.

Cookie exists
If cookie exists then check if the cookie language value is not equal to the current app selected language then update the cookie with language otherwise do nothing.

Cookie Not exist
If cookie do not exist then create the new cookie with the app selected language the set it.

I don't think there is a need to use userDefaults whats your thoughts on it?

Please have a look into updated logic.

Update web view cookie with in-app selected language

d09b027

mumer92 requested review from saeedbashir and salman2013 January 28, 2020 11:24

get preferred localizations from Bundle

19a70cf

mumer92 added 3 commits January 28, 2020 17:49

make host name generic for cookie

5bdfe91

extract root domain from host name

915a62b

move language cookie to extension

3a7bec3

salman2013 suggested changes Jan 31, 2020

View reviewed changes

fix github comments, move wekwebview extension to seperate file, upda…

b85f53b

…te function name

salman2013 suggested changes Feb 3, 2020

View reviewed changes

mumer92 added 2 commits February 4, 2020 13:02

fix github comments

9a97c4d

delete file

86ab8b8

salman2013 suggested changes Feb 4, 2020

View reviewed changes

mumer92 added 12 commits February 4, 2020 18:40

fix github comments

49e0dd3

Merge branch 'master' into umer/LEARNER-7498

5b2a243

Update web view cookie with in-app selected language

df7bec3

get preferred localizations from Bundle

9a9176d

make host name generic for cookie

4906220

extract root domain from host name

8e9d4ea

move language cookie to extension

ff1748f

fix github comments, move wekwebview extension to seperate file, upda…

6370b3b

…te function name

fix github comments

ba8bc26

delete file

d680f2f

fix github comments

ef87091

Merge branch 'umer/LEARNER-7498' of github.com:edx/edx-app-ios into u…

fa7fdf6

…mer/LEARNER-7498

mumer92 added 6 commits February 6, 2020 17:00

update language cookie when app language is changed

b35657f

check for device langue and cookie language

2de8811

get language code

faa823b

cleanup

4aa2d4b

add return statement

bf6b1e3

set cache

ab260ca

salman2013 suggested changes Feb 11, 2020

View reviewed changes

mumer92 added 2 commits February 13, 2020 12:11

compare cookie value if it exists

07e3ac6

update cookie on language default case

afcf622

mumer92 marked this pull request as draft April 14, 2022 11:18

kdmccormick closed this May 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

iOS - Update web view cookie with in-app selected language #1333

iOS - Update web view cookie with in-app selected language #1333

mumer92 commented Jan 28, 2020

codecov bot commented Jan 28, 2020 •

edited

salman2013 Jan 31, 2020

mumer92 Jan 31, 2020

mumer92 Feb 3, 2020

salman2013 Feb 3, 2020

salman2013 Feb 3, 2020

salman2013 Feb 3, 2020

salman2013 Feb 3, 2020

salman2013 Feb 4, 2020

salman2013 Feb 4, 2020

salman2013 Feb 4, 2020

salman2013 commented Feb 4, 2020

salman2013 commented Feb 6, 2020

mumer92 commented Feb 6, 2020

mumer92 commented Feb 6, 2020

salman2013 Feb 11, 2020

mumer92 Feb 13, 2020


		func loadRequest(_ request: URLRequest) {
		var request = request

iOS - Update web view cookie with in-app selected language #1333

iOS - Update web view cookie with in-app selected language #1333

Conversation

mumer92 commented Jan 28, 2020

Description

How to test this PR

codecov bot commented Jan 28, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salman2013 commented Feb 4, 2020

salman2013 commented Feb 6, 2020

mumer92 commented Feb 6, 2020

mumer92 commented Feb 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 28, 2020 •

edited