From 238e894377d9c3261b8bfe6e9e22435ca781b371 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= <jeremie@astori.fr>
Date: Sun, 24 Dec 2017 18:18:24 -0500
Subject: [PATCH] Improve the version checking part of the changelog feature

- There is no client caching of the changelog/version anymore. Instead, server returns the expiration date of its cache, and that is used by the client as well.
- There is now a "Check now" button on the client that appears when data is stale. This means that info is fetched only once and never refreshed (it was refreshed every hour before) unless the user explicitly wants to check latest version, which in turn is as stale as server info is, i.e. 15 minutes max.
- Button style is shared with the "Join a channel" feature, `.btn-small` (not `.btn-sm` to be explicit that this is not a Bootstrap thing).
- Version checker content is now centralized in the `version_checker` template, instead of being partially in the checker template, partially in the Help template,  and partially in the code.
- A "Try again" button lets user attempt to fetch info instead of forcing them to reload the page.
- Use Flexbox to display a nicer version checker: icon is slightly bigger, and button is always aligned on the right.
- Changelog logic has been removed from `lounge.js` and moved into the component file.
- Changelog template is only passed what it needs instead of everything the server gives us.
- Public version now displays version checker, since server is caching things.
- Cleaner code overall.
---
 client/css/style.css                 | 61 ++++++++++++++++++++--------
 client/js/lounge.js                  | 10 +----
 client/js/socket-events/changelog.js | 57 ++++++++++++++++++++++----
 client/views/index.js                |  2 +-
 client/views/join_channel.tpl        |  2 +-
 client/views/new_version.tpl         |  6 ---
 client/views/version_checker.tpl     | 27 ++++++++++++
 client/views/windows/changelog.tpl   | 10 ++---
 client/views/windows/help.tpl        |  6 +--
 src/plugins/changelog.js             | 10 +++--
 10 files changed, 136 insertions(+), 55 deletions(-)
 delete mode 100644 client/views/new_version.tpl
 create mode 100644 client/views/version_checker.tpl

diff --git a/client/css/style.css b/client/css/style.css
index f0487c55..5fa19b02 100644
--- a/client/css/style.css
+++ b/client/css/style.css
@@ -137,6 +137,10 @@ kbd {
 	cursor: pointer; /* This is useful for `<button>` elements */
 }
 
+.btn-small {
+	padding: 5px 13px;
+}
+
 .btn:disabled,
 .btn:hover,
 .btn:focus {
@@ -235,7 +239,7 @@ kbd {
 #chat .nick .from::before,
 #chat .action .from::before,
 #chat .toggle-button::after,
-.changelog-version::before,
+#version-checker::before,
 .context-menu-item::before,
 #help .website-link::before,
 #help .documentation-link::before,
@@ -991,9 +995,7 @@ kbd {
 #sidebar .join-form .btn {
 	display: block;
 	width: 80%;
-	padding: 1px;
 	margin: auto;
-	height: 29px;
 }
 
 #sidebar .add-channel-tooltip {
@@ -1635,47 +1637,70 @@ part/quit messages where we don't load previews (adds a blank line otherwise) */
 	padding-bottom: 7px;
 }
 
-.changelog-version {
-	display: block;
-	padding: 16px;
+#version-checker {
+	display: flex;
+	align-items: center;
+	padding: 10px;
 	margin-bottom: 16px;
 	border-radius: 2px;
-	background-color: #d9edf7;
-	color: #31708f;
 	transition: color 0.2s, background-color 0.2s;
 }
 
-.changelog-version::before {
-	margin-right: 6px;
+#version-checker p,
+#version-checker button {
+	margin-bottom: 0;
+}
+
+#version-checker p {
+	flex: 1;
+	padding-top: 6px;
+	padding-bottom: 6px;
+}
+
+#version-checker::before {
+	margin-left: 6px;
+	margin-right: 12px;
+	font-size: 1.2em;
+}
+
+#version-checker.loading {
+	background-color: #d9edf7;
+	color: #31708f;
+}
+
+#version-checker.loading::before {
 	content: "\f250"; /* http://fontawesome.io/icon/hourglass-o/ */
 }
 
-.changelog-version.new-version {
+#version-checker.new-version {
 	color: #8a6d3b;
 	background-color: #fcf8e3;
 }
 
-.changelog-version.new-version::before {
+#version-checker.new-version::before {
 	content: "\f087"; /* http://fontawesome.io/icon/thumbs-o-up/ */
 }
 
-.changelog-version.error {
+#version-checker.error {
 	color: #a94442;
 	background-color: #f2dede;
 }
 
-.changelog-version.error::before {
-	margin-right: 6px;
+#version-checker.error::before {
 	content: "\f06a"; /* http://fontawesome.io/icon/exclamation-circle/ */
 }
 
-.changelog-version.up-to-date {
+#version-checker.up-to-date {
 	background-color: #dff0d8;
 	color: #3c763d;
 }
 
-.changelog-version.up-to-date::before {
-	margin-right: 6px;
+#version-checker.up-to-date #check-now {
+	/* "Check now" button is hidden until data expires */
+	display: none;
+}
+
+#version-checker.up-to-date::before {
 	content: "\f00c"; /* http://fontawesome.io/icon/check/ */
 }
 
diff --git a/client/js/lounge.js b/client/js/lounge.js
index 4cbb37c0..7fdb4f5c 100644
--- a/client/js/lounge.js
+++ b/client/js/lounge.js
@@ -20,6 +20,7 @@ const utils = require("./utils");
 require("./webpush");
 require("./keybinds");
 require("./clipboard");
+const Changelog = require("./socket-events/changelog");
 const JoinChannel = require("./join-channel");
 
 $(function() {
@@ -332,8 +333,6 @@ $(function() {
 		$(this).closest(".msg.condensed").toggleClass("closed");
 	});
 
-	let changelogRequestedAt = 0;
-
 	const openWindow = function openWindow(e, data) {
 		var self = $(this);
 		var target = self.data("target");
@@ -426,12 +425,7 @@ $(function() {
 		}
 
 		if (target === "#help" || target === "#changelog") {
-			const now = Date.now();
-			// Don't check more than once an hour
-			if (now - changelogRequestedAt > 3600 * 1000) {
-				changelogRequestedAt = now;
-				socket.emit("changelog");
-			}
+			Changelog.requestIfNeeded();
 		}
 
 		focus();
diff --git a/client/js/socket-events/changelog.js b/client/js/socket-events/changelog.js
index 5eaf0229..edff4615 100644
--- a/client/js/socket-events/changelog.js
+++ b/client/js/socket-events/changelog.js
@@ -4,19 +4,60 @@ const $ = require("jquery");
 const socket = require("../socket");
 const templates = require("../../views");
 
+module.exports = {
+	requestIfNeeded,
+};
+
+// Requests version information if it hasn't been retrieved before (or if it has
+// been removed from the page, i.e. when clicking on "Check now". Displays a
+// loading state until received.
+function requestIfNeeded() {
+	if ($("#version-checker").is(":empty")) {
+		renderVersionChecker({status: "loading"});
+		socket.emit("changelog");
+	}
+}
+
 socket.on("changelog", function(data) {
-	const container = $("#changelog-version-container");
+	// 1. Release notes window for the current version
+	$("#changelog").html(templates.windows.changelog(data.current));
+
+	// 2. Version checker visible in Help window
+	let status;
 
 	if (data.latest) {
-		container.addClass("new-version");
-		container.html(templates.new_version(data));
+		status = "new-version";
 	} else if (data.current.changelog) {
-		container.addClass("up-to-date");
-		container.text("The Lounge is up to date!");
+		status = "up-to-date";
 	} else {
-		container.addClass("error");
-		container.text("An error has occurred, try to reload the page.");
+		status = "error";
 	}
 
-	$("#changelog").html(templates.windows.changelog(data));
+	renderVersionChecker({
+		latest: data.latest,
+		status,
+	});
+
+	// When there is a button to refresh the checker available, display it when
+	// data is expired. Before that, server would return same information anyway.
+	if (data.expiresAt) {
+		setTimeout(
+			() => $("#version-checker #check-now").show(),
+			data.expiresAt - Date.now()
+		);
+	}
 });
+
+// When clicking the "Check now" button, remove current checker information and
+// request a new one. Loading will be displayed in the meantime.
+$("#help").on("click", "#check-now", () => {
+	$("#version-checker").empty();
+	requestIfNeeded();
+});
+
+// Given a status and latest release information, update the version checker
+// (CSS class and content)
+function renderVersionChecker({status, latest}) {
+	$("#version-checker").attr("class", status)
+		.html(templates.version_checker({latest, status}));
+}
diff --git a/client/views/index.js b/client/views/index.js
index 4be9cc10..dd7f65f8 100644
--- a/client/views/index.js
+++ b/client/views/index.js
@@ -31,7 +31,6 @@ module.exports = {
 
 	chan: require("./chan.tpl"),
 	chat: require("./chat.tpl"),
-	new_version: require("./new_version.tpl"),
 	contextmenu_divider: require("./contextmenu_divider.tpl"),
 	contextmenu_item: require("./contextmenu_item.tpl"),
 	date_marker: require("./date-marker.tpl"),
@@ -50,4 +49,5 @@ module.exports = {
 	user: require("./user.tpl"),
 	user_filtered: require("./user_filtered.tpl"),
 	user_name: require("./user_name.tpl"),
+	version_checker: require("./version_checker.tpl"),
 };
diff --git a/client/views/join_channel.tpl b/client/views/join_channel.tpl
index 121acabb..92395175 100644
--- a/client/views/join_channel.tpl
+++ b/client/views/join_channel.tpl
@@ -1,5 +1,5 @@
 <form id="join-channel-{{id}}" class="join-form" method="post" action="" autocomplete="off">
 	<input type="text" class="input" name="channel" placeholder="Channel" pattern="[^\s]+" maxlength="200" title="The channel name may not contain spaces" required>
 	<input type="password" class="input" name="key" placeholder="Password (optional)" pattern="[^\s]+" maxlength="200" title="The channel password may not contain spaces">
-	<button type="submit" class="btn joinchan:submit" data-id="{{id}}">Join</button>
+	<button type="submit" class="btn btn-small" data-id="{{id}}">Join</button>
 </form>
diff --git a/client/views/new_version.tpl b/client/views/new_version.tpl
deleted file mode 100644
index 8825ed16..00000000
--- a/client/views/new_version.tpl
+++ /dev/null
@@ -1,6 +0,0 @@
-The Lounge <b>{{latest.version}}</b>{{#if latest.prerelease}} (pre-release){{/if}}
-is now available.
-
-<a href="{{latest.url}}" target="_blank" rel="noopener">
-	Read more on GitHub
-</a>
diff --git a/client/views/version_checker.tpl b/client/views/version_checker.tpl
new file mode 100644
index 00000000..8b75d09f
--- /dev/null
+++ b/client/views/version_checker.tpl
@@ -0,0 +1,27 @@
+{{#equal status "loading"}}
+	<p>
+		Checking for updates...
+	</p>
+{{else equal status "new-version"}}
+	<p>
+		The Lounge <b>{{latest.version}}</b>{{#if latest.prerelease}} (pre-release){{/if}}
+		is now available.
+		<br>
+
+		<a href="{{latest.url}}" target="_blank" rel="noopener">
+			Read more on GitHub
+		</a>
+	</p>
+{{else equal status "up-to-date"}}
+	<p>
+		The Lounge is up to date!
+	</p>
+
+	<button id="check-now" class="btn btn-small">Check now</button>
+{{else equal status "error"}}
+	<p>
+		Information about latest releases could not be retrieved.
+	</p>
+
+	<button id="check-now" class="btn btn-small">Try again</button>
+{{/equal}}
diff --git a/client/views/windows/changelog.tpl b/client/views/windows/changelog.tpl
index 2ffd5c57..222a1a6b 100644
--- a/client/views/windows/changelog.tpl
+++ b/client/views/windows/changelog.tpl
@@ -4,15 +4,15 @@
 <div class="container">
 	<a href="#" id="back-to-help" data-target="#help">« Help</a>
 
-	{{#if current}}
-		<h1 class="title">Release notes for {{current.version}}</h1>
+	{{#if version}}
+		<h1 class="title">Release notes for {{version}}</h1>
 
-		{{#if current.changelog}}
+		{{#if changelog}}
 			<h3>Introduction</h3>
-			<div class="changelog-text">{{{current.changelog}}}</div>
+			<div class="changelog-text">{{{changelog}}}</div>
 		{{else}}
 			<p>Unable to retrieve releases from GitHub.</p>
-			<p><a href="https://github.com/thelounge/lounge/releases/tag/v{{current.version}}" target="_blank" rel="noopener">View release notes for this version on GitHub</a></p>
+			<p><a href="https://github.com/thelounge/lounge/releases/tag/v{{version}}" target="_blank" rel="noopener">View release notes for this version on GitHub</a></p>
 		{{/if}}
 	{{else}}
 		<p>Loading changelog…</p>
diff --git a/client/views/windows/help.tpl b/client/views/windows/help.tpl
index 8f1e9b37..2802d89e 100644
--- a/client/views/windows/help.tpl
+++ b/client/views/windows/help.tpl
@@ -13,11 +13,7 @@
 	</h2>
 
 	<div class="about">
-		{{#unless public}}
-			<p id="changelog-version-container" class="changelog-version">
-				Checking for updates...
-			</p>
-		{{/unless}}
+		<div id="version-checker"></div>
 
 		{{#if gitCommit}}
 			<p>
diff --git a/src/plugins/changelog.js b/src/plugins/changelog.js
index 9f604102..2103ded4 100644
--- a/src/plugins/changelog.js
+++ b/src/plugins/changelog.js
@@ -3,6 +3,8 @@
 const pkg = require("../../package.json");
 const request = require("request");
 
+const TIME_TO_LIVE = 15 * 60 * 1000; // 15 minutes, in milliseconds
+
 module.exports = {
 	fetch,
 };
@@ -67,12 +69,14 @@ function fetch(callback) {
 			}
 		}
 
-		// Emptying cached information after 15 minutes
+		// Add expiration date to the data to send to the client for later refresh
+		versions.expiresAt = Date.now() + TIME_TO_LIVE;
+
+		// Emptying cached information after reaching said expiration date
 		setTimeout(() => {
 			delete versions.current.changelog;
 			delete versions.latest;
-		}, 15 * 60 * 1000
-		);
+		}, TIME_TO_LIVE);
 
 		callback(versions);
 	});