From b3bc0d428e6dcf6b940157f98296a0734693b57a Mon Sep 17 00:00:00 2001 From: trichoplax Date: Mon, 9 Jan 2023 23:32:05 +0000 Subject: [PATCH 01/16] Convert 'else if' to 'if' because it depends on data from the previous 'if' block --- app/assets/javascripts/qpixel_api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index df4236002..77317537c 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -186,7 +186,7 @@ window.QPixel = { this._preferences = null; } } - else if (this._preferences == null) { + if (this._preferences == null) { // If they're still null (or undefined) after loading from localStorage, we're probably on a site we haven't // loaded them for yet. Load from Redis via AJAX. const resp = await fetch('/users/me/preferences', { From 8be131c6cb77034ba0bcc628fcdfc0aa480fb92b Mon Sep 17 00:00:00 2001 From: trichoplax Date: Mon, 9 Jan 2023 23:38:04 +0000 Subject: [PATCH 02/16] Remove check for old preferences schema from over 2 years ago --- app/assets/javascripts/qpixel_api.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 77317537c..651d62a35 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -179,12 +179,6 @@ window.QPixel = { preferences: async () => { if (this._preferences == null && !!localStorage['qpixel.user_preferences']) { this._preferences = JSON.parse(localStorage['qpixel.user_preferences']); - - // If we don't have the global key, we're probably using an old preferences schema. - if (!this._preferences.global) { - delete localStorage['qpixel.user_preferences']; - this._preferences = null; - } } if (this._preferences == null) { // If they're still null (or undefined) after loading from localStorage, we're probably on a site we haven't From 4ba7d6a67003bbd65f448e7b7ee18ec5307e423c Mon Sep 17 00:00:00 2001 From: trichoplax Date: Mon, 9 Jan 2023 23:44:50 +0000 Subject: [PATCH 03/16] Add early return for most frequent case to reduce processing --- app/assets/javascripts/qpixel_api.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 651d62a35..c0c3d1fbb 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -177,6 +177,10 @@ window.QPixel = { * @returns {Promise} a JSON object containing user preferences */ preferences: async () => { + // Early return for the most frequent case + if (this._preferences != null) { + return this._preferences; + } if (this._preferences == null && !!localStorage['qpixel.user_preferences']) { this._preferences = JSON.parse(localStorage['qpixel.user_preferences']); } From aab2313947760dd50feee4a35d8ade543b8e9928 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Mon, 9 Jan 2023 23:50:26 +0000 Subject: [PATCH 04/16] Remove null check that is now redundant due to early return --- app/assets/javascripts/qpixel_api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index c0c3d1fbb..e9b2056e6 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -181,7 +181,7 @@ window.QPixel = { if (this._preferences != null) { return this._preferences; } - if (this._preferences == null && !!localStorage['qpixel.user_preferences']) { + if (!!localStorage['qpixel.user_preferences']) { this._preferences = JSON.parse(localStorage['qpixel.user_preferences']); } if (this._preferences == null) { From c0b42f4cdf756f30189c975d4a777300215ab15a Mon Sep 17 00:00:00 2001 From: trichoplax Date: Tue, 10 Jan 2023 00:58:22 +0000 Subject: [PATCH 05/16] Improve comment to state local variable contains preferences --- app/assets/javascripts/qpixel_api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index e9b2056e6..ac10c01ea 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -177,7 +177,7 @@ window.QPixel = { * @returns {Promise} a JSON object containing user preferences */ preferences: async () => { - // Early return for the most frequent case + // Early return for the most frequent case (local variable already contains the preferences) if (this._preferences != null) { return this._preferences; } From 0b5b4b50a8af2d0f93cab1395287758058b163c0 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Tue, 10 Jan 2023 01:07:41 +0000 Subject: [PATCH 06/16] Reject only null & undefined, rather than all falsy local preferences --- app/assets/javascripts/qpixel_api.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index ac10c01ea..c4c5d9a3e 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -181,8 +181,13 @@ window.QPixel = { if (this._preferences != null) { return this._preferences; } - if (!!localStorage['qpixel.user_preferences']) { - this._preferences = JSON.parse(localStorage['qpixel.user_preferences']); + // Early return the preferences from localStorage unless null or undefined + const localStoragePreferences = ('qpixel.user_preferences' in localStorage) + ? JSON.parse(localStorage['qpixel.user_preferences']) + : null; + if (localStoragePreferences != null) { + this._preferences = localStoragePreferences; + return this._preferences; } if (this._preferences == null) { // If they're still null (or undefined) after loading from localStorage, we're probably on a site we haven't From 4949c7fd212af3342d6b533fdaf1df24f8965d18 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Tue, 10 Jan 2023 01:21:13 +0000 Subject: [PATCH 07/16] Remove 'if' block made redundant by early returns --- app/assets/javascripts/qpixel_api.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index c4c5d9a3e..20f3e39fd 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -189,19 +189,17 @@ window.QPixel = { this._preferences = localStoragePreferences; return this._preferences; } - if (this._preferences == null) { - // If they're still null (or undefined) after loading from localStorage, we're probably on a site we haven't - // loaded them for yet. Load from Redis via AJAX. - const resp = await fetch('/users/me/preferences', { - credentials: 'include', - headers: { - 'Accept': 'application/json' - } - }); - const data = await resp.json(); - localStorage['qpixel.user_preferences'] = JSON.stringify(data); - this._preferences = data; - } + // Preferences are still null (or undefined) after loading from localStorage, so we're probably on a site we + // haven't loaded them for yet. Load from Redis via AJAX. + const resp = await fetch('/users/me/preferences', { + credentials: 'include', + headers: { + 'Accept': 'application/json' + } + }); + const data = await resp.json(); + localStorage['qpixel.user_preferences'] = JSON.stringify(data); + this._preferences = data; return this._preferences; }, From a1bb30418d59b27ef8097ceb99efb3b088c4d885 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Tue, 10 Jan 2023 17:40:23 +0000 Subject: [PATCH 08/16] Store preferences on QPixel rather than window ('this') --- app/assets/javascripts/qpixel_api.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 20f3e39fd..c99af586d 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -178,16 +178,16 @@ window.QPixel = { */ preferences: async () => { // Early return for the most frequent case (local variable already contains the preferences) - if (this._preferences != null) { - return this._preferences; + if (QPixel._preferences != null) { + return QPixel._preferences; } // Early return the preferences from localStorage unless null or undefined const localStoragePreferences = ('qpixel.user_preferences' in localStorage) ? JSON.parse(localStorage['qpixel.user_preferences']) : null; if (localStoragePreferences != null) { - this._preferences = localStoragePreferences; - return this._preferences; + QPixel._preferences = localStoragePreferences; + return QPixel._preferences; } // Preferences are still null (or undefined) after loading from localStorage, so we're probably on a site we // haven't loaded them for yet. Load from Redis via AJAX. @@ -199,8 +199,8 @@ window.QPixel = { }); const data = await resp.json(); localStorage['qpixel.user_preferences'] = JSON.stringify(data); - this._preferences = data; - return this._preferences; + QPixel._preferences = data; + return QPixel._preferences; }, /** @@ -224,7 +224,7 @@ window.QPixel = { }); const data = await resp.json(); localStorage['qpixel.user_preferences'] = JSON.stringify(data); - this._preferences = data; + QPixel._preferences = data; prefs = await QPixel.preferences(); value = community ? prefs.community[name] : prefs.global[name]; @@ -259,8 +259,8 @@ window.QPixel = { console.error(resp); } else { - this._preferences = data.preferences; - localStorage['qpixel.user_preferences'] = JSON.stringify(this._preferences); + QPixel._preferences = data.preferences; + localStorage['qpixel.user_preferences'] = JSON.stringify(QPixel._preferences); } }, From 94ba47e3d066ed963406a8c8b7bbad214cd82a77 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Tue, 10 Jan 2023 23:22:35 +0000 Subject: [PATCH 09/16] Suppress preferences retrieval when user is not signed in --- app/assets/javascripts/qpixel_api.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index c99af586d..2e1e7882d 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -177,6 +177,11 @@ window.QPixel = { * @returns {Promise} a JSON object containing user preferences */ preferences: async () => { + const user = await QPixel.user(); + // Do not attempt to access preferences if user is not signed in + if ('error' in user) { + return null; + } // Early return for the most frequent case (local variable already contains the preferences) if (QPixel._preferences != null) { return QPixel._preferences; From 33abac3eebbf9c1035638272ea131158758aff19 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Wed, 11 Jan 2023 00:15:59 +0000 Subject: [PATCH 10/16] Extract duplicated localStorage preferences code into a function --- app/assets/javascripts/qpixel_api.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 2e1e7882d..95f2a3f64 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -203,8 +203,7 @@ window.QPixel = { } }); const data = await resp.json(); - localStorage['qpixel.user_preferences'] = JSON.stringify(data); - QPixel._preferences = data; + updatePreferencesLocally(data); return QPixel._preferences; }, @@ -228,8 +227,7 @@ window.QPixel = { } }); const data = await resp.json(); - localStorage['qpixel.user_preferences'] = JSON.stringify(data); - QPixel._preferences = data; + updatePreferencesLocally(data); prefs = await QPixel.preferences(); value = community ? prefs.community[name] : prefs.global[name]; @@ -264,11 +262,15 @@ window.QPixel = { console.error(resp); } else { - QPixel._preferences = data.preferences; - localStorage['qpixel.user_preferences'] = JSON.stringify(QPixel._preferences); + updatePreferencesLocally(data.preferences); } }, + updatePreferencesLocally: data => { + QPixel._preferences = data; + localStorage['qpixel.user_preferences'] = JSON.stringify(QPixel._preferences); + } + /** * Get the word in a string that the given position is in, and the position within that word. * @param splat an array, containing the string already split by however you define a "word" From 306a5541d11629e7d021af2d6b203e121872a963 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Wed, 11 Jan 2023 00:36:56 +0000 Subject: [PATCH 11/16] Use unique localStorage key per user to avoid corruption --- app/assets/javascripts/qpixel_api.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 95f2a3f64..5333ac4f9 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -171,6 +171,11 @@ window.QPixel = { _preferences: null, + preferencesLocalStorageKey: async () => { + const user = await QPixel.user(); + return `qpixel.user_${user.id}_preferences`; + } + /** * Get an object containing the current user's preferences. Loads, in order of precedence, from local variable, * localStorage, or Redis via AJAX. @@ -187,8 +192,9 @@ window.QPixel = { return QPixel._preferences; } // Early return the preferences from localStorage unless null or undefined - const localStoragePreferences = ('qpixel.user_preferences' in localStorage) - ? JSON.parse(localStorage['qpixel.user_preferences']) + const key = await preferencesLocalStorageKey(); + const localStoragePreferences = (key in localStorage) + ? JSON.parse(localStorage[key]) : null; if (localStoragePreferences != null) { QPixel._preferences = localStoragePreferences; @@ -268,7 +274,8 @@ window.QPixel = { updatePreferencesLocally: data => { QPixel._preferences = data; - localStorage['qpixel.user_preferences'] = JSON.stringify(QPixel._preferences); + const key = await preferencesLocalStorageKey(); + localStorage[key] = JSON.stringify(QPixel._preferences); } /** From 1bba8a3756072347aed116ed28c6d8632574cb9e Mon Sep 17 00:00:00 2001 From: trichoplax Date: Wed, 11 Jan 2023 00:43:08 +0000 Subject: [PATCH 12/16] Make 'if' explicit and include mandatory braces for code standards --- app/assets/javascripts/qpixel_api.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 5333ac4f9..db9bc4448 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -158,7 +158,9 @@ window.QPixel = { * @returns {Promise} a JSON object containing user details */ user: async () => { - if (QPixel._user) return QPixel._user; + if (QPixel._user != null) { + return QPixel._user; + } const resp = await fetch('/users/me', { credentials: 'include', headers: { From 81caadb0734dd73048fb84c505cba7acf9905dbd Mon Sep 17 00:00:00 2001 From: trichoplax Date: Wed, 11 Jan 2023 02:49:30 +0000 Subject: [PATCH 13/16] Add the commas I forgot between new functions in QPixel object --- app/assets/javascripts/qpixel_api.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index db9bc4448..679284e1f 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -176,7 +176,7 @@ window.QPixel = { preferencesLocalStorageKey: async () => { const user = await QPixel.user(); return `qpixel.user_${user.id}_preferences`; - } + }, /** * Get an object containing the current user's preferences. Loads, in order of precedence, from local variable, @@ -278,7 +278,7 @@ window.QPixel = { QPixel._preferences = data; const key = await preferencesLocalStorageKey(); localStorage[key] = JSON.stringify(QPixel._preferences); - } + }, /** * Get the word in a string that the given position is in, and the position within that word. From 6a4b1476614b8b0b069c84f6dbcf3b117b22de39 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Wed, 11 Jan 2023 03:00:34 +0000 Subject: [PATCH 14/16] Change preference function to use early return --- app/assets/javascripts/qpixel_api.js | 32 +++++++++++++--------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 679284e1f..78c770f16 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -225,25 +225,23 @@ window.QPixel = { let prefs = await QPixel.preferences(); let value = community ? prefs.community[name] : prefs.global[name]; - // Deliberate === here: null is a valid value for a preference, but undefined means we haven't fetched it. - // If we haven't fetched a preference, that probably means it's new - run a full re-fetch. - if (value === undefined) { - const resp = await fetch('/users/me/preferences', { - credentials: 'include', - headers: { - 'Accept': 'application/json' - } - }); - const data = await resp.json(); - updatePreferencesLocally(data); - - prefs = await QPixel.preferences(); - value = community ? prefs.community[name] : prefs.global[name]; - return value; - } - else { + // Note that null is a valid value for a preference, but undefined means we haven't fetched it. + if (typeof(value) !== 'undefined') { return value; } + // If we haven't fetched a preference, that probably means it's new - run a full re-fetch. + const resp = await fetch('/users/me/preferences', { + credentials: 'include', + headers: { + 'Accept': 'application/json' + } + }); + const data = await resp.json(); + updatePreferencesLocally(data); + + prefs = await QPixel.preferences(); + value = community ? prefs.community[name] : prefs.global[name]; + return value; }, /** From eba672d79f450dd38d7f63b8c5ecbb4df96dbb34 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Thu, 12 Jan 2023 05:03:15 +0000 Subject: [PATCH 15/16] Extract common code into separate functions --- app/assets/javascripts/qpixel_api.js | 49 +++++++++++++--------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 78c770f16..4bf0f7079 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -173,19 +173,14 @@ window.QPixel = { _preferences: null, - preferencesLocalStorageKey: async () => { - const user = await QPixel.user(); - return `qpixel.user_${user.id}_preferences`; - }, - /** * Get an object containing the current user's preferences. Loads, in order of precedence, from local variable, * localStorage, or Redis via AJAX. * @returns {Promise} a JSON object containing user preferences */ preferences: async () => { - const user = await QPixel.user(); // Do not attempt to access preferences if user is not signed in + const user = await QPixel.user(); if ('error' in user) { return null; } @@ -194,7 +189,7 @@ window.QPixel = { return QPixel._preferences; } // Early return the preferences from localStorage unless null or undefined - const key = await preferencesLocalStorageKey(); + const key = await QPixel._preferencesLocalStorageKey(); const localStoragePreferences = (key in localStorage) ? JSON.parse(localStorage[key]) : null; @@ -204,14 +199,7 @@ window.QPixel = { } // Preferences are still null (or undefined) after loading from localStorage, so we're probably on a site we // haven't loaded them for yet. Load from Redis via AJAX. - const resp = await fetch('/users/me/preferences', { - credentials: 'include', - headers: { - 'Accept': 'application/json' - } - }); - const data = await resp.json(); - updatePreferencesLocally(data); + await QPixel._fetchPreferences(); return QPixel._preferences; }, @@ -230,14 +218,7 @@ window.QPixel = { return value; } // If we haven't fetched a preference, that probably means it's new - run a full re-fetch. - const resp = await fetch('/users/me/preferences', { - credentials: 'include', - headers: { - 'Accept': 'application/json' - } - }); - const data = await resp.json(); - updatePreferencesLocally(data); + await QPixel._fetchPreferences(); prefs = await QPixel.preferences(); value = community ? prefs.community[name] : prefs.global[name]; @@ -268,13 +249,29 @@ window.QPixel = { console.error(resp); } else { - updatePreferencesLocally(data.preferences); + await QPixel._updatePreferencesLocally(data.preferences); } }, - updatePreferencesLocally: data => { + _preferencesLocalStorageKey: async () => { + const user = await QPixel.user(); + return `qpixel.user_${user.id}_preferences`; + }, + + _fetchPreferences: async () => { + const resp = await fetch('/users/me/preferences', { + credentials: 'include', + headers: { + 'Accept': 'application/json' + } + }); + const data = await resp.json(); + await QPixel._updatePreferencesLocally(data); + }, + + _updatePreferencesLocally: async data => { QPixel._preferences = data; - const key = await preferencesLocalStorageKey(); + const key = await QPixel._preferencesLocalStorageKey(); localStorage[key] = JSON.stringify(QPixel._preferences); }, From 4582d436f632785ef73f0971fcafb98499544067 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Thu, 12 Jan 2023 05:44:41 +0000 Subject: [PATCH 16/16] Add JavaScript documentation comments for new functions --- app/assets/javascripts/qpixel_api.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 4bf0f7079..814f929e5 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -253,11 +253,19 @@ window.QPixel = { } }, + /** + * Get the key to use for storing user preferences in localStorage, to avoid conflating users + * @returns {Promise} the localStorage key + */ _preferencesLocalStorageKey: async () => { const user = await QPixel.user(); return `qpixel.user_${user.id}_preferences`; }, + /** + * Update local variable _preferences and localStorage with an AJAX call for the user preferences + * @returns {Promise} + */ _fetchPreferences: async () => { const resp = await fetch('/users/me/preferences', { credentials: 'include', @@ -269,6 +277,11 @@ window.QPixel = { await QPixel._updatePreferencesLocally(data); }, + /** + * Set local variable _preferences and localStorage to new preferences data + * @param data an object, containing the new preferences data + * @returns {Promise} + */ _updatePreferencesLocally: async data => { QPixel._preferences = data; const key = await QPixel._preferencesLocalStorageKey();