Artemis icon indicating copy to clipboard operation
Artemis copied to clipboard

`Exam Mode`: Add sidebar to student view to display exams

Open edkaya opened this issue 1 year ago • 1 comments

Checklist

General

Client

  • [x] 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.
  • [x] I strictly followed the client coding and design guidelines.
  • [x] 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.
  • [x] I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
  • [x] I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
  • [x] I documented the TypeScript code using JSDoc style.
  • [x] I added multiple screenshots/screencasts of my UI changes.
  • [x] I translated all newly inserted strings into English and German.

Motivation and Context

In the current design of Artemis, we display real and test exams side by side. In other parts of Artemis, such as lectures and exercises, we use sidebars to group elements. To maintain consistency, the sidebar is also implemented in the course exams view in order to group and display real and test exams. This implementation is therefore a part of the new design in exam mode.

Description

  • Added the new sidebar into course exams
  • Since the current implementation covers both exam-participation-cover (welcome&end page) and exam-participation components in the same route url, the problem occured in a way that both welcome page and exam itself is displayed in the sidebar content. But the intended behaviour is to display welcome-page in the sidebar content, whereas the exam itself as full-screen. To omit this problem, another route is created for the exam itself. So the changes in this PR also covers this implementation. So exam-participation-cover component is removed from the exam-participation template and instead, we have a navigation from exam-participation-cover to exam-participation, which allowed to display exam as full-screen. To display necessary information in welcome page as well as to get the necessary exam from the server, we download the exam from the server in exam-participation-cover instead of in exam-participation.
  • Welcome-page, exam-summary and submission error text are displayed in exam-participation-cover component
  • Exam and end-page are displayed in exam-participation component

Steps for Testing

Prerequisites:

  • 1 Admin
  • 1 Students
  • 1 Real Exam
  • 1 Test Exam
  1. Log in to Artemis with Admin account
  2. Navigate to Course Management
  3. Select a Course
  4. Navigate to Exams
  5. Create an Exam
  6. Register the student account
  7. Go to Student Exams
  8. Click 'Generate individual exams' and then 'Prepare exercise start'
  9. Log in to Artemis with Student account
  10. Select the course
  11. Navigate to Exams
  12. Verify that you have real and test exams being displayed in the sidebar with correct information
  13. Verify that you can hide the sidebar with toggle feature (CTRL+Shift+B)
  14. Verify that you see the welcome page of the exam in the sidebar-content
  15. Verify that you are correctly navigated to exam after clicking on 'Start' with new url (courses/:courseId/exams/:examId/participation)
  16. Verify that you can see the exam summary in the sidebar-content when you finished the exam

Exam Mode Testing

Prerequisites:

  • 1 Student
  • 1 Exam
  1. Log in to Artemis
  2. Participate in the exam as a student
  3. Make sure that nothing changes in the exam itself, both functionally and visually.

Testserver States

[!NOTE] These badges show the state of the test servers. Green = Currently available, Red = Currently locked

Review Progress

Performance Review

  • [x] 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

Code Review

  • [ ] Code Review 1
  • [ ] Code Review 2

Manual Tests

  • [ ] Test 1
  • [ ] Test 2

Exam Mode Test

  • [ ] Test 1
  • [ ] Test 2

Test Coverage

Screenshots

edkaya avatar May 24 '24 17:05 edkaya

Walkthrough

This update includes various modifications to Angular HTML templates and TypeScript files in a web application. Key changes encompass the addition of exam-related functionalities, including the handling of exam participation, user interface adjustments, and the enhancement of visual indicators for exam states. Additionally, there are corrections to syntax and formatting, implementation of new services, and updates to translation files.

Changes

File Path Change Summary
src/.../exam-scores/exam-scores.component.html Set median score to 0 when falsy.
src/.../participate/exam-participation-cover.component.html Removed trailing comma from exam.title in interpolation.
src/.../quiz-participation.component.html Removed trailing commas in percentage and ngClass bindings.
src/.../exercise-assessment-dashboard.component.html Adjusted button labels and submission assessments.
src/.../feedback.component.html Corrected a syntax error by removing a trailing comma.
src/.../drag-and-drop-question.component.html Removed trailing commas in ngStyle attribute bindings.
src/.../course-overview.component.html Updated layout and logic based on isExamStarted.
src/.../course-statistics.component.html Removed trailing comma from percentage assignment.
src/.../course-exam-attempt-review-detail.component.html Modified commentary and visual indication for finished exams.
src/.../course-overview.component.ts Added CourseExamsComponent import and exam participation logic with ExamParticipationService.
src/.../course-exams.component.html Enhanced layout for course and exam selections.
src/.../course-exams.component.ts Introduced several properties and methods for managing exam-related data and sidebar.
src/.../course-exams.module.ts Updated imports to include ArtemisSidebarModule and removed RouterModule.
src/.../sidebar-card-item.component.html Added conditional rendering based on sidebarType for exams.
src/.../sidebar-card-item.component.scss Introduced .small-title-color class.
src/.../sidebar.component.ts Added getSize() method to handle different sidebarType sizes.
src/.../types/sidebar.ts Expanded types to support exams, including new properties and enums.
src/.../i18n/de/student-dashboard.json Added new German translations for exam-related elements.
src/.../i18n/en/student-dashboard.json Added new English translations for exam-related elements.
src/.../course-overview.service.ts Added support for handling exams, including new methods for exam mapping and sorting.
src/.../courses-routing.module.ts Introduced new components and services for exam management and participation.
src/test/.../course-overview.service.spec.ts Extended tests to include handling of past and future exams.
src/test/.../sidebar-card-large.component.spec.ts Modified navigation path comparison in tests.
src/test/.../sidebar.component.spec.ts Added tests for different sidebar sizes based on sidebarType.

Recent review details

Configuration used: .coderabbit.yaml Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between fe85502b1edd6110752818cb24813fe573326caf and 62c80f2d8677ff5f6cbefa28ed33c75338a3b63c.

Files selected for processing (4)
  • src/main/webapp/app/overview/course-exams/course-exams.component.html (1 hunks)
  • src/main/webapp/app/overview/course-exams/course-exams.component.ts (6 hunks)
  • src/main/webapp/app/overview/course-exams/course-exams.module.ts (1 hunks)
  • src/main/webapp/app/types/sidebar.ts (3 hunks)
Additional context used
Path-based instructions (4)
src/main/webapp/app/overview/course-exams/course-exams.module.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

src/main/webapp/app/overview/course-exams/course-exams.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/types/sidebar.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

src/main/webapp/app/overview/course-exams/course-exams.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

Biome
src/main/webapp/app/types/sidebar.ts

[error] 56-56: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)

any disables many type checking rules. Its use should be avoided.

src/main/webapp/app/overview/course-exams/course-exams.component.ts

[error] 93-93: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 98-98: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 116-116: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)

Simplify your code by directly assigning the result without using a ternary operator. If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code. Check for more details about NOT operator. Unsafe fix: Remove the conditional expression with


[error] 125-125: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 125-125: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 164-166: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead.


[error] 167-167: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 167-167: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 186-186: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 186-186: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 189-189: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 189-189: Forbidden non-null assertion. (lint/style/noNonNullAssertion)


[error] 211-211: Template literals are preferred over string concatenation. (lint/style/useTemplate)

Unsafe fix: Use a template literal.

Additional comments not posted (8)
src/main/webapp/app/overview/course-exams/course-exams.module.ts (2)

7-7: Addition of ArtemisSidebarModule:

The addition of ArtemisSidebarModule is appropriate given the context of integrating sidebar functionality into the course exams component. This aligns with the PR's objective of enhancing the sidebar's capabilities and interactions within the student view.


10-10: Module Imports Consolidation:

The consolidation of module imports into a single array is clean and follows Angular best practices for module organization. This approach enhances readability and maintainability of the module's dependencies.

src/main/webapp/app/overview/course-exams/course-exams.component.html (1)

1-17: Use of Angular's New Syntax:

The use of the new Angular syntax @if instead of *ngIf is a significant improvement in terms of readability and aligns with the latest Angular recommendations. This change should help in reducing the common pitfalls associated with the asterisk-prefixed structural directives.

src/main/webapp/app/types/sidebar.ts (2)

9-9: Documentation for ExamGroupCategory:

It's beneficial to add comments explaining the purpose and usage of newly introduced types, especially enums, to improve code readability and maintainability. This is particularly important for ExamGroupCategory to clarify its role in categorizing exams.


4-4: Use import type for Type-Only Imports:

To optimize the build and ensure that TypeScript only uses these imports for type checking, consider using the import type syntax.

- import dayjs from 'dayjs/esm';
+ import type dayjs from 'dayjs/esm';

Likely invalid or redundant comment.

src/main/webapp/app/overview/course-exams/course-exams.component.ts (3)

3-3: Import Optimization:

Using TypeScript's type-only import syntax for ActivatedRoute and Router can help with tree shaking and potentially reduce the final bundle size.


116-116: Simplify Boolean Assignment:

The conditional expression for examSelected is unnecessarily verbose. Simplify it by directly casting examId to a boolean.

Tools
Biome

[error] 116-116: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)

Simplify your code by directly assigning the result without using a ternary operator. If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code. Check for more details about NOT operator. Unsafe fix: Remove the conditional expression with


211-211: Use Template Literals for String Concatenation:

To enhance readability and maintain modern JavaScript practices, replace string concatenation with template literals.

Tools
Biome

[error] 211-211: Template literals are preferred over string concatenation. (lint/style/useTemplate)

Unsafe fix: Use a template literal.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar May 24 '24 17:05 coderabbitai[bot]

Tested on TS3, starting the exam does not seem to work any more. image It seems to create a new exam session for every click on the start button though, visible under the student exams overview.

Thanks a lot for your insights! It seems I need to adapt my routing according to the new changes in the develop, because there are some new changes regarding the routing in courses module. I was going to fix this tomorrow, so exam will work again but it seems I forgot to change the label to "Work in Progress", sorry about that. I'll let you know when I am finished with it.

edkaya avatar May 26 '24 19:05 edkaya

Tested on ts4: Exams are displayed in the sidebar. The sidebar can be toggled on and off using CTRL+Shift+B. Starting the exam correctly takes me to the exam page. Tested on ts5: Exam results are displayed correctly for previous exams.

Idea: In the past, open exams were easily distinguishable from closed exams. It may be useful to add an indicator in this PR or in a follow up, although this is very optional and just something I noticed.

Thanks for your feedback. I will implement status icons in a follow up PR to indicate if the exam is open/closed or in another status. Since this process is still in discussion, I didn't implement in this PR.

edkaya avatar Jun 04 '24 11:06 edkaya

Tested on TS4 on Safari, all interactions with sidebar and content work as expected, hotkey works 👍

One opinion generally about the sidebar for the future - I think it would be more intuitive if sidebar was visible every time user navigates back to exams/exercises. Because it may be hard to guess where to toggle it back on or generally user may forget about its existence and can't find the list of exams/exercises 😄

Thank you for your advice and opinion 🙂 I think it is the intended behaviour for all sidebar components such as lectures, exercises, exams and tutorial groups. Because the sidebar gets invisible if only the user manually toggles it, which means that user is aware of that feature. But since exams can be long and user may forget about this feature, I can discuss this with Stephan and Ramona about it 👍

edkaya avatar Jun 04 '24 17:06 edkaya

Could you please check if the Playwright tests are running correctly? I had to make some adjustments after modifying the sidebar on the messages page, and it might be necessary to do the same here.

egekurt123 avatar Jun 20 '24 12:06 egekurt123