Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General: Display recently accessed courses on top #7827

Merged
merged 6 commits into from
Dec 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ module.exports = {
// TODO: in the future, the following values should increase to at least 90%
statements: 86.7,
branches: 73.7,
functions: 80.8,
functions: 80.7,
lines: 86.7,
},
},
Expand Down
32 changes: 32 additions & 0 deletions src/main/webapp/app/course/course-access-storage.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Injectable } from '@angular/core';
import { LocalStorageService } from 'ngx-webstorage';

@Injectable({
providedIn: 'root',
})
export class CourseAccessStorageService {
private static readonly STORAGE_KEY = 'artemis.courseAccess';

constructor(private localStorage: LocalStorageService) {}

onCourseAccessed(courseId: number): void {
const courseAccessMap: { [key: number]: number } = this.localStorage.retrieve(CourseAccessStorageService.STORAGE_KEY) || {};

courseAccessMap[courseId] = Date.now();

if (Object.keys(courseAccessMap).length > 3) {
const oldestEntry = Object.entries(courseAccessMap).reduce((prev, curr) => (prev[1] < curr[1] ? prev : curr));
delete courseAccessMap[oldestEntry[0]];
}

this.localStorage.store(CourseAccessStorageService.STORAGE_KEY, courseAccessMap);
}
Comment on lines +12 to +23
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method onCourseAccessed correctly updates the timestamp for the accessed course. However, the logic to limit the stored courses to 3 is not scalable if the requirements change to store more or fewer courses. Consider making the limit a configurable property of the service.

- if (Object.keys(courseAccessMap).length > 3) {
+ const MAX_STORED_COURSES = 3; // This can be moved to a configurable part of the service
+ if (Object.keys(courseAccessMap).length > MAX_STORED_COURSES) {

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
onCourseAccessed(courseId: number): void {
const courseAccessMap: { [key: number]: number } = this.localStorage.retrieve(CourseAccessStorageService.STORAGE_KEY) || {};
courseAccessMap[courseId] = Date.now();
if (Object.keys(courseAccessMap).length > 3) {
const oldestEntry = Object.entries(courseAccessMap).reduce((prev, curr) => (prev[1] < curr[1] ? prev : curr));
delete courseAccessMap[oldestEntry[0]];
}
this.localStorage.store(CourseAccessStorageService.STORAGE_KEY, courseAccessMap);
}
onCourseAccessed(courseId: number): void {
const MAX_STORED_COURSES = 3; // This can be moved to a configurable part of the service
const courseAccessMap: { [key: number]: number } = this.localStorage.retrieve(CourseAccessStorageService.STORAGE_KEY) || {};
courseAccessMap[courseId] = Date.now();
if (Object.keys(courseAccessMap).length > MAX_STORED_COURSES) {
const oldestEntry = Object.entries(courseAccessMap).reduce((prev, curr) => (prev[1] < curr[1] ? prev : curr));
delete courseAccessMap[oldestEntry[0]];
}
this.localStorage.store(CourseAccessStorageService.STORAGE_KEY, courseAccessMap);
}


getLastAccessedCourses(): number[] {
const courseAccessMap: { [key: number]: number } = this.localStorage.retrieve(CourseAccessStorageService.STORAGE_KEY) || {};

return Object.entries(courseAccessMap)
.sort((a, b) => b[1] - a[1])
.map((entry) => Number(entry[0]));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { CourseAdminService } from 'app/course/manage/course-admin.service';
import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
import { ProfileService } from 'app/shared/layouts/profiles/profile.service';
import { PROFILE_LOCALCI } from 'app/app.constants';
import { CourseAccessStorageService } from 'app/course/course-access-storage.service';

@Component({
selector: 'jhi-course-management-tab-bar',
Expand Down Expand Up @@ -85,6 +86,7 @@ export class CourseManagementTabBarComponent implements OnInit, OnDestroy {
private router: Router,
private modalService: NgbModal,
private profileService: ProfileService,
private courseAccessStorageService: CourseAccessStorageService,
) {}

/**
Expand All @@ -108,6 +110,9 @@ export class CourseManagementTabBarComponent implements OnInit, OnDestroy {
this.localCIActive = profileInfo?.activeProfiles.includes(PROFILE_LOCALCI);
}
});

// Notify the course access storage service that the course has been accessed
this.courseAccessStorageService.onCourseAccessed(courseId);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ <h4 class="fw-medium" id="course-page-heading" jhiTranslate="artemisApp.course.h
<div class="course-table-container mb-3">
<div class="control-label my-3" (click)="semesterCollapsed[semester] = !semesterCollapsed[semester]">
<fa-icon size="2x" class="pe-3" [icon]="semesterCollapsed[semester] ? faAngleDown : faAngleUp"></fa-icon>
@if (semester !== '' && semester !== 'test') {
@if (semester !== '' && semester !== 'test' && semester !== 'recent') {
<span style="font-size: 24px"> {{ 'artemisApp.course.semester' | artemisTranslate }}: {{ semester }} </span>
}
@if (semester === '') {
Expand All @@ -41,6 +41,11 @@ <h4 class="fw-medium" id="course-page-heading" jhiTranslate="artemisApp.course.h
{{ 'artemisApp.course.testCourse.plural' | artemisTranslate }}
</span>
}
@if (semester === 'recent') {
<span style="font-size: 24px">
{{ 'artemisApp.course.recentlyAccessed' | artemisTranslate }}
</span>
}
</div>
@if (!semesterCollapsed[semester]) {
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { CourseManagementOverviewStatisticsDto } from 'app/course/manage/overvie
import { EventManager } from 'app/core/util/event-manager.service';
import { faAngleDown, faAngleUp, faPlus } from '@fortawesome/free-solid-svg-icons';
import { DocumentationType } from 'app/shared/components/documentation-button/documentation-button.component';
import { CourseAccessStorageService } from 'app/course/course-access-storage.service';

@Component({
selector: 'jhi-course',
Expand Down Expand Up @@ -46,6 +47,7 @@ export class CourseManagementComponent implements OnInit, OnDestroy, AfterViewIn
private alertService: AlertService,
private eventManager: EventManager,
private guidedTourService: GuidedTourService,
private courseAccessStorageService: CourseAccessStorageService,
) {}

/**
Expand Down Expand Up @@ -136,26 +138,40 @@ export class CourseManagementComponent implements OnInit, OnDestroy, AfterViewIn
/**
* Sorts the courses into the coursesBySemester map.
* Fills the semesterCollapsed map depending on if the semester should be expanded by default.
* The first semester is always expanded. The test course group is also expanded.
* The first semester group, the test courses and the recently accessed courses are expanded by default.
*/
private sortCoursesIntoSemesters(): void {
this.semesterCollapsed = {};
this.coursesBySemester = {};

// Get last accessed courses
const lastAccessedCourseIds = this.courseAccessStorageService.getLastAccessedCourses();
const recentlyAccessedCourses = this.courses.filter((course) => lastAccessedCourseIds.includes(course.id!));

let firstExpanded = false;
for (const semester of this.courseSemesters) {
this.semesterCollapsed[semester] = firstExpanded;
firstExpanded = true;
this.coursesBySemester[semester] = this.courses.filter((course) => !course.testCourse && (course.semester ?? '') === semester);
this.coursesBySemester[semester] = this.courses.filter(
(course) => !course.testCourse && !lastAccessedCourseIds.includes(course.id!) && (course.semester ?? '') === semester,
);
}

// Add a new category "recent"
this.courseSemesters.unshift('recent');
this.semesterCollapsed['recent'] = false;
this.coursesBySemester['recent'] = recentlyAccessedCourses;

// Add an extra category for test courses
const testCourses = this.courses.filter((course) => course.testCourse);
const testCourses = this.courses.filter((course) => course.testCourse && !lastAccessedCourseIds.includes(course.id!));
if (testCourses.length > 0) {
this.courseSemesters[this.courseSemesters.length] = 'test';
this.semesterCollapsed['test'] = false;
this.coursesBySemester['test'] = testCourses;
}

// Remove all semesters that have no courses
this.courseSemesters = this.courseSemesters.filter((semester) => this.coursesBySemester[semester].length > 0);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/main/webapp/app/overview/course-overview.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { CourseExerciseService } from 'app/exercises/shared/course-exercises/cou
import { BarControlConfiguration, BarControlConfigurationProvider } from 'app/shared/tab-bar/tab-bar';
import { FeatureToggle } from 'app/shared/feature-toggle/feature-toggle.service';
import { CourseStorageService } from 'app/course/manage/course-storage.service';
import { CourseAccessStorageService } from 'app/course/course-access-storage.service';

@Component({
selector: 'jhi-course-overview',
Expand Down Expand Up @@ -110,6 +111,7 @@ export class CourseOverviewComponent implements OnInit, OnDestroy, AfterViewInit
private changeDetectorRef: ChangeDetectorRef,
private metisConversationService: MetisConversationService,
private router: Router,
private courseAccessStorageService: CourseAccessStorageService,
) {}

async ngOnInit() {
Expand All @@ -119,6 +121,9 @@ export class CourseOverviewComponent implements OnInit, OnDestroy, AfterViewInit

this.course = this.courseStorageService.getCourse(this.courseId);

// Notify the course access storage service that the course has been accessed
this.courseAccessStorageService.onCourseAccessed(this.courseId);

await firstValueFrom(this.loadCourse());
await this.initAfterCourseLoad();
}
Expand Down
18 changes: 17 additions & 1 deletion src/main/webapp/app/overview/courses.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,24 @@ <h3 class="fw-medium" jhiTranslate="artemisApp.studentDashboard.title">Your curr
<a class="btn btn-primary" [routerLink]="['/courses/enroll']">{{ 'artemisApp.studentDashboard.enroll.title' | artemisTranslate }}</a>
</div>
</div>
@if (recentlyAccessedCourses.length > 0) {
<div class="row">
<h4 class="col mb-3 fw-medium" jhiTranslate="artemisApp.studentDashboard.recentlyAccessed">Recently Accessed Courses</h4>
</div>
<div class="row">
@for (course of recentlyAccessedCourses; track course) {
<jhi-overview-course-card class="col-12 col-lg-6 col-xl-4 pe-2 ps-2 mb-3" [course]="course" [hasGuidedTour]="course === courseForGuidedTour">
</jhi-overview-course-card>
}
</div>
@if (regularCourses.length > 0) {
<div class="row">
<h4 class="col mb-3 fw-medium" jhiTranslate="artemisApp.studentDashboard.otherCourses">Other Courses</h4>
</div>
}
}
Comment on lines +52 to +67
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditional rendering for 'Recently Accessed Courses' and 'Other Courses' is implemented correctly using Angular's structural directives. Ensure that the trackBy function is implemented for the ngFor directive to improve performance by minimizing DOM manipulations.

- @for (course of recentlyAccessedCourses; track course) {
+ @for (course of recentlyAccessedCourses; trackBy: trackCourse) {

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
@if (recentlyAccessedCourses.length > 0) {
<div class="row">
<h4 class="col mb-3 fw-medium" jhiTranslate="artemisApp.studentDashboard.recentlyAccessed">Recently Accessed Courses</h4>
</div>
<div class="row">
@for (course of recentlyAccessedCourses; track course) {
<jhi-overview-course-card class="col-12 col-lg-6 col-xl-4 pe-2 ps-2 mb-3" [course]="course" [hasGuidedTour]="course === courseForGuidedTour">
</jhi-overview-course-card>
}
</div>
@if (regularCourses.length > 0) {
<div class="row">
<h4 class="col mb-3 fw-medium" jhiTranslate="artemisApp.studentDashboard.otherCourses">Other Courses</h4>
</div>
}
}
@if (recentlyAccessedCourses.length > 0) {
<div class="row">
<h4 class="col mb-3 fw-medium" jhiTranslate="artemisApp.studentDashboard.recentlyAccessed">Recently Accessed Courses</h4>
</div>
<div class="row">
@for (course of recentlyAccessedCourses; trackBy: trackCourse) {
<jhi-overview-course-card class="col-12 col-lg-6 col-xl-4 pe-2 ps-2 mb-3" [course]="course" [hasGuidedTour]="course === courseForGuidedTour">
</jhi-overview-course-card>
}
</div>
@if (regularCourses.length > 0) {
<div class="row">
<h4 class="col mb-3 fw-medium" jhiTranslate="artemisApp.studentDashboard.otherCourses">Other Courses</h4>
</div>
}
}

<div class="row">
@for (course of courses; track course) {
@for (course of regularCourses; track course) {
<jhi-overview-course-card class="col-12 col-lg-6 col-xl-4 pe-2 ps-2 mb-3" [course]="course" [hasGuidedTour]="course === courseForGuidedTour"> </jhi-overview-course-card>
}
</div>
25 changes: 23 additions & 2 deletions src/main/webapp/app/overview/courses.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,23 @@ import dayjs from 'dayjs/esm';
import { Exam } from 'app/entities/exam.model';
import { Router } from '@angular/router';
import { faPenAlt } from '@fortawesome/free-solid-svg-icons';
import { CourseAccessStorageService } from 'app/course/course-access-storage.service';
import { CourseForDashboardDTO } from 'app/course/manage/course-for-dashboard-dto';

@Component({
selector: 'jhi-overview',
templateUrl: './courses.component.html',
styleUrls: ['./courses.component.scss'],
})
export class CoursesComponent implements OnInit, OnChanges, OnDestroy {
public courses: Course[];
courses: Course[];
public nextRelevantCourse?: Course;
nextRelevantCourseForExam?: Course;
nextRelevantExams?: Exam[];

public recentlyAccessedCourses: Course[] = [];
public regularCourses: Course[] = [];

courseForGuidedTour?: Course;
quizExercisesChannels: string[] = [];

Expand All @@ -41,6 +46,7 @@ export class CoursesComponent implements OnInit, OnChanges, OnDestroy {
private teamService: TeamService,
private jhiWebsocketService: JhiWebsocketService,
private router: Router,
private courseAccessStorageService: CourseAccessStorageService,
) {}

async ngOnInit() {
Expand All @@ -66,14 +72,15 @@ export class CoursesComponent implements OnInit, OnChanges, OnDestroy {
next: (res: HttpResponse<CoursesForDashboardDTO>) => {
if (res.body) {
const courses: Course[] = [];
res.body.courses.forEach((courseDto) => {
res.body.courses.forEach((courseDto: CourseForDashboardDTO) => {
courses.push(courseDto.course);
});
this.courses = courses.sort((a, b) => (a.title ?? '').localeCompare(b.title ?? ''));
this.courseForGuidedTour = this.guidedTourService.enableTourForCourseOverview(this.courses, courseOverviewTour, true);

this.nextRelevantExams = res.body.activeExams ?? [];
this.nextRelevantExercise = this.findNextRelevantExercise();
this.sortCoursesInRecentlyAccessedAndRegularCourses();
}
},
});
Expand Down Expand Up @@ -113,6 +120,20 @@ export class CoursesComponent implements OnInit, OnChanges, OnDestroy {
}
}

/**
* Sorts the courses into recently accessed and regular courses.
* If there are less than 5 courses, all courses are displayed in the regular courses section.
*/
sortCoursesInRecentlyAccessedAndRegularCourses() {
if (this.courses.length <= 5) {
this.regularCourses = this.courses;
} else {
const lastAccessedCourseIds = this.courseAccessStorageService.getLastAccessedCourses();
this.recentlyAccessedCourses = this.courses.filter((course) => lastAccessedCourseIds.includes(course.id!));
this.regularCourses = this.courses.filter((course) => !lastAccessedCourseIds.includes(course.id!));
}
}
Comment on lines +123 to +135
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method sortCoursesInRecentlyAccessedAndRegularCourses is implemented correctly. However, consider adding error handling for cases where course.id might be undefined to prevent runtime errors.

- this.recentlyAccessedCourses = this.courses.filter((course) => lastAccessedCourseIds.includes(course.id!));
+ this.recentlyAccessedCourses = this.courses.filter((course) => course.id && lastAccessedCourseIds.includes(course.id));

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
/**
* Sorts the courses into recently accessed and regular courses.
* If there are less than 5 courses, all courses are displayed in the regular courses section.
*/
sortCoursesInRecentlyAccessedAndRegularCourses() {
if (this.courses.length <= 5) {
this.regularCourses = this.courses;
} else {
const lastAccessedCourseIds = this.courseAccessStorageService.getLastAccessedCourses();
this.recentlyAccessedCourses = this.courses.filter((course) => lastAccessedCourseIds.includes(course.id!));
this.regularCourses = this.courses.filter((course) => !lastAccessedCourseIds.includes(course.id!));
}
}
/**
* Sorts the courses into recently accessed and regular courses.
* If there are less than 5 courses, all courses are displayed in the regular courses section.
*/
sortCoursesInRecentlyAccessedAndRegularCourses() {
if (this.courses.length <= 5) {
this.regularCourses = this.courses;
} else {
const lastAccessedCourseIds = this.courseAccessStorageService.getLastAccessedCourses();
this.recentlyAccessedCourses = this.courses.filter((course) => course.id && lastAccessedCourseIds.includes(course.id));
this.regularCourses = this.courses.filter((course) => !lastAccessedCourseIds.includes(course.id!));
}
}


/**
* Sets the course for the next upcoming exam and returns the next upcoming exam or undefined
*/
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/de/course.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
"startDate": "Start",
"endDate": "Ende",
"semester": "Semester",
"recentlyAccessed": "Zuletzt verwendet",
"maxPoints": {
"title": "Maximale Punktzahl",
"info": "Dieser Wert wird für die Berechnung von Beispielen (z.B. im Notenschlüssel) verwendet und hat keinen Einfluss auf die Noten der Studierenden. Die Noten werden mit Hilfe der im Kurs erreichbaren Punkte berechnet."
Expand Down
2 changes: 2 additions & 0 deletions src/main/webapp/i18n/de/student-dashboard.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"artemisApp": {
"studentDashboard": {
"title": "Deine aktuellen Kurse",
"recentlyAccessed": "Zuletzt besuchte Kurse",
"otherCourses": "Andere Kurse",
"exerciseTitle": "Aktuelle aktive Übung von \"{{ course }}\"",
"exerciseTitleWithoutDueDate": "Aktuelle Übung von \"{{ course }}\":",
"examTitle": "Aktuelle Prüfung von \"{{ course }}\":",
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/en/course.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
"startDate": "Start",
"endDate": "End",
"semester": "Semester",
"recentlyAccessed": "Recently accessed",
"maxPoints": {
"title": "Maximum number of points for course",
"info": "This value is used for example calculations (e.g. in the grading key) and does not influence the students' grades. The grades are calculated based on the points achievable in the course."
Expand Down
2 changes: 2 additions & 0 deletions src/main/webapp/i18n/en/student-dashboard.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"artemisApp": {
"studentDashboard": {
"title": "Your current courses",
"recentlyAccessed": "Recently accessed courses",
"otherCourses": "Other courses",
"exerciseTitle": "Current active exercise in \"{{ course }}\"",
"exerciseTitleWithoutDueDate": "Current exercise in \"{{ course }}\":",
"examTitle": "Current exam in \"{{ course }}\":",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { HasAnyAuthorityDirective } from 'app/shared/auth/has-any-authority.dire
import { FeatureToggleLinkDirective } from 'app/shared/feature-toggle/feature-toggle-link.directive';
import { FeatureToggleHideDirective } from 'app/shared/feature-toggle/feature-toggle-hide.directive';
import { MockRouter } from '../../helpers/mocks/mock-router';
import { CourseAccessStorageService } from 'app/course/course-access-storage.service';

describe('Course Management Tab Bar Component', () => {
let component: CourseManagementTabBarComponent;
Expand All @@ -25,6 +26,7 @@ describe('Course Management Tab Bar Component', () => {
let courseManagementService: CourseManagementService;
let courseAdminService: CourseAdminService;
let eventManager: EventManager;
let courseAccessStorageService: CourseAccessStorageService;

const router = new MockRouter();
router.setUrl('');
Expand Down Expand Up @@ -60,6 +62,7 @@ describe('Course Management Tab Bar Component', () => {
},
{ provide: Router, useValue: router },
MockProvider(CourseManagementService),
MockProvider(CourseAccessStorageService),
],
})
.compileComponents()
Expand All @@ -69,6 +72,7 @@ describe('Course Management Tab Bar Component', () => {
courseManagementService = TestBed.inject(CourseManagementService);
courseAdminService = TestBed.inject(CourseAdminService);
eventManager = TestBed.inject(EventManager);
courseAccessStorageService = TestBed.inject(CourseAccessStorageService);
});
});

Expand All @@ -81,11 +85,14 @@ describe('Course Management Tab Bar Component', () => {
jest.restoreAllMocks();
});

it('should register changes in course on init', () => {
it('should register changes in course and notify courseAccessStorageService on init', () => {
const spy = jest.spyOn(courseAccessStorageService, 'onCourseAccessed');

componentFixture.detectChanges();
component.ngOnInit();

expect(component.course).toEqual(course);
expect(spy).toHaveBeenCalledWith(course.id);
});

it('should destroy event subscriber onDestroy', () => {
Expand Down
Loading
Loading