From 979f1e4b35b991af550c1a701adeb02fc67c553b Mon Sep 17 00:00:00 2001
From: Niko Strijbol <strijbol.niko@gmail.com>
Date: Thu, 24 Sep 2020 15:53:27 +0200
Subject: [PATCH 1/4] Show number of unanswered questions in title

---
 app/assets/javascripts/util.js        | 12 +++++++++++-
 app/controllers/courses_controller.rb |  2 +-
 app/javascript/packs/course.js        |  2 ++
 app/views/courses/questions.js.erb    |  1 +
 config/locales/views/courses/en.yml   |  1 +
 config/locales/views/courses/nl.yml   |  1 +
 6 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/util.js b/app/assets/javascripts/util.js
index 51ff030032..5967e45f77 100644
--- a/app/assets/javascripts/util.js
+++ b/app/assets/javascripts/util.js
@@ -209,6 +209,15 @@ function makeVisible(element) {
     element.style.visibility = "visible";
 }
 
+/**
+ * Set the title of the webpage.
+ *
+ * @param {string} title The new title.
+ */
+function setDocumentTitle(title) {
+    document.title = title;
+}
+
 export {
     delay,
     fetch,
@@ -224,5 +233,6 @@ export {
     initTooltips,
     initTokenClickables,
     makeInvisible,
-    makeVisible
+    makeVisible,
+    setDocumentTitle
 };
diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb
index 544a6e57d6..ed581f7f19 100644
--- a/app/controllers/courses_controller.rb
+++ b/app/controllers/courses_controller.rb
@@ -196,7 +196,6 @@ def scoresheet
   end
 
   def questions
-    @title = I18n.t('courses.questions.questions.title')
     @crumbs = [[@course.name, course_path(@course)], [I18n.t('courses.questions.questions.title'), '#']]
 
     @refresh = ActiveRecord::Type::Boolean.new.deserialize(params.fetch(:refresh, @course.enabled_questions?.to_s))
@@ -212,6 +211,7 @@ def questions
                        .order(updated_at: :desc)
                        .includes(:user, :last_updated_by, submission: [:exercise])
                        .paginate(page: parse_pagination_param(params[:answered_page]), per_page: 10)
+    @title = I18n.t('courses.questions.questions.page_title', count: @course.unanswered_questions.count)
   end
 
   def update_membership
diff --git a/app/javascript/packs/course.js b/app/javascript/packs/course.js
index 4859dda69a..6dfd8d7246 100644
--- a/app/javascript/packs/course.js
+++ b/app/javascript/packs/course.js
@@ -10,6 +10,7 @@ import {
 import {
     QuestionTable
 } from "question_table.ts";
+import { setDocumentTitle } from "util.js";
 
 window.dodona.initSeriesReorder = initSeriesReorder;
 window.dodona.initCourseForm = initCourseForm;
@@ -19,3 +20,4 @@ window.dodona.initCourseMembers = initCourseMembers;
 window.dodona.loadUsers = loadUsers;
 
 window.dodona.questionTable = QuestionTable;
+window.dodona.setDocumentTitle = setDocumentTitle;
diff --git a/app/views/courses/questions.js.erb b/app/views/courses/questions.js.erb
index f788fddf56..088beca1b5 100644
--- a/app/views/courses/questions.js.erb
+++ b/app/views/courses/questions.js.erb
@@ -6,3 +6,4 @@ document.querySelector(".question-table-answered").innerHTML = "<%= escape_javas
 <% else %>
   document.getElementById("questions-in-progress").classList.remove("hidden");
 <% end %>
+dodona.setDocumentTitle("Dodona - <%= @title %>");
diff --git a/config/locales/views/courses/en.yml b/config/locales/views/courses/en.yml
index ce1fa3711c..d5d6d8f942 100644
--- a/config/locales/views/courses/en.yml
+++ b/config/locales/views/courses/en.yml
@@ -173,6 +173,7 @@ en:
         open: "Unanswered questions"
         in_progress: "Questions in progress"
         closed: "Answered questions"
+        page_title: "Questions (%{count})"
         title: "Questions"
         auto_refresh: Automatically refresh questions
       ago: "%{when} ago"
diff --git a/config/locales/views/courses/nl.yml b/config/locales/views/courses/nl.yml
index 78bf2017b2..77b6c4ea9e 100644
--- a/config/locales/views/courses/nl.yml
+++ b/config/locales/views/courses/nl.yml
@@ -170,6 +170,7 @@ nl:
         in_progress: "Vragen in behandeling"
         closed: "Beantwoorde vragen"
         title: "Vragen"
+        page_title: "Vragen (%{count})"
         auto_refresh: Vragen automatisch vernieuwen
       ago: "%{when} geleden"
       last_edited_by:

From 0b0a898e0f0892edd5a903b839cfd56111f99a23 Mon Sep 17 00:00:00 2001
From: Niko Strijbol <strijbol.niko@gmail.com>
Date: Fri, 25 Sep 2020 17:41:27 +0200
Subject: [PATCH 2/4] Add favicon dot for questions

---
 app/assets/javascripts/favicon.ts             | 49 +++++++++++++++++++
 app/assets/javascripts/notification.ts        | 11 +++--
 app/controllers/application_controller.rb     |  3 ++
 app/controllers/courses_controller.rb         |  1 +
 app/javascript/packs/application.js           |  2 +
 app/views/courses/questions.js.erb            |  5 ++
 app/views/layouts/application.html.erb        |  6 ++-
 .../_notifications_table.html.erb             |  2 +-
 .../_small_notifications_table.html.erb       |  2 +-
 9 files changed, 72 insertions(+), 9 deletions(-)
 create mode 100644 app/assets/javascripts/favicon.ts

diff --git a/app/assets/javascripts/favicon.ts b/app/assets/javascripts/favicon.ts
new file mode 100644
index 0000000000..a09f519d6a
--- /dev/null
+++ b/app/assets/javascripts/favicon.ts
@@ -0,0 +1,49 @@
+/**
+ * Manager for the favicon.
+ */
+export class FaviconManager {
+    private readonly tags: Set<string> = new Set<string>();
+
+    public constructor(initial: string[]) {
+        initial.forEach(e => this.tags.add(e));
+    }
+
+    /**
+     * Request that you want to show a dot in the favicon. This will always
+     * result in a dot being shown.
+     *
+     * @param {string} tag The tag you want to add.
+     */
+    public requestDot(tag: string): void {
+        // Thread safe, since JS is single threaded in the browser.
+        this.tags.add(tag);
+        FaviconManager.showDot();
+    }
+
+    /**
+     * Indicate you no longer want to show a dot in the favicon. If you were the
+     * last one requesting the dot, the dot will be hidden.
+     *
+     * @param {string} tag The tag you want to release.
+     */
+    public releaseDot(tag: string): void {
+        this.tags.delete(tag);
+        if (this.tags.size === 0) {
+            FaviconManager.hideDot();
+        }
+    }
+
+    private static showDot(): void {
+        document.querySelector("link[rel=\"shortcut icon\"][href=\"/icon.png\"]")
+            ?.setAttribute("href", "/icon-not.png");
+        document.querySelector("link[rel=\"shortcut icon\"][href=\"/favicon.ico\"]")
+            ?.setAttribute("href", "/favicon-not.ico");
+    }
+
+    private static hideDot(): void {
+        document.querySelector("link[rel=\"shortcut icon\"][href=\"/icon-not.png\"]")
+            ?.setAttribute("href", "/icon.png");
+        document.querySelector("link[rel=\"shortcut icon\"][href=\"/favicon-not.ico\"]")
+            ?.setAttribute("href", "/favicon.ico");
+    }
+}
diff --git a/app/assets/javascripts/notification.ts b/app/assets/javascripts/notification.ts
index 7e484573e6..ba92049cb9 100644
--- a/app/assets/javascripts/notification.ts
+++ b/app/assets/javascripts/notification.ts
@@ -1,3 +1,4 @@
+import { FaviconManager } from "favicon";
 import { fetch } from "util.js";
 /**
  * Model for a notification in the navbar. It adds three listeners to the notification view:
@@ -15,13 +16,15 @@ export class Notification {
     private readonly element: Element;
     private readonly url: string;
     private readonly notifiableUrl: string;
+    private readonly faviconManager: FaviconManager;
     private read: boolean;
 
-    constructor(id: number, url: string, read: boolean, notifiableUrl: string, installClickHandler: boolean) {
+    constructor(id: number, url: string, read: boolean, notifiableUrl: string, installClickHandler: boolean, manager: FaviconManager) {
         this.element = document.querySelector(`.notification[data-id="${id}"]`);
         this.read = read;
         this.url = url;
         this.notifiableUrl = notifiableUrl;
+        this.faviconManager = manager;
 
         this.element.querySelector(".read-toggle-button").addEventListener("click", event => {
             this.toggleRead();
@@ -63,12 +66,10 @@ export class Notification {
         }
         if (document.querySelectorAll(".notification.unread").length === 0) {
             document.querySelector("#navbar-notifications .dropdown-toggle")?.classList?.remove("notification");
-            document.querySelector("link[rel=\"shortcut icon\"][href=\"/icon-not.png\"]")?.setAttribute("href", "/icon.png");
-            document.querySelector("link[rel=\"shortcut icon\"][href=\"/favicon-not.ico\"]")?.setAttribute("href", "/favicon.ico");
+            this.faviconManager.releaseDot("notifications");
         } else {
             document.querySelector("#navbar-notifications .dropdown-toggle")?.classList?.add("notification");
-            document.querySelector("link[rel=\"shortcut icon\"][href=\"/icon.png\"]")?.setAttribute("href", "/icon-not.png");
-            document.querySelector("link[rel=\"shortcut icon\"][href=\"/favicon.ico\"]")?.setAttribute("href", "/favicon-not.ico");
+            this.faviconManager.requestDot("notifications");
         }
     }
 
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index adc7dd3f83..1fb12510ed 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -186,5 +186,8 @@ def set_notifications
         true
       end
     end
+    # This variable counts for which services the dot in the favicon should be shown.
+    # On most pages this will be empty or contain :notifications
+    @dot_icon = @unread_notifications.any? ? %i[notifications] : []
   end
 end
diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb
index ed581f7f19..7386732827 100644
--- a/app/controllers/courses_controller.rb
+++ b/app/controllers/courses_controller.rb
@@ -212,6 +212,7 @@ def questions
                        .includes(:user, :last_updated_by, submission: [:exercise])
                        .paginate(page: parse_pagination_param(params[:answered_page]), per_page: 10)
     @title = I18n.t('courses.questions.questions.page_title', count: @course.unanswered_questions.count)
+    @dot_icon.push(:questions) if @unanswered.any?
   end
 
   def update_membership
diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js
index c643cffd7b..fc4279b220 100644
--- a/app/javascript/packs/application.js
+++ b/app/javascript/packs/application.js
@@ -25,6 +25,7 @@ import { Toast } from "toast";
 import { Notification } from "notification";
 import { checkTimeZone, checkIframe, initCSRF, initTooltips } from "util.js";
 import { initClipboard } from "copy";
+import { FaviconManager } from "favicon";
 
 // Initialize clipboard.js
 initClipboard();
@@ -42,6 +43,7 @@ $(initTooltips);
 
 // Use a global dodona object to prevent polluting the global na
 const dodona = window.dodona || {};
+dodona.dotManager = new FaviconManager(dodona.dotCount || []);
 dodona.checkTimeZone = checkTimeZone;
 dodona.Toast = Toast;
 dodona.Notification = Notification;
diff --git a/app/views/courses/questions.js.erb b/app/views/courses/questions.js.erb
index 088beca1b5..744e9432de 100644
--- a/app/views/courses/questions.js.erb
+++ b/app/views/courses/questions.js.erb
@@ -7,3 +7,8 @@ document.querySelector(".question-table-answered").innerHTML = "<%= escape_javas
   document.getElementById("questions-in-progress").classList.remove("hidden");
 <% end %>
 dodona.setDocumentTitle("Dodona - <%= @title %>");
+<% if @unanswered.any? %>
+  dodona.dotManager.requestDot("questions");
+<% else %>
+  dodona.dotManager.releaseDot("questions");
+<% end %>
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index b71900d44a..43025b5ae0 100644
--- a/app/views/layouts/application.html.erb
+++ b/app/views/layouts/application.html.erb
@@ -17,8 +17,8 @@
   <meta name="msapplication-TileColor" content="#2196F3"/>
   <meta name="msapplication-TileImage" content="/icon.png"/>
   <link rel="apple-touch-icon-precomposed" href="/icon.png"/>
-  <link rel="shortcut icon" sizes="192x192" href="<%= @unread_notifications&.any? ? "/icon-not.png" : "/icon.png" %>">
-  <link rel="shortcut icon" href="<%= @unread_notifications&.any? ? "/favicon-not.ico" : "/favicon.ico" %>">
+  <link rel="shortcut icon" sizes="192x192" href="<%= @dot_icon&.any? ? "/icon-not.png" : "/icon.png" %>">
+  <link rel="shortcut icon" href="<%= @dot_icon&.any? ? "/favicon-not.ico" : "/favicon.ico" %>">
   <link rel="manifest" href="/manifest.json">
   <title><%= "#{@title} - " if @title %>Dodona</title>
   <link href="https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,300;0,400;0,500;0,700;1,400&display=swap" rel="stylesheet">
@@ -29,6 +29,7 @@
     <script>
       window.dodona = window.dodona || {};
       window.dodona.darkMode = false;
+      window.dodona.dotCount = <%= @dot_icon.to_json.html_safe %>;
       const lightModeCSS = '<%= stylesheet_link_tag "application", media: "all" %>';
       const darkModeCSS = '<%= stylesheet_link_tag "application-dark", media: "all" %>';
 
@@ -63,6 +64,7 @@
     <script>
       window.dodona = window.dodona || {};
       window.dodona.darkMode = <%= session.include?(:dark) && session[:dark] %>;
+      window.dodona.dotCount = <%= @dot_icon&.to_json&.html_safe || "[]" %>;
     </script>
   <% end %>
 
diff --git a/app/views/notifications/_notifications_table.html.erb b/app/views/notifications/_notifications_table.html.erb
index 74fcd8a88c..1ea4c5cdbc 100644
--- a/app/views/notifications/_notifications_table.html.erb
+++ b/app/views/notifications/_notifications_table.html.erb
@@ -32,7 +32,7 @@
   <script>
     $(() => {
       <% notifications.each do |n| %>
-        new dodona.Notification(<%= n.id %>, "<%= notification_path(n, format: :json) %>", <%= n.read %>, "<%= notifiable_url(n) %>", false);
+        new dodona.Notification(<%= n.id %>, "<%= notification_path(n, format: :json) %>", <%= n.read %>, "<%= notifiable_url(n) %>", false, dodona.dotManager);
       <% end %>
     })
   </script>
diff --git a/app/views/notifications/_small_notifications_table.html.erb b/app/views/notifications/_small_notifications_table.html.erb
index bac6dcbbdc..32aa6835c7 100644
--- a/app/views/notifications/_small_notifications_table.html.erb
+++ b/app/views/notifications/_small_notifications_table.html.erb
@@ -30,7 +30,7 @@
   <script>
     $(() => {
       <% notifications.each do |n| %>
-        new dodona.Notification(<%= n.id %>, "<%= notification_path(n, format: :json) %>", <%= n.read %>, "<%= notifiable_url(n) %>", true);
+        new dodona.Notification(<%= n.id %>, "<%= notification_path(n, format: :json) %>", <%= n.read %>, "<%= notifiable_url(n) %>", true, dodona.dotManager);
       <% end %>
     })
   </script>

From 3a5e259a8688c46515850edd598a3db8a290e0af Mon Sep 17 00:00:00 2001
From: Niko Strijbol <strijbol.niko@gmail.com>
Date: Tue, 29 Sep 2020 19:02:30 +0200
Subject: [PATCH 3/4] Move count to front & only show if > 0

---
 app/controllers/courses_controller.rb | 7 ++++++-
 app/views/courses/questions.js.erb    | 2 +-
 config/locales/views/courses/en.yml   | 2 +-
 config/locales/views/courses/nl.yml   | 2 +-
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb
index 7386732827..2db0235905 100644
--- a/app/controllers/courses_controller.rb
+++ b/app/controllers/courses_controller.rb
@@ -211,7 +211,12 @@ def questions
                        .order(updated_at: :desc)
                        .includes(:user, :last_updated_by, submission: [:exercise])
                        .paginate(page: parse_pagination_param(params[:answered_page]), per_page: 10)
-    @title = I18n.t('courses.questions.questions.page_title', count: @course.unanswered_questions.count)
+    count = @course.unanswered_questions.count
+    @title = if count == 0
+               I18n.t('courses.questions.questions.title')
+             else
+               I18n.t('courses.questions.questions.page_title', count: count)
+             end
     @dot_icon.push(:questions) if @unanswered.any?
   end
 
diff --git a/app/views/courses/questions.js.erb b/app/views/courses/questions.js.erb
index 744e9432de..f3eceac5eb 100644
--- a/app/views/courses/questions.js.erb
+++ b/app/views/courses/questions.js.erb
@@ -6,7 +6,7 @@ document.querySelector(".question-table-answered").innerHTML = "<%= escape_javas
 <% else %>
   document.getElementById("questions-in-progress").classList.remove("hidden");
 <% end %>
-dodona.setDocumentTitle("Dodona - <%= @title %>");
+dodona.setDocumentTitle("<%= @title %> - Dodona");
 <% if @unanswered.any? %>
   dodona.dotManager.requestDot("questions");
 <% else %>
diff --git a/config/locales/views/courses/en.yml b/config/locales/views/courses/en.yml
index d5d6d8f942..771ee647d2 100644
--- a/config/locales/views/courses/en.yml
+++ b/config/locales/views/courses/en.yml
@@ -173,7 +173,7 @@ en:
         open: "Unanswered questions"
         in_progress: "Questions in progress"
         closed: "Answered questions"
-        page_title: "Questions (%{count})"
+        page_title: "(%{count}) Questions"
         title: "Questions"
         auto_refresh: Automatically refresh questions
       ago: "%{when} ago"
diff --git a/config/locales/views/courses/nl.yml b/config/locales/views/courses/nl.yml
index 77b6c4ea9e..01a0cdaeba 100644
--- a/config/locales/views/courses/nl.yml
+++ b/config/locales/views/courses/nl.yml
@@ -170,7 +170,7 @@ nl:
         in_progress: "Vragen in behandeling"
         closed: "Beantwoorde vragen"
         title: "Vragen"
-        page_title: "Vragen (%{count})"
+        page_title: "(%{count}) Vragen"
         auto_refresh: Vragen automatisch vernieuwen
       ago: "%{when} geleden"
       last_edited_by:

From 0b19633856f1de3ff929f8014326e1d380f946d1 Mon Sep 17 00:00:00 2001
From: Niko Strijbol <strijbol.niko@gmail.com>
Date: Thu, 1 Oct 2020 17:55:38 +0200
Subject: [PATCH 4/4] Add test for question page title

---
 test/controllers/courses_controller_test.rb | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/test/controllers/courses_controller_test.rb b/test/controllers/courses_controller_test.rb
index ec2dd03023..e85b277a6c 100644
--- a/test/controllers/courses_controller_test.rb
+++ b/test/controllers/courses_controller_test.rb
@@ -571,4 +571,17 @@ def with_users_signed_in(users)
       assert :ok, "#{who} should not be able to view questions"
     end
   end
+
+  test 'question page title is correct' do
+    sign_in @admins.first
+    get questions_course_path(@course)
+    assert_select 'title', /^([^0-9]*)$/
+
+    submission = create :submission, course: @course
+    create :question, question_state: :answered, submission: submission
+    create :question, question_state: :unanswered, submission: submission
+    create :question, question_state: :in_progress, submission: submission
+    get questions_course_path(@course)
+    assert_select 'title', /\(1\)/
+  end
 end