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

Iris: Enable Iris for exercises based on their categories #9461

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Hialus
Copy link
Member

@Hialus Hialus commented Oct 13, 2024

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) and too complex database calls.
  • I strictly followed the principle of data economy for all 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 (e.g. using paging).
  • I strictly followed the principle of data economy for all client-server REST calls.
  • 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 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.

Changes affecting Programming Exercises

  • High priority: I tested all changes and their related features with all corresponding user types on a test server configured with the integrated lifecycle setup (LocalVC and LocalCI).
  • I tested all changes and their related features with all corresponding user types on a test server configured with Gitlab and Jenkins.

Not applicable, due to instructor only feature

Motivation and Context

Currently, instructors have to enable Iris for every individual exercise, which is annoying and easy to miss for one exercise.
A better option would be to enable Iris automatically for exercises based on some criteria or to switch between opt-in and opt-out for exercises.
After talking with @bassner and @krusche, we decided that the way to go is to enable Iris automatically based on the categories of exercises.

Description

  • Added a settings option in the course setting to select categories, for which Iris will be activated
  • Whenever the course settings categories are updated, the exercises' Iris-enabled states are synched
  • Whenever an exercise's categories change, the Iris-enabled state is synched
  • Also fixed a bug that prevented Text exercises with Iris Settings to be deleted

Steps for Testing

Prerequisites:

  • 1 Instructor
  • 1 Programming Exercise
  • 1 Text exercise
  • 1 Course with Iris enabled (needs an admin account)
  1. Log in to Artemis
  2. Navigate to Course Administration
  3. Add some categories to the exercises
  4. Open the Iris settings of the course
  5. Select some categories
  6. Check the exercises Iris settings
    a. If the exercise has a category that is included in the course Iris settings, Iris is enabled
    b. If the exercise has no category that is in the course Iris settings, Iris is disabled (unless manually enabled)
  7. Change the categories in either the course Iris settings or the exercise
  8. Iris will be correctly enabled/disabled for the exercises

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

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 even for very large courses with more than 2000 students.
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance even for very large courses with more than 2000 students.

Not applicable, due to instructor only feature

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Will be added, once the server tests on GitHub run through 🙃

Screenshots

image

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced enhanced exercise deletion process with optional Iris settings management.
    • Added category selection for Iris settings, allowing users to enable or disable features based on categories.
    • New UI elements for managing category selection in settings.
  • Bug Fixes

    • Improved handling of exercise settings updates based on category changes.
  • Documentation

    • Updated internationalization files with new keys for category-based activation.
  • Tests

    • Enhanced test coverage for category selection functionality in Iris settings management.

@coderabbitai ignore (Because of spam)

@Hialus Hialus requested a review from a team as a code owner October 13, 2024 00:39
@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!) database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. exercise Pull requests that affect the corresponding module iris Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module text Pull requests that affect the corresponding module labels Oct 13, 2024
@Hialus Hialus marked this pull request as draft October 13, 2024 00:40

This comment was marked as spam.

coderabbitai[bot]

This comment was marked as outdated.

@Hialus Hialus marked this pull request as ready for review October 13, 2024 01:36
coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as spam.

@Hialus

This comment was marked as outdated.

Copy link

@HawKhiem HawKhiem left a comment

Choose a reason for hiding this comment

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

Tested on TS3. Works as expected

Screenshot 2024-10-16 201354
Screenshot 2024-10-16 201430
Screenshot 2024-10-16 201451

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!) database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. exercise Pull requests that affect the corresponding module iris Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module ready for review server Pull requests that update Java code. (Added Automatically!) tests text Pull requests that affect the corresponding module
Projects
Status: Ready For Review
Development

Successfully merging this pull request may close these issues.

3 participants