Refactor multiple areas for simplicity and readability

This is a squash commit of many commits

app/commands/add-feed.js

@@ -14,31 +14,25 @@ module.exports = new Core.Command({
 });
 
 function invoke({ message, params, guildData, client }) {
-	const feedUrl = [...GetUrls(message.content)][0];
-	const channel = message.mentions.channels.first();
+	const feedUrl = [...GetUrls(message.content)][0],
+		channel = message.mentions.channels.first();
 
 	if (!feedUrl || !channel)
 		return Promise.reject("Please provide both a channel and an RSS feed URL. You can optionally @mention a role also.");
 
-	const role = message.mentions.roles.first();
-
-	const feedData = new FeedData({
-		url: feedUrl,
-		channelName: channel.name,
-		roleName: role ? role.name : null,
-		maxCacheSize: Config.maxCacheSize
-	});
+	const role = message.mentions.roles.first(),
+		feedData = new FeedData({
+			url: feedUrl,
+			channelID: channel.id,
+			roleID: role ? role.id : null,
+			maxCacheSize: Config.maxCacheSize
+		});
 
 	return new Promise((resolve, reject) => {
 		//ask the user if they're happy with the details they set up, save if yes, don't if no
 		Core.util.ask(client, message.channel, message.member, "Are you happy with this (yes/no)?\n" + feedData.toString())
 			.then(responseMessage => {
-
-				//if they responded yes, save the feed and let them know, else tell them to start again
 				if (responseMessage.content.toLowerCase() === "yes") {
-					if (!guildData)
-						guildData = new GuildData({ id: message.guild.id, feeds: [] });
-
 					guildData.feeds.push(feedData);
 					guildData.cachePastPostedLinks(message.guild)
 						.then(() => resolve("Your new feed has been saved!"));

app/index.js

@@ -16,15 +16,15 @@ client.on("beforeLogin", () => {
 client.on("ready", () => {
 	parseLinksInGuilds(client.guilds, client.guildsData)
 		.then(() => checkFeedsInGuilds(client.guildsData));
-});
 
-client.on("message", message => {
-	const guildData = client.guildsData[message.guild.id];
-	if (guildData)
-		guildData.feeds.forEach(feedData => {
-			if (message.channel.name === feedData.channelName)
-				feedData.cachedLinks.push(...GetUrls(message.content));
-		});
+	client.on("message", message => {
+		const guildData = client.guildsData[message.guild.id];
+		if (guildData)
+			guildData.feeds.forEach(feedData => {
+				if (message.channel.name === feedData.channelName)
+					feedData.cachedLinks.push(...GetUrls(message.content));
+			});
+	});
 });
 
 client.bootstrap();

app/models/feed-data.js

@@ -9,68 +9,78 @@ const GetUrls = require("get-urls"); //for extracting urls from messages
 const ShortID = require("shortid"); //to provide ids for each feed, allowing guilds to remove them
 
 module.exports = class FeedData {
-	constructor({ id = null, url, channelName, roleName, cachedLinks = null, maxCacheSize }) {
+	constructor({ id = null, url, channelID, roleID, cachedLinks = null, maxCacheSize }) {
 		this.id = id || ShortID.generate();
 		this.url = url;
-		this.channelName = channelName;
-		this.roleName = roleName;
+		this.channelID = channelID;
+		this.roleID = roleID;
 		this.cachedLinks = cachedLinks || [];
 		this.maxCacheSize = maxCacheSize || 10;
 
 		this.cachedLinks.push = (...elements) => {
-			const unique = elements
-				.map(el => normaliseUrl(el)) //normalise all the urls
-				.filter(el => !this.cachedLinks.includes(el)); //filter out any already cached
-			Array.prototype.push.apply(this.cachedLinks, unique);
+			Array.prototype.push.apply(
+				this.cachedLinks,
+				elements
+					.map(el => normaliseUrl(el))
+					.filter(el => !this.cachedLinks.includes(el))
+			);
 
-			if (this.cachedLinks.length > this.maxCacheSize)
-				this.cachedLinks.splice(0, this.cachedLinks.length - this.maxCacheSize); //remove the # of elements above the max from the beginning
+			//seeing as new links come in at the end of the array, we need to remove the old links from the beginning
+			this.cachedLinks.splice(0, this.cachedLinks.length - this.maxCacheSize);
 		};
 	}
 
 	/**@param param*/
 	updatePastPostedLinks(guild) {
-		const channel = guild.channels.find(ch => ch.type === "text" && ch.name === this.channelName);
-		
+		const channel = guild.channels.get(this.channelID);
+
+		if (!channel)
+			return Promise.reject("Channel not found!");
+
 		return new Promise((resolve, reject) => {
 			channel.fetchMessages({ limit: 100 })
 				.then(messages => {
-					new Map([...messages].reverse()).forEach(m => this.cachedLinks.push(...GetUrls(m.content))); //push all the links in each message into our links array
-					resolve(this);
+					/* we want to push the links in oldest first, but discord.js returns messages newest first, so we need to reverse them
+					 * discord.js returns a map, and maps don't have .reverse methods, hence needing to spread the elements into an array first */
+					[...messages.values()].reverse().forEach(m => this.cachedLinks.push(...GetUrls(m.content)));
+					resolve();
 				})
 				.catch(reject);
 		});
 	}
 
 	/**@param param */
-	check(guild) {
-		Dns.resolve(Url.parse(this.url).host || "", err => { //check we can resolve the host, so we can throw an appropriate error if it fails
+	fetchLatest(guild) {
+		Dns.resolve(Url.parse(this.url).host || "", err => {
 			if (err)
-				DiscordUtil.dateError("Connection Error: Can't resolve host", err); //log our error if we can't resolve the host
+				DiscordUtil.dateError("Connection Error: Can't resolve host", err.message || err);
 			else
-				FeedRead(this.url, (err, articles) => { //check the feed
-					if (err)
-						DiscordUtil.dateError(err);
-					else {
-						let latest = articles[0].link; //extract the latest link
-						latest = normaliseUrl(latest); //standardise it a bit
-
-						//if we don't have it cached already, cache it and callback
-						if (!this.cachedLinks.includes(latest)) {
-							this.cachedLinks.push(latest);
-
-							const channel = guild.channels.find(ch => ch.type === "text" && ch.name.toLowerCase() === this.channelName.toLowerCase());
-							const role = this.roleName ? guild.roles.find(role => role.name.toLowerCase() === this.roleName.toLowerCase()) : null;
-							channel.send((role ? role + " " : "") + latest).catch(err => DiscordUtil.dateError(`Error posting in ${channel.id}`));
-						}
-					}
-				});
+				this._doFetchRSS(guild);
 		});
 	}
 
 	toString() {
 		const blacklist = ["cachedLinks", "maxCacheSize"];
-		return `\`\`\`JavaScript\n ${JSON.stringify(this, (k, v) => !blacklist.includes(k) ? v : undefined, "\t")} \`\`\``;
+		return `\`\`\`JavaScript\n ${JSON.stringify(this, (k, v) => !blacklist.find(x => x === k) ? v : undefined, "\t")} \`\`\``;
+	}
+
+	_doFetchRSS(guild) {
+		FeedRead(this.url, (err, articles) => {
+			if (err)
+				return DiscordUtil.dateError(err.message || err);
+
+			const latest = normaliseUrl(articles[0].link);
+
+			if (!this.cachedLinks.includes(latest)) {
+				this.cachedLinks.push(latest);
+
+				const channel = guild.channels.get(this.channelID),
+					role = guild.roles.get(this.roleID);
+
+				channel.send((role ? role + " " : "") + latest)
+					.catch(err => DiscordUtil.dateError(`Error posting in ${channel.id}: ${err.message || err}`));
+			}
+		});
 	}
 };
 

app/models/guild-data.js

@@ -2,23 +2,26 @@ const DiscordUtil = require("../../discord-bot-core").util;
 const Core = require("../../discord-bot-core");
 const FeedData = require("./feed-data.js");
 
-module.exports = class GuildData extends Core.BaseGuildData{
-	constructor({ id, feeds }) {
+module.exports = class GuildData extends Core.BaseGuildData {
+	constructor({ id, feeds = [] }) {
 		super(id);
-		this.feeds = (feeds || []).map(feed => new FeedData(feed));
+		this.feeds = feeds.map(feed => new FeedData(feed));
 	}
 
 	cachePastPostedLinks(guild) {
 		const promises = [];
 
-		this.feeds.forEach(feed => {
-			promises.push(feed.updatePastPostedLinks(guild).catch(err => DiscordUtil.dateError(`Error reading history in ${err.path}`)));
-		});
+		this.feeds.forEach(feed =>
+			promises.push(
+				feed.updatePastPostedLinks(guild)
+					.catch(err => DiscordUtil.dateError(`Error reading history in ${err.path}`))
+			)
+		);
 
 		return Promise.all(promises);
 	}
 
 	checkFeeds(guilds) {
-		this.feeds.forEach(feed => feed.check(guilds.get(this.id)));
+		this.feeds.forEach(feed => feed.fetchLatest(guilds.get(this.id)));
 	}
 };

discord-bot-core/Client.js

@@ -2,7 +2,7 @@ const FileSystem = require("fs");
 const Discord = require("discord.js");
 const JsonFile = require("jsonfile");
 const RequireAll = require("require-all");
-const CoreUtil = require("./util.js");
+const CoreUtil = require("./Util.js");
 const HandleMessage = require("./HandleMessage.js");
 // @ts-ignore
 const InternalConfig = require("./internal-config.json");
@@ -49,7 +49,7 @@ module.exports = class Client extends Discord.Client {
 	onMessage(message) {
 		if (message.channel.type === "text" && message.member) {
 			if(!this.guildsData[message.guild.id])
-				this.guildsData[message.guild.id] = new this.guildDataModel(message.guild.id);
+				this.guildsData[message.guild.id] = new this.guildDataModel({ id: message.guild.id });
 			HandleMessage(this, message, this.commands, this.guildsData[message.guild.id]);
 		}
 	}

discord-bot-core/index.js

@@ -2,7 +2,7 @@
 const InternalConfig = require("./internal-config.json");
 
 module.exports = {
-	Client: require("./client.js"),
+	Client: require("./Client.js"),
 	BaseGuildData: require("./BaseGuildData.js"),
 	Command: require("./Command.js"),
 	util: require("./Util.js"),