Faulty check for old preferences schema #947
Description
Describe the bug
Line 186 of qpixel_api.js sets this._preferences to null, but the block that follows is an else if block rather than an if block, so this won't have the intended effect of reloading the preferences until next time this function is called.
To Reproduce
It seems unlikely this will bug will ever be triggered because it seems unlikely that there exist any users with the old preferences schema. This is being raised more as code tidying than bug fixing.
Expected behavior
The preferences should be reloaded by AJAX immediately rather than next time the function is called. As the old preferences schema has been out of use for over 2 years would it be better to simply remove the code that checks for it?
Screenshots
This is the context of the faulty code:
/**
* Get an object containing the current user's preferences. Loads, in order of precedence, from local variable,
* localStorage, or Redis via AJAX.
* @returns {Promise<Object>} a JSON object containing user preferences
*/
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;
}
}
else 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;
}
return this._preferences;
},If the else if block were changed to an if block the problem would be fixed. Might be better to tidy up and remove redundancy instead (for example, the duplication of if (this._preferences == null).
Desktop (please complete the following information):
Smartphone (please complete the following information):
Not operating system related.
Additional context
This was found while looking through the code for learning - there is no reported incorrect behaviour.