refactor: Removing then/catch from async/await calls (#22)

* refactored async/await for import helper to not also use then/catch

* added enum

* refactor webhookHelper and tests to not use then/catch

* changed docstring

* refactoring bot and tests to not use then/catch

* refactoring commands.js and tests to not use then/catch

* refactoring memberHelper.js and tests to not use then/catch

* removing then/catch from messageHelper.test.js

* fixed set up for commands tests

* edited bot to have top level main function

* one more test in commands.js, and removed console.error

* fixed typo in webhookHelper

* forgot to switch over some tests in bot.test and commands.test

* removed console.log from import helper

* put console.error in commands

* converted utils.js to not use then/catch

* tested utils checkImageFormatValidity

* removed jest-fetch-mock since it turns out I was just manually mocking it anyway

* refactored database to not use then/catch

* added dash to commands.js and test to pass

* added the remaining webhook tests

* changed utils to check for 10MB size not 1MB

* removed unnecessary try/catch from utils

* Simplify getWebhook to use .find() instead of foreach logic

* make memberCommand exit when error occurs with parseMemberCommand

* changed commands.js to not have user interaction within the catch

* updated console.error message in database.js

* made importHelper mock throw error instead of "resolve" error

* replaced "pk;" with "pf;" in test

* Got rid of unnecessary check for empty message from user (Fluxer doesn't allow this to happen)

Removed export of token

* getAllMembersInfo checks for fields.length

* added default case to memberCommandHandler to throw error if command is not recognized

* reversed check for valid proxy (was returning valid if the proxy existed and invalid if it didn't)

* pushes e.message instead of full error object to errors array in importHelper

* adjusted tests to properly use mockRejectedValue for async rejections

* changed getAllMembersInfo map to say `index` not `name` as it actually gets the index of a member and then the member object

* adjusted importHelper to properly test throwing of aggregate error

* revamped setting of expiration warning (moved to utils and changed logic, wrote tests)

---------

Co-authored-by: Aster Fialla <asterfialla@gmail.com>

src/enums.js

@@ -44,7 +44,10 @@ helperEnums.help = {
 }
 
 helperEnums.misc = {
-    ATTACHMENT_SENT_BY: "Attachment sent by:"
+    ATTACHMENT_SENT_BY: "Attachment sent by:",
+    ATTACHMENT_EXPIRATION_WARNING: "**NOTE:** Because this profile picture is hosted on Fluxer, it will expire. To avoid this, upload the picture to another website like <https://imgbb.com/> and link to it directly.",
+    FLUXER_ATTACHMENT_URL: "https://fluxerusercontent.com/attachments/"
+
 }
 
 export const enums = helperEnums;

src/helpers/memberHelper.js

@@ -283,8 +283,8 @@ mh.updatePropic = async function (authorId, memberName, values, attachmentUrl =
     const imgUrl = values ?? attachmentUrl;
     // Throws error if invalid
     await utils.checkImageFormatValidity(imgUrl);
-
-    return await mh.updateMemberField(authorId, memberName, "propic", imgUrl, attachmentExpiration);
+    const expirationWarning = utils.setExpirationWarning(imgUrl, attachmentExpiration);
+    return await mh.updateMemberField(authorId, memberName, "propic", imgUrl, expirationWarning);
 }
 
 /**
@@ -369,15 +369,15 @@ mh.addFullMember = async function (authorId, memberName, displayName = null, pro
     if (propic && propic.length > 0) {
         try {
             isValidPropic = await utils.checkImageFormatValidity(propic);
-
         }
         catch(e) {
             errors.push(`Tried to set profile picture to \"${propic}\". ${e.message}. ${enums.err.SET_TO_NULL}`);
             isValidPropic = false;
         }
     }
-    if (isValidPropic && attachmentExpiration) {
-        errors.push(mh.setExpirationWarning(attachmentExpiration));
+    const expirationWarning = utils.setExpirationWarning(propic, attachmentExpiration);
+    if (expirationWarning) {
+        errors.push(expirationWarning);
     }
     const member = await database.members.create({
         name: memberName, userid: authorId, displayname: isValidDisplayName ? displayName : null, proxy: isValidProxy ? proxy : null, propic: isValidPropic ? propic : null
@@ -394,17 +394,11 @@ mh.addFullMember = async function (authorId, memberName, displayName = null, pro
  * @param {string} memberName - The member to update
  * @param {string} columnName - The column name to update.
  * @param {string} value - The value to update to.
- * @param {string | null} [attachmentExpiration] - The attachment expiration date (if any)
+ * @param {string | null} [expirationWarning] - The attachment expiration warning (if any)
  * @returns {Promise<string>} A successful update.
  * @throws {Error} When no member row was updated.
  */
-mh.updateMemberField = async function (authorId, memberName, columnName, value, attachmentExpiration = null) {
-    let fluxerPropicWarning;
-
-    // indicates that an attachment was uploaded on Fluxer directly
-    if (columnName === "propic" && attachmentExpiration) {
-        fluxerPropicWarning = mh.setExpirationWarning(value);
-    }
+mh.updateMemberField = async function (authorId, memberName, columnName, value, expirationWarning = null) {
     const res = await database.members.update({[columnName]: value}, {
         where: {
             name: {[Op.iLike]: memberName},
@@ -414,21 +408,7 @@ mh.updateMemberField = async function (authorId, memberName, columnName, value,
     if (res[0] === 0) {
         throw new Error(`Can't update ${memberName}. ${enums.err.NO_MEMBER}.`);
     } else {
-        return `Updated ${columnName} for ${memberName} to ${value}${fluxerPropicWarning ?? ''}.`;
-    }
-}
-
-/**
- * Sets the warning for an expiration date.
- *
- * @param {string} expirationString - An expiration date string.
- * @returns {string} A description of the expiration, interpolating the expiration string.
- */
-mh.setExpirationWarning = function (expirationString) {
-    let expirationDate = new Date(expirationString);
-    if (!isNaN(expirationDate.valueOf())) {
-        expirationDate = expirationDate.toDateString();
-        return `\n**NOTE:** Because this profile picture was uploaded via Fluxer, it will currently expire on *${expirationDate}*. To avoid this, upload the picture to another website like <https://imgbb.com/> and link to it directly`
+        return `Updated ${columnName} for ${memberName} to ${value}${expirationWarning ? `. ${expirationWarning}.` : '.'}`;
     }
 }
 

src/helpers/utils.js

@@ -34,4 +34,24 @@ u.checkImageFormatValidity = async function (imageUrl) {
     return true;
 }
 
+/**
+ * Sets the warning that a Fluxer-uploaded image will expire.
+ *
+ * @param {string | null} [imgUrl] - An image URL.
+ * @param {string | null} [expirationString] - An expiration date string.
+ * @returns {string | null} A description of the expiration, or null.
+ */
+u.setExpirationWarning = function (imgUrl = null, expirationString = null) {
+    if (imgUrl && imgUrl.startsWith(enums.misc.FLUXER_ATTACHMENT_URL)) {
+        return enums.misc.ATTACHMENT_EXPIRATION_WARNING;
+    }
+    else if (expirationString) {
+        let expirationDate = new Date(expirationString);
+        if (!isNaN(expirationDate.valueOf())) {
+            return `${enums.misc.ATTACHMENT_EXPIRATION_WARNING}. Expiration date: *${expirationString}*.`;
+        }
+    }
+    return null;
+}
+
 export const utils = u;

tests/helpers/memberHelper.test.js

@@ -39,7 +39,7 @@ describe('MemberHelper', () => {
         name: "somePerson",
         displayname: "Some Person",
         proxy: "--text",
-        propic: attachmentUrl
+        propic: 'ono.png'
     }
 
     beforeEach(() => {
@@ -446,14 +446,13 @@ describe('MemberHelper', () => {
 
     describe('updatePropic', () => {
         test.each([
-            [null, attachmentUrl, null, attachmentUrl],
-            [mockMember.propic, null, null, mockMember.propic],
-            [mockMember.propic, attachmentUrl, null, attachmentUrl],
-            [null, attachmentUrl, attachmentExpiration, attachmentUrl]
+            [null, attachmentUrl, undefined, attachmentUrl],
+            [mockMember.propic, null, undefined, mockMember.propic],
+            [mockMember.propic, attachmentUrl, undefined, mockMember.propic],
         ])('calls checkImageFormatValidity and updateMemberField and returns string', async (imgUrl, attachmentUrl, attachmentExpiration, expected) => {
             // Arrange
-
             jest.spyOn(memberHelper, 'updateMemberField').mockResolvedValue("Updated");
+            utils.setExpirationWarning = jest.fn().mockReturnValue(undefined);
             // Act
             const result = await memberHelper.updatePropic(authorId, mockMember.name, imgUrl, attachmentUrl, attachmentExpiration);
             // Assert
@@ -461,7 +460,21 @@ describe('MemberHelper', () => {
             expect(utils.checkImageFormatValidity).toHaveBeenCalledTimes(1);
             expect(utils.checkImageFormatValidity).toHaveBeenCalledWith(expected);
             expect(memberHelper.updateMemberField).toHaveBeenCalledTimes(1);
-            expect(memberHelper.updateMemberField).toHaveBeenCalledWith(authorId, mockMember.name, "propic", expected, attachmentExpiration);
+            expect(memberHelper.updateMemberField).toHaveBeenCalledWith(authorId, mockMember.name, "propic", expected, undefined);
+        })
+
+        test('calls setExpirationWarning', async() => {
+            // Arrange
+            jest.spyOn(memberHelper, 'updateMemberField').mockResolvedValue("Updated");
+            utils.setExpirationWarning = jest.fn().mockReturnValue(enums.misc.ATTACHMENT_EXPIRATION_WARNING);
+            // Act
+            const result = await memberHelper.updatePropic(authorId, mockMember.name, null, attachmentUrl, attachmentExpiration);
+            // Assert
+            expect(result).toEqual("Updated");
+            expect(utils.setExpirationWarning).toHaveBeenCalledTimes(1);
+            expect(utils.setExpirationWarning).toHaveBeenCalledWith(attachmentUrl, attachmentExpiration);
+            expect(memberHelper.updateMemberField).toHaveBeenCalledTimes(1);
+            expect(memberHelper.updateMemberField).toHaveBeenCalledWith(authorId, mockMember.name, "propic", attachmentUrl, enums.misc.ATTACHMENT_EXPIRATION_WARNING);
         })
     })
 
@@ -612,6 +625,17 @@ describe('MemberHelper', () => {
             expect(database.members.create).toHaveBeenCalledTimes(1);
         })
 
+        test('calls setExpirationWarning if attachmentExpiration exists', async () => {
+            // Arrange
+            utils.checkImageFormatValidity = jest.fn().mockResolvedValue(true);
+            jest.spyOn(memberHelper, 'setExpirationWarning').mockReturnValue(`${enums.misc.ATTACHMENT_EXPIRATION_WARNING}`);
+            // Act
+            await memberHelper.addFullMember(authorId, mockMember.name, null, null, mockMember.propic, attachmentExpiration)
+            // Assert
+            expect(memberHelper.setExpirationWarning).toHaveBeenCalledTimes(1);
+            expect(memberHelper.setExpirationWarning).toHaveBeenCalledWith(mockMember.propic, attachmentExpiration);
+        })
+
         test('if all values are valid, call database.members.create', async () => {
             // Arrange
             jest.spyOn(memberHelper, 'checkIfProxyExists').mockResolvedValue(false);
@@ -644,22 +668,18 @@ describe('MemberHelper', () => {
             };
         })
 
-        test('calls setExpirationWarning if attachmentExpiration', async () => {
-            await memberHelper.updateMemberField(authorId, mockMember.name, "propic", mockMember.propic, attachmentExpiration)
-            expect(memberHelper.setExpirationWarning).toHaveBeenCalledTimes(1);
-            expect(memberHelper.setExpirationWarning).toHaveBeenCalledWith(mockMember.propic);
-        })
-
         test.each([
-            ['name', mockMember.name, null, `Updated name for ${mockMember.name} to ${mockMember.name}`],
-            ['displayname', mockMember.displayname, null, `Updated name for ${mockMember.name} to ${mockMember.displayname}`],
-            ['proxy', mockMember.proxy, null, `Updated name for ${mockMember.name} to ${mockMember.proxy}`],
-            ['propic', mockMember.propic, null, `Updated name for ${mockMember.name} to ${mockMember.propic}`],
-            ['propic', mockMember.propic, attachmentExpiration, `Updated name for ${mockMember.name} to ${mockMember.propic} warning}`]
-        ])('calls database.members.update with correct column and value and return string', async (columnName, value, attachmentExpiration) => {
+            ['name', mockMember.name, undefined, `Updated name for ${mockMember.name} to ${mockMember.name}.`],
+            ['displayname', mockMember.displayname, undefined, `Updated displayname for ${mockMember.name} to ${mockMember.displayname}.`],
+            ['proxy', mockMember.proxy, undefined, `Updated proxy for ${mockMember.name} to ${mockMember.proxy}.`],
+            ['propic', mockMember.propic, undefined, `Updated propic for ${mockMember.name} to ${mockMember.propic}.`],
+            ['propic', mockMember.propic,
+                'warning', `Updated propic for ${mockMember.name} to ${mockMember.propic}. warning.`]
+        ])('calls database.members.update with correct column and value and return string', async (columnName, value, attachmentExpiration, expected) => {
             // Act
-            await memberHelper.updateMemberField(authorId, mockMember.name, columnName, value, attachmentExpiration)
+            const res = await memberHelper.updateMemberField(authorId, mockMember.name, columnName, value, attachmentExpiration)
             // Assert
+            expect(res).toEqual(expected);
             expect(database.members.update).toHaveBeenCalledTimes(1);
             expect(database.members.update).toHaveBeenCalledWith({[columnName]: value}, {
                 where: {

tests/helpers/utils.test.js

@@ -5,6 +5,7 @@ const {utils} = require("../../src/helpers/utils.js");
 describe('utils', () => {
 
     const attachmentUrl = 'oya.png';
+    const expirationString = new Date("2026-01-01").toDateString();
     let blob;
 
     beforeEach(() => {
@@ -60,6 +61,28 @@ describe('utils', () => {
         })
     })
 
+    describe('setExpirationWarning', () => {
+        test('sets warning if image Url starts with Fluxer host', () => {
+            // Act
+            const result = utils.setExpirationWarning(`${enums.misc.FLUXER_ATTACHMENT_URL}${attachmentUrl}`);
+            // Assert
+            expect(result).toEqual(enums.misc.ATTACHMENT_EXPIRATION_WARNING);
+        })
+
+        test('sets warning if expiration string exists', () => {
+            const result = utils.setExpirationWarning(null, expirationString);
+            // Assert
+            expect(result).toEqual(`${enums.misc.ATTACHMENT_EXPIRATION_WARNING}. Expiration date: *${expirationString}*.`);
+        })
+
+       test('returns null if img url does not start iwth fluxer host and no expiration', () => {
+           // Act
+           const result = utils.setExpirationWarning(attachmentUrl);
+           // Assert
+           expect(result).toBeNull();
+       })
+    })
+
     afterEach(() => {
         // restore the spy created with spyOn
         jest.restoreAllMocks();