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: Preserve settings on course and exercise update #7265

Merged
merged 6 commits into from
Oct 3, 2023

Conversation

Hialus
Copy link
Member

@Hialus Hialus commented Sep 25, 2023

Note

Only deploy on 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 documented the Java code using JavaDoc style.

Changes affecting Programming Exercises

  • I tested all changes and their related features with all corresponding user types on Test Server 1 (Atlassian Suite).
  • I tested all changes and their related features with all corresponding user types on Test Server 2 (Jenkins and Gitlab).

Motivation and Context

Currently there is a bug, where the Iris settings are deleted for a course/programming exercise if they are updated.

Description

This PR fixes this behaviour by copying the Iris Settings into the new object.

Steps for Testing

Prerequisites:

  1. Log in to Artemis
  2. Go to the exercise instructor view
  3. Edit the exercise -> Iris still enabled
  4. Edit the course -> Iris still enabled

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

Server

Class/File Line Coverage Confirmation (assert/expect)
IrisSettingsService.java 88%
ProgrammingExerciseService.java 93%
CourseResource.java 94%

@Hialus Hialus requested a review from a team as a code owner September 25, 2023 00:38
@Hialus Hialus self-assigned this Sep 25, 2023
@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) labels Sep 25, 2023
JohannesStoehr
JohannesStoehr previously approved these changes Sep 25, 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.

Code looks good to me

Copy link
Collaborator

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

Left one suggestion. Other than that code LGTM 👍

@krusche krusche changed the title Iris: Preserve settings on course/exercise update Iris: Preserve settings on course and exercise update Oct 3, 2023
@krusche krusche modified the milestones: 6.5.4, 6.5.5 Oct 3, 2023
# Conflicts:
#	src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseService.java
#	src/test/java/de/tum/in/www1/artemis/course/CourseTestService.java
@Hialus Hialus dismissed stale reviews from JohannesStoehr and MichaelOwenDyer via 85c2c0a October 3, 2023 10:59
@krusche krusche merged commit 61ba7b5 into develop Oct 3, 2023
@krusche krusche deleted the bugfix/preserve-iris-settings-on-update branch October 3, 2023 12:10
@krusche krusche removed this from the 6.5.5 milestone Oct 4, 2023
@krusche krusche added this to the 6.5.4 milestone Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:Iris ready for review server Pull requests that update Java code. (Added Automatically!) tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants