Merge pull request #1814 from codidact/0valt/voting-improvements

JSON response handling improvements

Author: Oaphi

app/assets/javascripts/filters.js

@@ -122,7 +122,7 @@ $(() => {
     async function saveFilter() {
       if (!$form[0].reportValidity()) { return; }
 
-      const filter = /** @type {QPixelFlag} */({});
+      const filter = /** @type {QPixelFilter} */({});
 
       for (const el of $formFilters) {
         filter[el.dataset.name] = $(el).val();

app/assets/javascripts/notifications.js

@@ -21,6 +21,9 @@ $(() => {
     return template;
   };
 
+  /**
+   * @param {number} change 
+   */
   const changeInboxCount = (change) => {
     const counter = $('.inbox-count');
     let count;

app/assets/javascripts/qpixel_api.js

@@ -110,7 +110,7 @@ window.QPixel = {
   },
 
   /**
-   * @type {QPixelFlag[]|null}
+   * @type {QPixelFilter[]|null}
    */
   _filters: null,
 
@@ -206,7 +206,8 @@ window.QPixel = {
       headers: { 'Accept': 'application/json' }
     });
 
-    const data = await resp.json();
+    /** @type {QPixelResponseJSON<{ preferences: UserPreferences }>} */
+    const data = await QPixel.parseJSONResponse(resp, 'Failed to save preference');
 
     QPixel.handleJSONResponse(data, (data) => {
       QPixel._updatePreferencesLocally(data.preferences);
@@ -239,27 +240,19 @@ window.QPixel = {
     return data.name;
   },
 
-  setFilterAsDefault: async (categoryId, name) => {
-    await QPixel.fetchJSON(`/categories/${categoryId}/filters/default`, { name }, {
-      headers: { 'Accept': 'application/json' }
-    });
-  },
-
   setFilter: async (name, filter, category, isDefault) => {
     const resp = await QPixel.fetchJSON('/users/me/filters',
       Object.assign(filter, {name, category, is_default: isDefault}), {
         headers: { 'Accept': 'application/json' }
       });
+
+    /** @type {QPixelResponseJSON<{ filters: QPixelFilter[] }>} */
+    const data = await QPixel.parseJSONResponse(resp, 'Failed to save filter');
 
-    const data = await resp.json();
-    if (data.status !== 'success') {
-      console.error(`Filter persist failed (${name})`);
-      console.error(resp);
-    }
-    else {
+    QPixel.handleJSONResponse(data, (data) => {
       this._filters = data.filters;
       QPixel.Storage?.set('user_filters', this._filters);
-    }
+    });
   },
 
   deleteFilter: async (name, system = false) => {
@@ -268,16 +261,13 @@ window.QPixel = {
       method: 'DELETE'
     });
 
-    const data = await resp.json();
+    /** @type {QPixelResponseJSON<{ filters: QPixelFilter[] }>} */
+    const data = await QPixel.parseJSONResponse(resp, 'Failed to delete filter');
 
-    if (data.status !== 'success') {
-      console.error(`Filter deletion failed (${name})`);
-      console.error(resp);
-    }
-    else {
+    QPixel.handleJSONResponse(data, (data) => {
       this._filters = data.filters;
       QPixel.Storage?.set('user_filters', this._filters);
-    }
+    });
   },
 
   _preferencesLocalStorageKey: () => {
@@ -410,11 +400,30 @@ window.QPixel = {
     return content;
   },
 
+  parseJSONResponse: async (response, errorMessage) => {
+    try {
+      const data = await response.json();
+
+      return data;
+    }
+    catch (error) {
+      if (response.ok) {
+        console.error(error);
+      }
+
+      return {
+        status: 'failed',
+        message: errorMessage
+      };
+    }
+  },
+
   handleJSONResponse: (data, onSuccess, onFinally) => {
+    const is_modified = data.status === 'modified';
     const is_success = data.status === 'success';
 
-    if (is_success) {
-      onSuccess(data);
+    if (is_modified || is_success) {
+      onSuccess(/** @type {Parameters<typeof onSuccess>[0]} */(data));
     }
     else {
       QPixel.createNotification('danger', data.message);
@@ -430,9 +439,16 @@ window.QPixel = {
       headers: { 'Accept': 'application/json' }
     });
 
-    const data = await resp.json();
+    return QPixel.parseJSONResponse(resp, 'Failed to flag');
+  },
 
-    return data;
+  vote: async (postId, voteType) => {
+    const resp = await QPixel.fetchJSON('/votes/new', {
+      post_id: postId,
+      vote_type: voteType
+    });
+
+    return QPixel.parseJSONResponse(resp, 'Failed to vote');
   },
 
   deleteComment: async (id) => {
@@ -441,19 +457,15 @@ window.QPixel = {
       method: 'DELETE'
     });
 
-    const data = await resp.json();
-
-    return data;
+    return QPixel.parseJSONResponse(resp, 'Failed to delete comment');
   },
 
   followComments: async (postId) => {
     const resp = await QPixel.fetchJSON(`/comments/post/${postId}/follow`, {}, {
       headers: { 'Accept': 'application/json' }
     });
 
-    const data = await resp.json();
-
-    return data;
+    return QPixel.parseJSONResponse(resp, 'Failed to follow post comments');
   },
 
   deleteDraft: async () => {
@@ -463,9 +475,7 @@ window.QPixel = {
       headers: { 'Accept': 'application/json' }
     });
 
-    const data = await resp.json();
-
-    return data;
+    return QPixel.parseJSONResponse(resp, 'Failed to delete post draft');
   },
 
   undeleteComment: async (id) => {
@@ -474,39 +484,37 @@ window.QPixel = {
       method: 'PATCH'
     });
 
-    const data = await resp.json();
-
-    return data;
+    return QPixel.parseJSONResponse(resp, 'Failed to undelete comment');
   },
 
   unfollowComments: async (postId) => {
     const resp = await QPixel.fetchJSON(`/comments/post/${postId}/unfollow`, {}, {
       headers: { 'Accept': 'application/json' }
     });
 
-    const data = await resp.json();
-
-    return data;
+    return QPixel.parseJSONResponse(resp, 'Failed to unfollow post comments');
   },
 
   lockThread: async (id) => {
     const resp = await QPixel.fetchJSON(`/comments/thread/${id}/restrict`, {
       type: 'lock'
     });
 
-    const data = await resp.json();
-
-    return data;
+    return QPixel.parseJSONResponse(resp, 'Failed to lock thread');
   },
 
   renameTag: async (categoryId, tagId, name) => {
     const resp = await QPixel.fetchJSON(`/categories/${categoryId}/tags/${tagId}/rename`, { name }, {
       headers: { 'Accept': 'application/json' }
     });
 
-    const data = await resp.json();
+    return QPixel.parseJSONResponse(resp, 'Failed to rename tag');
+  },
 
-    return data;
+  retractVote: async (id) => {
+    const resp = await QPixel.fetchJSON(`/votes/${id}`, {}, { method: 'DELETE' });
+
+    return QPixel.parseJSONResponse(resp, 'Failed to retract vote');
   },
 
   saveDraft: async (draft) => {
@@ -515,8 +523,6 @@ window.QPixel = {
       path: location.pathname
     });
 
-    const data = await resp.json();
-
-    return data;
+    return QPixel.parseJSONResponse(resp, 'Failed to save draft');
   },
 };

app/assets/javascripts/votes.js

@@ -1,4 +1,4 @@
-$(() => {
+document.addEventListener('DOMContentLoaded', () => {
   $(document).on('click', '.vote-button', async (evt) => {
     const $tgt = $(evt.target).is('button') ? $(evt.target) : $(evt.target).parents('button');
     const $post = $tgt.parents('.post');
@@ -7,38 +7,27 @@ $(() => {
 
     const $up = $container.find('.js-upvote-count');
     const $down = $container.find('.js-downvote-count');
+    const postId = $post.data('post-id');
     const voteType = $tgt.data('vote-type');
     const voted = $tgt.hasClass('is-active');
 
     if (voted) {
       const voteId = $tgt.attr('data-vote-id');
 
-      const resp = await QPixel.fetchJSON(`/votes/${voteId}`, {}, { method: 'DELETE' });
+      const data = await QPixel.retractVote(voteId);
 
-      const data = await resp.json();
-
-      if (data.status === 'OK') {
+      QPixel.handleJSONResponse(data, (data) => {
         $up.text(`+${data.upvotes}`);
         $down.html(`−${data.downvotes}`);
         $container.attr("title", `Score: ${data.score}`);
         $tgt.removeClass('is-active')
             .removeAttr('data-vote-id');
-      }
-      else {
-        console.error('Vote delete failed');
-        console.log(resp);
-        QPixel.createNotification('danger', `<strong>Failed:</strong> ${data.message} (${resp.status})`);
-      }
+      });
     }
     else {
-      const resp = await QPixel.fetchJSON('/votes/new', {
-        post_id: $post.data('post-id'),
-        vote_type: voteType
-      });
+      const data = await QPixel.vote(postId, voteType);
 
-      const data = await resp.json();
-
-      if (data.status === 'modified' || data.status === 'OK') {
+      QPixel.handleJSONResponse(data, (data) => {
         $up.text(`+${data.upvotes}`);
         $down.html(`&minus;${data.downvotes}`);
         $container.attr("title", `Score: ${data.score}`);
@@ -50,12 +39,7 @@ $(() => {
           $oppositeVote.removeClass('is-active')
                        .removeAttr('data-vote-id');
         }
-      }
-      else {
-        console.error('Vote create failed');
-        console.log(resp);
-        QPixel.createNotification('danger', `<strong>Failed:</strong> ${data.message} (${resp.status})`);
-      }
+      });
     }
   });
 });

app/controllers/votes_controller.rb

@@ -42,7 +42,7 @@ def create
     AbilityQueue.add(post.user, "Vote Change on ##{post.id}")
 
     modified = !destroyed.empty?
-    state = { status: (modified ? 'modified' : 'OK'),
+    state = { status: (modified ? 'modified' : 'success'),
               vote_id: vote.id,
               upvotes: post.upvote_count,
               downvotes: post.downvote_count,
@@ -62,7 +62,7 @@ def destroy
 
     if vote.destroy
       AbilityQueue.add(post.user, "Vote Change on ##{post.id}")
-      render json: { status: 'OK',
+      render json: { status: 'success',
                      upvotes: post.upvote_count,
                      downvotes: post.downvote_count,
                      score: post.score }