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

Adaptive learning: Extend learning path feature to recommend sequence of competencies #7091

Merged
merged 328 commits into from
Oct 9, 2023

Conversation

MaximilianAnzinger
Copy link
Collaborator

@MaximilianAnzinger MaximilianAnzinger commented Aug 21, 2023

⚠️ Stacked on #7077 please don't merge accidentally

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
  • I followed the coding and design guidelines.
  • I added multiple integration tests (Spring) related to the features (with a high test coverage).
  • I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
  • I documented the Java code using JavaDoc style.

Client

  • Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data.
  • I followed the coding and design guidelines.
  • Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
  • I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
  • I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
  • I documented the TypeScript code using JSDoc style.
  • I added multiple screenshots/screencasts of my UI changes.
  • I translated all newly inserted strings into English and German.

Motivation and Context

We want to provide a sequential order of lecture units and exercises for students participating via the learning paths feature.

Description

This PR adds the first step towards generating this sequential order by recommending an order of the competencies.
The LearningPathService is now able to analyze the student's progress and assign utilities to the corresponding competency. The utility accounts for upcoming due dates, prior competencies, relation types between competencies, and the mastery progress. The utilities can then be used to construct the ordering.

Steps for Testing

Prerequisites:

  • 1 Instructor
  • 1 Student
  • 1 Course with multiple competencies and learning paths enabled
  • exercises and lecture units for each type linked to competencies
  • all possible Competency Relations configured at least once
  1. Log in to Artemis as Instructor
  2. Navigate to Learning Path Management
  3. Open the details view of the learning path by clicking on the View-button
  4. Make sure the displayed graph contains all competencies (start & end node), their linked learning objects (after competency start, before competency end), and the edges corresponding to the configured competency relations (A relates/assumes/extends B -> edge from B to A, A matches B -> edges from matching start node to A & B, edges from A & B to matching end node)
  5. Log in to Artemis as Student
  6. Navigate to the Learning Path participation view in the course
  7. Make sure the visualized graph on the left side of your screen is displayed as expected (no mastered competencies displayed, the competencies are ordered sequentially/a chain from top to bottom excluding the nodes for the learning objects)

Review Progress

Performance Review

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Client

Class/File Line Coverage Confirmation (assert/expect)
learning-path-graph.component.ts 90.32%
learning-path-progress-modal.component.ts 100%
learning-path.service.ts 70.58%
learning-path-graph-sidebar.component.ts 70.58%

Server

Class/File Line Coverage Confirmation (assert/expect)
LearningPathRepository.java 58%
CompetencyProgressService.java 91%
LearningPathService.java 98%
LearningPathResource.java 91%

Screenshots

As this PR doesn't change the UI there are no screenshots of shiny new components, just a comparison between the old graph received from the server (in participation view) and the new one:

Previously:
Screenshot 2023-08-31 at 11 47 55

Now:
Screenshot 2023-08-31 at 11 47 07

lennart-keller
lennart-keller previously approved these changes Sep 28, 2023
Copy link
Contributor

@lennart-keller lennart-keller left a comment

Choose a reason for hiding this comment

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

Retested with Postgres. Works :)

rstief
rstief previously approved these changes Sep 28, 2023
Copy link
Contributor

@rstief rstief left a comment

Choose a reason for hiding this comment

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

Tested in Testing Session 👍

Strohgelaender
Strohgelaender previously approved these changes Sep 29, 2023
Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

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

lgtm now

Base automatically changed from feature/learning-path/health-status to develop September 29, 2023 11:53
@bassner bassner dismissed stale reviews from rstief, tobias-lippert, lennart-keller, Strohgelaender, basak-akan, and JohannesStoehr September 29, 2023 11:53

The base branch was changed.

@MaximilianAnzinger MaximilianAnzinger removed the stacked-pr PR that depends on another PR label Sep 30, 2023
Copy link
Contributor

@JohannesStoehr JohannesStoehr left a comment

Choose a reason for hiding this comment

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

Approve code after changes

Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

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

re-approve code

Copy link
Contributor

@Kroko-fant Kroko-fant left a comment

Choose a reason for hiding this comment

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

looks good to me

Copy link
Contributor

@laurenzfb laurenzfb left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@florian-glombik florian-glombik left a comment

Choose a reason for hiding this comment

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

Tested the changes since c522378 on ts2, the code of these changes looks fine and the alerts are displayed properly.

@bassner bassner added this to the 6.6.0 milestone Oct 9, 2023
@bassner bassner merged commit 18ea125 into develop Oct 9, 2023
@bassner bassner deleted the feature/learning-path/recommendation branch October 9, 2023 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) no-stale ready to merge server Pull requests that update Java code. (Added Automatically!) tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.