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: Rework settings system #7424

Merged
merged 47 commits into from
Nov 24, 2023
Merged

Iris: Rework settings system #7424

merged 47 commits into from
Nov 24, 2023

Conversation

Hialus
Copy link
Member

@Hialus Hialus commented Oct 19, 2023

Only deploy to TS9

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

The Iris Settings are currently in a very early state and are not very well implemented and scalable to new features. This PR aims to remedy that.

Description

Completely reworked the server side of the Iris Settings:

  • Uses inheritance for the settings and sub-settings now
  • Changed the DB relationships around
  • Added a code editor settings type for an upcoming feature
  • Completely new settings services
  • New way of sending settings to the client
  • Support for allowed models
  • Auto-update system for the global settings

Smaller adjustments to the UI:

  • Support for allowed models
  • Auto-update settings for the global settings
  • Made inheritance related stuff clearer
  • Refactoring for better maintainability in the future

Steps for Testing

Prerequisites:

  • 1 Admin account (your TUM account if you are an artemis-dev)
  1. Log in to Artemis
  2. Navigate to Server Administration > Iris Settings
  3. Change them around a little bit (not the template please) and save it
  4. Go to a course management page
  5. Select Iris
  6. Change settings a bit and test the inheritance functions and save
  7. Go to a programming exercise in the course
  8. Click on the Iris button
  9. Same procedure
  10. Continue if the chat feature is enabled on all 3 levels
  11. Go to the student view of the exercise
  12. Chat with Iris -> It works as before

TS9 Status

Note

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

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)
course-management-tab-bar.component.ts 95.77%
iris-settings.model.ts 81.81%
iris-sub-settings.model.ts 100%
programming-exercise-detail.component.ts 71.1%
iris-course-settings-update.component.ts 100%
iris-exercise-settings-update.component.ts 100%
iris-global-settings-update.component.ts 100%
iris-chat-sub-settings-update.component.ts 100%
iris-code-editor-sub-settings-update.component.ts 83.56%
iris-common-sub-settings-update.component.ts 100%
iris-global-autoupdate-settings-update.component.ts 75%
iris-hestia-sub-settings-update.component.ts 100%
iris-settings-update.component.ts 89.83%
iris-settings.service.ts 95%

Server

Class/File Line Coverage Confirmation (assert/expect)
Course.java 91%
ProgrammingExercise.java 91%
IrisChatSubSettings.java 100%
IrisCodeEditorSubSettings.java 100%
IrisCourseSettings.java 97%
IrisExerciseSettings.java 93%
IrisGlobalSettings.java 97%
IrisHestiaSubSettings.java 100%
IrisModelListConverter.java 63%
IrisSettings.java 100%
IrisSettingsType.java 100%
IrisSubSettings.java 100%
IrisSubSettingsType.java 100%
CourseRepository.java 78%
IrisSettingsRepository.java 61%
IrisCombinedChatSubSettingsDTO.java 100%
IrisCombinedCodeEditorSubSettingsDTO.java 100%
IrisCombinedHestiaSubSettingsDTO.java 100%
IrisCombinedSettingsDTO.java 100%
IrisRateLimitService.java 100%
IrisChatSessionService.java 82%
IrisHestiaSessionService.java 90%
IrisSettingsService.java 79%
IrisSubSettingsService.java 89%
ProgrammingExerciseService.java 93%
CourseResource.java 94%
AdminIrisSettingsResource.java 42%
CodeHintResource.java 86%
IrisSessionResource.java 82%
IrisSettingsResource.java 96%

Screenshots

Global Settings:
image
Course Settings (now as a proper sub-page)
image
Moved Iris Button on PE page:
image
Programming Exercise Settings:
image

Enable/Disable on the exercise/course detail pages:
image
image

# Conflicts:
#	src/main/java/de/tum/in/www1/artemis/repository/iris/IrisSettingsRepository.java
#	src/main/java/de/tum/in/www1/artemis/service/iris/IrisSettingsService.java
@Hialus Hialus self-assigned this Oct 19, 2023
@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. labels Oct 19, 2023
@github-actions github-actions bot added the client Pull requests that update TypeScript code. (Added Automatically!) label Oct 20, 2023
@RY997 RY997 added deploy:artemis-test9 Testserver for Project Theia and removed lock:artemis-test9 labels Nov 12, 2023
@RY997 RY997 temporarily deployed to artemis-test9.artemis.cit.tum.de November 12, 2023 11:48 — with GitHub Actions Inactive
@github-actions github-actions bot added lock:artemis-test9 and removed deploy:artemis-test9 Testserver for Project Theia labels Nov 12, 2023
Copy link
Contributor

@RY997 RY997 left a comment

Choose a reason for hiding this comment

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

Code lgtm in general, leave some comments

@Hialus Hialus requested a review from RY997 November 13, 2023 09:47
RY997
RY997 previously approved these changes Nov 20, 2023
Copy link
Contributor

@RY997 RY997 left a comment

Choose a reason for hiding this comment

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

Code LGTM and worked well!

@Hialus Hialus linked an issue Nov 23, 2023 that may be closed by this pull request
Copy link
Contributor

@RY997 RY997 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

@MichaelOwenDyer MichaelOwenDyer 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 our Code Editor Chatbot PR, works well.

@krusche krusche added this to the 6.7.0 milestone Nov 24, 2023
@krusche krusche merged commit bd0d757 into develop Nov 24, 2023
28 of 34 checks passed
@krusche krusche deleted the refactor/iris-settings-rework branch November 24, 2023 18:40
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!) component:Iris database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. ready for review 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.

Iris: Label is linked to wrong switch
4 participants