Add scoping tabs to activity list pages to improve usability

app/assets/javascripts/components/filter_tabs.ts

@@ -0,0 +1,55 @@
+import { html, TemplateResult } from "lit";
+import { customElement, property } from "lit/decorators.js";
+import { FilterCollectionElement, Label } from "components/filter_collection_element";
+import { watchMixin } from "components/watch_mixin";
+
+/**
+ * This component inherits from FilterCollectionElement.
+ * It represent a list of tabs, where each tab shows a filtered set of the search results
+ *
+ * @element d-filter-tabs
+ *
+ * @prop {{id: string, name: string, title: string}[]} labels - all labels that could potentially be selected
+ */
+@customElement("d-filter-tabs")
+export class FilterTabs extends watchMixin(FilterCollectionElement) {
+    @property()
+    multi = false;
+    @property()
+    param = "tab";
+    @property()
+    paramVal = (label: Label): string => label.id.toString();
+    @property({ type: Array })
+    labels: {id: string, name: string, title: string}[];
+
+    processClick(e: Event, label: Label): void {
+        if (!this.isSelected(label)) {
+            this.select(label);
+        }
+        e.preventDefault();
+    }
+
+    watch = {
+        labels: () => {
+            if (this.getSelectedLabels().length == 0) {
+                this.select(this.labels[0]);
+            }
+        }
+    };
+
+    render(): TemplateResult {
+        return html`
+            <div class="card-tab">
+                <ul class="nav nav-tabs" role="tablist">
+                    ${this.labels.map(label => html`
+                        <li role="presentation" data-bs-toggle="tooltip" data-bs-title="${label.title ? label.title : ""}" data-bs-trigger="hover">
+                            <a href="#" @click=${e => this.processClick(e, label)} class="${this.isSelected(label) ? "active" : ""}">
+                                ${label.name}
+                            </a>
+                        </li>
+                    `)}
+                </ul>
+            </div>
+        `;
+    }
+}

app/assets/javascripts/course.ts

@@ -333,45 +333,11 @@ function initCourseNew(): void {
     init();
 }
 
-function initCoursesListing(firstTab: string): void {
-    initCourseTabs(firstTab);
-
-    function initCourseTabs(firstTab: string): void {
-        document.querySelectorAll<HTMLElement>("#course-tabs li a").forEach(tab => {
-            tab.addEventListener("click", event => {
-                event.preventDefault(); // used to prevent popstate event from firing
-                selectTab(tab);
-            });
-        });
-
-        // If the url hash is a valid tab, use that, otherwise use the given tab
-        const hash = searchQuery.queryParams.params.get("tab");
-        const tab = document.querySelector<HTMLElement>(`a[data-tab='${hash}']`) ??
-            document.querySelector(`a[data-tab='${firstTab}']`);
-        selectTab(tab);
-    }
-
-    function selectTab(tab: HTMLElement): void {
-        // If the current tab is already loaded or if it's blank, do nothing
-        if (!tab || tab.classList.contains("active")) return;
-
-        const state = tab.getAttribute("data-tab");
-        loadCourses(state);
-        document.querySelector("#course-tabs a.active")?.classList?.remove("active");
-        tab.classList.add("active");
-    }
-
-    function loadCourses(tab: string): void {
-        searchQuery.queryParams.updateParam("tab", tab);
-    }
-}
-
 export {
     initSeriesReorder,
     initCourseForm,
     initCourseNew,
     initCourseShow,
     initCourseMembers,
-    initCoursesListing,
     loadUsers,
 };

app/assets/javascripts/search.ts

@@ -80,14 +80,6 @@ export class SearchQuery {
         const url = new URL(_url.replace(/%5B%5D/g, "[]"), window.location.origin);
         this.baseUrl = url.href;
 
-        // Reset old params
-        for (const key of this.arrayQueryParams.params.keys()) {
-            this.arrayQueryParams.updateParam(key, undefined);
-        }
-        for (const key of this.queryParams.params.keys()) {
-            this.queryParams.updateParam(key, undefined);
-        }
-
         // initialise present parameters
         this.initialiseParams(url.searchParams);
     }
@@ -117,6 +109,7 @@ export class SearchQuery {
 
         window.onpopstate = e => {
             if (this.updateAddressBar && e.state === "set_by_search") {
+                this.resetAllQueryParams();
                 this.setBaseUrl();
             }
         };
@@ -169,7 +162,8 @@ export class SearchQuery {
     paramChange(key: string): void {
         this.changedParams.push(key);
         this.paramChangeDelayer(() => {
-            if (this.queryParams.params.get("page") !== "1" && this.changedParams.every(k => k !== "page")) {
+            if (this.queryParams.params.get("page") !== undefined && this.queryParams.params.get("page") !== "1" && this.changedParams.every(k => k !== "page")) {
+                // if we were not on the first page and we changed something else than the page, we should go back to the first page
                 this.changedParams = [];
                 this.queryParams.updateParam("page", "1");
                 return;
@@ -217,15 +211,6 @@ export class SearchQuery {
             const searchParamsStringFromStorage = localStorage.getItem(this.localStorageKey);
             if (searchParamsStringFromStorage) {
                 const searchParamsFromStorage = new URLSearchParams(searchParamsStringFromStorage);
-                // don't overwrite currently set params with params from the localStorage
-                searchParamsFromStorage.forEach((_value: string, key:string) => {
-                    if (this.queryParams.params.get(key) !== undefined ||
-                        (this.isArrayQueryParamsKey(key) &&
-                            this.arrayQueryParams.params.get(this.extractArrayQueryParamsKey(key)) !== undefined)) {
-                        searchParamsFromStorage.delete(key);
-                    }
-                });
-
                 this.initialiseParams(searchParamsFromStorage);
             }
         }

app/controllers/activities_controller.rb

@@ -23,6 +23,10 @@ class ActivitiesController < ApplicationController
   has_scope :in_repository, as: 'repository_id'
   has_scope :by_description_languages, as: 'description_languages', type: :array
   has_scope :by_judge, as: 'judge_id'
+  has_scope :repository_scope, as: 'tab' do |controller, scope, value|
+    course = Series.find(controller.params[:id]).course if controller.params[:id]
+    scope.repository_scope(scope: value, user: controller.current_user, course: course)
+  end
 
   content_security_policy only: %i[show] do |policy|
     policy.frame_src -> { ["'self'", sandbox_url] }
@@ -63,13 +67,24 @@ def index
 
     unless @activities.empty?
       @activities = apply_scopes(@activities)
-      @activities = @activities.order("name_#{I18n.locale}").order(path: :asc).paginate(page: parse_pagination_param(params[:page]))
+      @activities = @activities.order(Arel.sql("name_#{I18n.locale} IS NULL, name_#{I18n.locale}")).order(path: :asc).paginate(page: parse_pagination_param(params[:page]))
     end
     @labels = policy_scope(Label.all)
     @programming_languages = policy_scope(ProgrammingLanguage.all)
     @repositories = policy_scope(Repository.all)
     @judges = policy_scope(Judge.all)
     @title = I18n.t('activities.index.title')
+
+    @tabs = []
+    @tabs << { id: :mine, name: I18n.t('activities.index.tabs.mine'), title: I18n.t('activities.index.tabs.mine_title') }
+    if current_user.institution.present?
+      @tabs << { id: :my_institution,
+                 name: I18n.t('activities.index.tabs.my_institution', institution: current_user.institution.short_name || current_user.institution.name),
+                 title: I18n.t('activities.index.tabs.my_institution_title') }
+    end
+    @tabs << { id: :featured, name: I18n.t('activities.index.tabs.featured'), title: I18n.t('activities.index.tabs.featured_title') }
+    @tabs << { id: :all, name: I18n.t('activities.index.tabs.all'), title: I18n.t('activities.index.tabs.all_title') }
+    @tabs = @tabs.filter { |t| Activity.repository_scope(scope: t[:id], user: current_user).any? }
   end
 
   def available

app/controllers/series_controller.rb

@@ -91,6 +91,17 @@ def edit
     @labels = policy_scope(Label.all)
     @programming_languages = policy_scope(ProgrammingLanguage.all)
     @repositories = policy_scope(Repository.all)
+
+    @tabs = []
+    @tabs << { id: :mine, name: I18n.t('activities.index.tabs.mine'), title: I18n.t('activities.index.tabs.mine_course_title') }
+    if current_user.institution.present?
+      @tabs << { id: :my_institution,
+                 name: I18n.t('activities.index.tabs.my_institution', institution: current_user.institution.short_name || current_user.institution.name),
+                 title: I18n.t('activities.index.tabs.my_institution_title') }
+    end
+    @tabs << { id: :featured, name: I18n.t('activities.index.tabs.featured'), title: I18n.t('activities.index.tabs.featured_title') }
+    @tabs << { id: :all, name: I18n.t('activities.index.tabs.all'), title: I18n.t('activities.index.tabs.all_title') }
+    @tabs = @tabs.filter { |t| Activity.repository_scope(scope: t[:id], user: current_user, course: @series.course).any? }
   end
 
   # POST /series

app/javascript/packs/course.js

@@ -4,7 +4,6 @@ import {
     initCourseForm,
     initCourseNew,
     initCourseShow,
-    initCoursesListing,
     loadUsers,
 } from "course.ts";
 
@@ -19,7 +18,6 @@ window.dodona.initCourseForm = initCourseForm;
 window.dodona.initCourseNew = initCourseNew;
 window.dodona.initCourseShow = initCourseShow;
 window.dodona.initCourseMembers = initCourseMembers;
-window.dodona.initCoursesListing = initCoursesListing;
 window.dodona.loadUsers = loadUsers;
 
 window.dodona.questionTable = RefreshingQuestionTable;

app/javascript/packs/search.js

@@ -6,3 +6,4 @@ import "components/search_field.ts";
 import "components/search_token.ts";
 import "components/filter_button.ts";
 import "components/dropdown_filter";
+import "components/filter_tabs";

app/models/activity.rb

@@ -86,6 +86,20 @@ class Activity < ApplicationRecord
     by_language
   }
 
+  scope :repository_scope, lambda { |options|
+    case options[:scope]&.to_sym
+    when :mine
+      where(repository: Repository.has_admin(options[:user]))
+        .or(where(repository: Repository.has_allowed_course(options[:course])))
+    when :my_institution
+      joins(:repository).merge(Repository.owned_by_institution(options[:user].institution))
+    when :featured
+      joins(:repository).merge(Repository.featured)
+    else
+      all
+    end
+  }
+
   def content_page?
     false
   end

app/models/repository.rb

@@ -10,6 +10,7 @@
 #  created_at   :datetime         not null
 #  updated_at   :datetime         not null
 #  clone_status :integer          default("queued"), not null
+#  featured     :boolean          default(FALSE)
 #
 require 'open3'
 require 'pathname'
@@ -53,6 +54,11 @@ class Repository < ApplicationRecord
   has_many :content_pages, dependent: :restrict_with_error
   has_many :exercises, dependent: :restrict_with_error
 
+  scope :has_allowed_course, ->(course) { joins(:course_repositories).where(course_repositories: { course_id: course&.id }) }
+  scope :has_admin, ->(user) { joins(:repository_admins).where(repository_admins: { user_id: user&.id }) }
+  scope :owned_by_institution, ->(institution) { where(id: RepositoryAdmin.joins(:user).where(users: { institution_id: institution&.id }).group(:repository_id).select(:repository_id)) }
+  scope :featured, -> { where(featured: true) }
+
   def full_path
     Pathname.new File.join(ACTIVITY_LOCATIONS, path)
   end

app/policies/repository_policy.rb

@@ -66,7 +66,9 @@ def reprocess?
   end
 
   def permitted_attributes
-    if user&.admin?
+    if user&.zeus?
+      %i[name remote judge_id featured]
+    elsif user&.admin?
       %i[name remote judge_id]
     else
       []

app/views/activities/index.html.erb

@@ -5,6 +5,7 @@
         <h2 class="card-title-text"><%= t ".title" %></h2>
       </div>
       <div class="card-supporting-text">
+        <d-filter-tabs labels="<%= @tabs.to_json %>"></d-filter-tabs>
         <%= render partial: 'layouts/searchbar', locals: {labels: @labels, programming_languages: @programming_languages, repositories: @repositories, activity_types: [ContentPage, Exercise], description_languages: ["en", "nl"], judges: @judges, no_dropdown_for: ["judges"]} %>
         <div id="activities-table-wrapper">
           <%= render partial: 'activities_table', locals: {activities: @activities} %>

app/views/courses/index.html.erb

@@ -17,26 +17,17 @@
     </div>
     <div class="card course-search">
       <div class="card-supporting-text">
-        <div class="card-tab">
-          <ul id="course-tabs" class="nav nav-tabs" data-baseurl="<%= courses_url %>" role="tablist">
-            <% if @show_institution_courses %>
-              <li role="presentation"><a href="#" data-tab="institution"><%= t '.institution_courses', institution: (current_user.institution&.short_name || current_user.institution&.name) %></a></li>
-            <% end %>
-            <li role="presentation"><a href="#" data-tab="featured"><%= t '.featured_courses' %></a></li>
-            <li role="presentation"><a href="#" data-tab="all"><%= t '.all_courses' %></a></li>
-            <% if @show_my_courses %>
-              <li role="presentation"><a href="#" data-tab="my"><%= t '.my_courses' %></a></li>
-            <% end %>
-          </ul>
-        </div>
-        <%= render partial: 'layouts/searchbar', locals: {institutions: Institution.all, eager: false, no_dropdown_for: ["institutions"], actions: [{search: {can_register: true}, type: 'can-register', text: "#{t(".only_show_can_register")}"}]  } %>
+        <%
+          labels = []
+          labels << { name: t('.institution_courses', institution: (current_user.institution&.short_name || current_user.institution&.name)), id: 'institution' } if @show_institution_courses
+          labels << { name: t('courses.index.featured_courses'), id: 'featured' }
+          labels << { name: t('courses.index.all_courses'), id: 'all' }
+          labels << { name: t('courses.index.my_courses'), id: 'my' } if @show_my_courses
+        %>
+        <d-filter-tabs labels="<%= labels.to_json %>"></d-filter-tabs>
+        <%= render partial: 'layouts/searchbar', locals: {institutions: Institution.all, eager: true, no_dropdown_for: ["institutions"], actions: [{search: {can_register: true}, type: 'can-register', text: "#{t(".only_show_can_register")}"}]  } %>
         <div id="courses-table-wrapper"></div>
       </div>
     </div>
   </div>
 </div>
-<script type="text/javascript">
-  dodona.ready.then(function () {
-    dodona.initCoursesListing('<%= @show_institution_courses ? "institution" : "featured" %>');
-  });
-</script>

app/views/repositories/_form.html.erb

@@ -41,4 +41,16 @@
     <div class="col-sm-6"><%= f.select :judge_id, Judge.all.collect {|j| [j.name, j.id]}, {}, {class: "form-select"} %></div>
     <span class="help-block offset-sm-3 col-sm-6"><%= t ".judge_help" %></span>
   </div>
+
+  <% if f.permission?(:featured) %>
+    <div class="field form-group row">
+      <%= f.label :featured, t(".featured.title"), class: 'col-sm-3 col-form-label' %>
+      <div class="col-sm-6 pt-2">
+        <div class="form-check">
+          <%= f.check_box :featured, class: 'form-check-input' %>
+          <%= f.label :featured, t(".featured.toggle-label"), class: 'form-check-label' %>
+        </div>
+      </div>
+    </div>
+  <% end %>
 <% end %>

app/views/series/edit.html.erb

@@ -84,6 +84,7 @@
         <div class="card-supporting-text card-border">
           <div class="col-12">
             <h4 id="add-activities"><%= t ".add_activities" %></h4>
+            <d-filter-tabs labels="<%= @tabs.to_json %>"></d-filter-tabs>
             <%= render partial: 'layouts/searchbar', locals: {baseUrl: available_activities_series_path(@series), eager: true, labels: @labels, programming_languages: @programming_languages, repositories: @repositories, activity_types: [ContentPage, Exercise], description_languages: ["en", "nl"], local_storage_key: 'seriesActivitySearch'} %>
             <div id="activities-table-wrapper">
             </div>

config/locales/views/activities/en.yml

@@ -55,6 +55,16 @@ en:
         exercise: Exercise
         exercise_language: "%{language} exercise"
         content: Reading activity
+      tabs:
+        my_institution: "%{institution} activities"
+        my_institution_title: "Activities owned by you or one of your colleagues"
+        mine_title: "Activities owned by you"
+        mine_course_title: "Activities owned by you or to which this course has been granted access"
+        featured_title: "Activities featured by the Dodona team"
+        all_title: "All activities to which you have access"
+        mine: "My activities"
+        featured: "Featured activities"
+        all: "All activities"
     show:
       code: Code
       handin: Hand in

config/locales/views/activities/nl.yml

@@ -55,6 +55,16 @@ nl:
         exercise: Oefening
         exercise_language: "%{language} oefening"
         content: Leesactiviteit
+      tabs:
+        my_institution: "%{institution} activiteiten"
+        my_institution_title: "Activiteiten van jou of een van jouw collega's"
+        mine_title: "Activiteiten waarvan je de eigenaar bent"
+        mine_course_title: "Activiteiten waarvan je de eigenaar bent of waar deze cursus toegang toe heeft"
+        featured_title: "Activiteiten die zijn uitgelicht door het Dodona-team"
+        all_title: "Alle activiteiten waar je toegang tot hebt"
+        mine: "Mijn activiteiten"
+        featured: "Uitgelichte activiteiten"
+        all: "Alle activiteiten"
     show:
       code: Code
       handin: Indienen

config/locales/views/repositories/en.yml

@@ -3,6 +3,9 @@ en:
     form:
       remote_help: "The SSH clone URL of the exercise repository. Dodona needs read and write permissions to use the repository."
       judge_help: "This judge will be used when the exercise doesn't specify one."
+      featured:
+        title: "Featured"
+        toggle-label: "Feature this repository"
       remote_cant_be_edited_html: The clone URL can't be edited after the repository was created. Contact <a href="mailto:dodona@ugent.be">dodona@ugent.be</a> if it needs to be changed.
     index:
       title: All exercise repositories