Use AJAX for notifications table

[zeripath]

Use AJAX to update the notifications count and table.
Delay calculating notification count until rendering to prevent initial incorrect display of notification count.

Signed-off-by: Andrew Thornton [email protected]

[codecov-io]

Codecov Report

Merging #10961 into master will decrease coverage by 0.00%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##           master   #10961      +/-   ##
==========================================
- Coverage   43.34%   43.33%   -0.01%     
==========================================
  Files         601      601              
  Lines       85675    85694      +19     
==========================================
+ Hits        37133    37139       +6     
- Misses      43956    43969      +13     
  Partials     4586     4586              

Impacted Files	Coverage Δ
modules/setting/setting.go	45.28% <ø> (ø)
routers/user/notification.go	39.70% <25.00%> (-5.46%)	⬇️
modules/templates/helper.go	43.03% <100.00%> (+0.84%)	⬆️
services/pull/update.go	52.54% <0.00%> (-6.78%)	⬇️
services/pull/temp_repo.go	31.62% <0.00%> (-2.57%)	⬇️
services/pull/pull.go	33.20% <0.00%> (-0.20%)	⬇️
modules/git/repo.go	51.88% <0.00%> (+3.76%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e74c4e1...edc068d. Read the comment docs.

[6543]

Did not worked at all :/
-> only open related issue works
if you klick on pin/read/unread it also open the related issue/pull

[zeripath]

@6543 what browser are you using? Are you sure that you cleaned out your service worker?

[6543]

@6543 what browser are you using? Are you sure that you cleaned out your service worker?

Crome and Firefox; and yes

[zeripath]

@6543 I'm confused because it genuinely works here. Are you able to just double check that the index.js contains #notification_table?

[techknowlogick]

@6543 is service worker caching the old JS for you?

[6543]

sorry for the late responce I'll thest this again ... EDIT: no I'm sure it doesnt work :(

[6543]

update fix: e1d913c5b76cf20ab88a5f0b1ad3e5d799826c0c worked
and 5ef3dd983946e54748c975d04a03a6b35deb0581 do the job 👍

[6543]

make ajax first - ged rid of page.reload() after

[jaqra]

$notificationTable will be never null

[6543]

I'll have a look at this later :)

[jolheiser]

Is there supposed to be an ever-present label?

Also, perhaps they can be hidden on-click? I tend to open my notifications with middle-click (open in a new tab), which then means I end up with something like this.

[jolheiser]

I believe the class for hiding is actually hidden
Alternatively there is invisible if you only want to make them invisible.

[jolheiser]

Sorry, I looked into the CSS classes a little more. hide does work, however it's being overridden by .ui.label whereas .hidden uses !important 🙃

[6543]

I realy like this pull 👍

[6543]

thanks 👍 ... one thing 😅 a config option need it's documentation :D

[6543]

I would say kind/feature ?

[6543]

this pull close #10198

[silverwind]

Some suggestions:

diff --git a/.eslintrc b/.eslintrc
index 8fd53d54a..a8f7f1ae2 100644
--- a/.eslintrc
+++ b/.eslintrc
@@ -54,8 +54,9 @@ rules:
   no-new: [0]
   no-param-reassign: [0]
   no-plusplus: [0]
   no-restricted-syntax: [0]
+  no-return-await: [0]
   no-shadow: [0]
   no-unused-vars: [2, {args: all, argsIgnorePattern: ^_, varsIgnorePattern: ^_, ignoreRestSiblings: true}]
   no-use-before-define: [0]
   no-var: [2]
diff --git a/web_src/js/features/notification.js b/web_src/js/features/notification.js
index 37369df2e..f2bde28a0 100644
--- a/web_src/js/features/notification.js
+++ b/web_src/js/features/notification.js
@@ -1,19 +1,19 @@
 const {AppSubUrl, csrf, NotificationSettings} = window.config;
 
 export function initNotificationsTable() {
-  $('#notification_table .button').click(function () {
-    updateNotification(
+  $('#notification_table .button').on('click', async function () {
+    const data = await updateNotification(
       $(this).data('url'),
       $(this).data('status'),
       $(this).data('page'),
       $(this).data('q'),
       $(this).data('notification-id')
-    ).then((data) => {
-      $('#notification_div').replaceWith(data);
-      initNotificationsTable();
-      updateNotificationCount();
-    });
+    );
+
+    $('#notification_div').replaceWith(data);
+    initNotificationsTable();
+    await updateNotificationCount();
     return false;
   });
 }
 
@@ -24,63 +24,61 @@ export function initNotificationCount() {
 
   if ($('.notification_count').length > 0) {
     const lastCount = $('.notification_count').text();
     const fn = (callback, timeout, lastCount) => {
-      setTimeout(() => {
-        updateNotificationCount(callback, timeout, lastCount);
+      setTimeout(async () => {
+        await updateNotificationCount(callback, timeout, lastCount);
       }, timeout);
     };
 
     fn(fn, NotificationSettings.MinTimeout, lastCount);
   }
 }
 
-function updateNotificationCount(callback, timeout, lastCount) {
-  const currentCount = $('.notification_count').text();
+async function updateNotificationCount(callback, timeout, lastCount) {
+  let currentCount = $('.notification_count').text();
   if (callback && (lastCount !== currentCount)) {
     callback(callback, NotificationSettings.MinTimeout, currentCount);
     return;
   }
-  $.ajax({
+
+  const data = await $.ajax({
     type: 'GET',
     url: `${AppSubUrl}/api/v1/notifications/new`,
     headers: {
       'X-Csrf-Token': csrf,
     },
-  }).then((data) => {
-    const notificationCount = $('.notification_count');
-    const notificationDependent = $('.notification_dependent');
-    if (data.new === 0) {
-      notificationCount.addClass('hidden');
-      notificationDependent.addClass('hide');
-    } else {
-      notificationCount.removeClass('hidden');
-      notificationDependent.removeClass('hide');
-    }
-    const currentCount = $('.notification_count').text();
-    if (lastCount !== `${data.new}` || currentCount !== `${data.new}`) {
-      notificationCount.text(data.new);
-      timeout = NotificationSettings.MinTimeout;
-    } else if (timeout < NotificationSettings.MaxTimeout) {
-      timeout += NotificationSettings.TimeoutStep;
-    }
-    return {
-      timeout,
-      nextCount: `${data.new}`,
-    };
-  }).then((data) => {
-    if (callback) {
-      callback(callback, data.timeout, data.nextCount);
-    }
   });
+
+  const notificationCount = $('.notification_count');
+  const notificationDependent = $('.notification_dependent');
+  if (data.new === 0) {
+    notificationCount.addClass('hidden');
+    notificationDependent.addClass('hide');
+  } else {
+    notificationCount.removeClass('hidden');
+    notificationDependent.removeClass('hide');
+  }
+
+  currentCount = $('.notification_count').text();
+  if (lastCount !== `${data.new}` || currentCount !== `${data.new}`) {
+    notificationCount.text(data.new);
+    timeout = NotificationSettings.MinTimeout;
+  } else if (timeout < NotificationSettings.MaxTimeout) {
+    timeout += NotificationSettings.TimeoutStep;
+  }
+
+  if (callback) {
+    callback(callback, timeout, `${data.new}`);
+  }
 }
 
 async function updateNotification(url, status, page, q, notificationID) {
   if (status !== 'pinned') {
     $(`#notification_${notificationID}`).remove();
   }
 
-  return $.ajax({
+  return await $.ajax({
     type: 'POST',
     url,
     data: {
       _csrf: csrf,