Desktop Notifications for flags and queue items

After our recent election, I got elected as a moderator here on Code Review (Thank you, community members!). One of the first things I missed among the moderator tools was desktop notifications for when there were any flags.

As no current tool existed, I figured that I just have to create one...

Following the approach of "Disassemble your problems so you can assemble your solutions" (which I believe that I normally follow whether I am aware of it or not) I figured that I am gonna need the following things:

• Desktop Notifications

• Automatically reload the page (could also possibly work with AJAX requests)

• jQuery

• It should be functional on Stack Exchange

• Load/Save local data

At first I used some advice from SO and a general Stack Exchange userscript template, but later I realized I didn't need those things (and they just prevented me from using some GM_* functions).

The script adds a link on the /review URL (of any Stack Exchange site) to /review/desktop-notifications, which technically leads to a 404 page but gets some added information thanks to the user-script.

The /review link looks like this:

and /review/desktop-notifications looks like:

A desktop notification looks like the following:

The source code is available as a part of my SE-Scripts GitHub repository. Any feature-requests and pull requests are appreciated.

The code has been tested with Tampermonkey 3.11 for Google Chrome, but I believe it should work with other user-script handlers as well.

Code

```
// ==UserScript==
// @name Moderator Flag Notification
// @author Simon Forsberg
// @description Shows a desktop notification when there are flags or review items to be handled.
// @namespace https://github.com/Zomis/SE-Scripts
// @grant GM_getValue
// @grant GM_setValue
// @grant GM_notification
// @match ://.stackexchange.com/review*
// @match ://.stackoverflow.com/review*
// @match ://.superuser.com/review*
// @match ://.serverfault.com/review*
// @match ://.askubun

Magic Numbers

You did a very nice job of eliminating all magic numbers/values in the top of your code. However, there is still this one line:

var DELAY = 60 * 1000;

Where did 60 come from? And 1000?

These are still magic numbers, but they are not worthy of new constants; I would just add a simple comment explaining what DELAY is being set to.

Thanks to Mast for this:


var DELAY = 60 * 1000; usually gets a comment in my code indicating
nature should be blamed if they ever need to be changed. Also read
100 is a magic
number.
Explanation of what the 60 and 1000 stand for is good (even required),
but giving them their own constants...

The radix parameter

You call the function parseInt a few times in your code. Like here:

var topbarFlagCount = parseInt(topbarFlag);

and here:

reviewCount = parseInt(reviewItems.html());

Although it is not commonly known, the parseInt function actually takes a second parameter called the radix. This parameter is used to specify which numerical base the first parameter is in.

Most commonly, this number is 10 (base 10, or decimal). You can read more about this parameter here.

So, for example, you might write this as one of your lines:

var topbarFlagCount = parseInt(topbarFlag, 10);

Going back to your old post

Quite a while ago, you posted another UserScript question.

In an answer to that question, the answered mentioned that, to help compatibility with FireFox, you should encase the beginning part of your UserScript like this:

/** @preserve
// ==UserScript==
.....
// ==/UserScript==
*/

There are a number of UserScript conventions you are failing to adhere
to.


First up, for Firefox, you need to wrap the header section in to a
"preserved" comment block:


...


The preserve is required to keep the comment block after the
javascript is processed. This allows the 'compiled' version to keep
the references, and for FireFox to know what it is about still.


Other browsers may not have the same requirements.

Compressed lines

This line here:

setTimeout(function(){ window.location.reload(); }, DELAY);

Took me a few times to read before I could match up the {}s and the ()s. I think this would be much more readable if you expanded it, like this:

setTimeout(function() {
    window.location.reload(); 
}, DELAY);

Wrap your code

This is just a small thing that I wanted to point out.

You should encase your code in this:

(function (window, undefined) {
    (function ($) {
        [code]
    })(window.jQuery);
})(Function("return this")());

The reason for this is because the names window, undefined and $ are all valid variable names that could easily be used in any variable.

Only do this if you aren't writing anything important.

For example, open up your console and type:

var $ = 3;
console.log($);

You will get the output


3

As for window and undefined, the same will not happen for them: if you were to try:

var window = 5;
console.log(window);

The actual WOM object will be outputted. This is because for these variables, overriding them will only change their values locally; they will not be changed on a global level.

Read more in this chat conversation.

Basically, by wrapping your code in these anonymous functions, you are ensuring that no matter what other scripts do with those variable names, those variables will always be the way you want them.

And, the reason why the function for setting $ is not combined with the first anonymous function is because the one for setting $ needs to use the window object, which the first anonymous function ensures is valid.

Some more reasoning why there is an anonymous function for $ here.


Revert the $ alias and then create and execute a function to provide
the $ as a jQuery alias inside the function's scope. Inside the
function the original $ object is not available. This works well for
most plugins that don't rely on any other library.

EDIT:

The reason why you use the inner anonymous function is because of the function jQuery.noConflict.

If any code earlier in the document were to call this function, the alias $ would be stripped away from jQuery. This function restores the alias to jQuery, just in case this function was called.

Read more in this chat conversation.

Thanks to Ismael Miguel.

Split up your code

In your inner if statement of your larger one, you have two sections: one for getting all the flags, and one for getting all that is in the review queue.

To take some off the load of your main code, you should split these into respective functions, probably called getFlags and getReviewItems.

"use strict";

Your code should have "use strict";`.

StackExchange Code Review Q#97268, answer score: 15