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

Learning paths: Redesign learning path instructor view #9144

Merged
merged 61 commits into from
Oct 18, 2024

Conversation

JohannesWt
Copy link
Contributor

@JohannesWt JohannesWt commented Jul 28, 2024

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
  • I strictly followed the server 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 strictly followed the client 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

The current instructor view for the learning path feature only displays the learning paths of students in the course. To provide a clearer overview, the instructor view should be redesigned to include more aggregated data.

Description

This PR introduces a new design for the learning path instructor view. Notifications about whether everything is set up correctly, which were previously displayed at the top of the learning paths table, now have a dedicated section. In this section, the instructor is alerted if competencies or their relationships need to be created, or if some student learning paths have not yet been generated. These notifications are represented by small, color-coded banners indicating the severity of the issue.

Additionally, the learning paths table has been simplified and slightly redesigned. A new analysis section has also been introduced, which provides more aggregated data for the entire course. This section includes a competency path that shows the average mastery of competencies, offering the instructor a clearer overview of student progress.

Steps for Testing

Prerequisites:

  • 1 Instructor
  • 2 Students
  • No competencies
  1. Log in to Artemis
  2. Navigate to Instructor Learning path view
  3. Verify that the notification "No competencies" is shown
  4. Create two or more competencies (without relations)
  5. Verify that the notification "No relations" is shown
  6. Create relations between the competencies and add a lecture unit to each competency
  7. Verify that that the notification "Missing learning paths" is shown
  8. Log in as a student and complete the lecture unit
  9. Verify as an Instructor that the data in the learning paths table and analytics graph is shown correctly

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.







Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Client

Class/File Line Coverage Confirmation (assert/expect)
competency-graph-modal.component.ts 100%
competency-node.component.ts 88.46% ✅ ❌
learning-path-nav-overview.component.ts 94.11% ✅ ❌
learning-paths-analytics.component.ts 100%
learning-paths-configuration.component.ts 94.73% ✅ ❌
learning-paths-state.component.ts 100%
learning-paths-table.component.ts 97.67% ✅ ❌
learning-path-management.component.ts 94.25% ✅ ❌
learning-path-instructor-page.component.ts 100%
base-api-http.service.ts 95% ✅ ❌
learning-path-api.service.ts 95.83% ✅ ❌
learning-path-health.model.ts 100%
learning-path.model.ts 96.55% ✅ ❌

Screenshots

Before image
After image

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Added methods to retrieve non-zero competency progress records and calculate the average mastery progress for specified competencies.
    • Introduced a new method to generate competency graphs for courses.
    • Added a new localization file for German language support in learning path management.
    • Launched a new Angular component for displaying learning paths in a table format with search and pagination functionalities.
  • Bug Fixes

    • Simplified health status determination in the learning path service by removing the "OK" status.
  • Chores

    • Updated the constructor of the learning path service to include new repository dependencies.
    • Modified the LearningPathHealthDTO to remove the "OK" status from the health status enum.
    • Removed outdated test cases related to health status checks from the LearningPathService tests.

@JohannesWt JohannesWt self-assigned this Jul 28, 2024
@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) client Pull requests that update TypeScript code. (Added Automatically!) labels Jul 28, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 7fb8a9b and 1935c2f.

📒 Files selected for processing (1)
  • src/main/webapp/app/course/learning-paths/components/learning-paths-table/learning-paths-table.component.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/course/learning-paths/components/learning-paths-table/learning-paths-table.component.ts (1)

Pattern src/main/webapp/**/*.ts: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

🔇 Additional comments (4)
src/main/webapp/app/course/learning-paths/components/learning-paths-table/learning-paths-table.component.ts (4)

20-26: Component configuration is well-defined

The LearningPathsTableComponent is appropriately declared with the correct selector, standalone flag, imports, template URL, and style URLs. This adheres to the Angular style guide and coding guidelines.


38-38: Efficient use of computed property for learningPaths

The computed signal learningPaths efficiently derives its value from searchResults, ensuring reactive updates when the underlying data changes.


61-78: Proper implementation of loadLearningPaths method

The loadLearningPaths method correctly handles asynchronous data fetching with appropriate loading state management and error handling. This ensures a smooth user experience and aligns with best practices.


80-84: Effective debouncing in the search method

Implementing debouncing in the search method optimizes API calls, preventing unnecessary requests when users type quickly. This enhances performance and aligns with the coding guideline code_style:arrow_funcs.

Copy link
Contributor

@az108 az108 left a comment

Choose a reason for hiding this comment

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

works as expected now, ty for the quick fix

Copy link
Contributor

@pzdr7 pzdr7 left a comment

Choose a reason for hiding this comment

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

Reapprove

Copy link
Contributor

@SimonEntholzer SimonEntholzer left a comment

Choose a reason for hiding this comment

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

Reapprove

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.

Code

Copy link
Contributor

@MaximilianAnzinger MaximilianAnzinger left a comment

Choose a reason for hiding this comment

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

Maintainer approved

@MaximilianAnzinger MaximilianAnzinger added maintainer-approved The feature maintainer has approved the PR and removed ready for review labels Oct 11, 2024
@krusche krusche removed the maintainer-approved The feature maintainer has approved the PR label Oct 12, 2024
@krusche krusche added this to the 7.6.1 milestone Oct 13, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 59e21d7 and 1fd81fb.

📒 Files selected for processing (1)
  • src/main/java/de/tum/cit/aet/artemis/atlas/web/LearningPathResource.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/atlas/web/LearningPathResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

@MaximilianAnzinger MaximilianAnzinger changed the title Learning paths: Refactor learning path instructor view Learning path: Refactor learning path instructor view Oct 13, 2024
@krusche krusche changed the title Learning path: Refactor learning path instructor view Learning path: Redesign learning path instructor view Oct 18, 2024
@krusche krusche merged commit 96284c7 into develop Oct 18, 2024
25 of 32 checks passed
@krusche krusche deleted the feature/learning-paths/refactor-instructor-view branch October 18, 2024 06:57
@krusche krusche changed the title Learning path: Redesign learning path instructor view Learning paths: Redesign learning path instructor view Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
atlas Pull requests that affect the corresponding module client Pull requests that update TypeScript code. (Added Automatically!) core Pull requests that affect the corresponding module server Pull requests that update Java code. (Added Automatically!) tests
Projects
Status: Merged
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants