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

Plagiarsim checks: Fix default value of after due date checks #7657

Merged
merged 11 commits into from
Dec 8, 2023

Conversation

Strohgelaender
Copy link
Contributor

@Strohgelaender Strohgelaender commented Nov 23, 2023

Checklist

General

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

Fixes #7651
Fixes #7611
Follow-up of #7665

Description

When creating or importing an exercise, the "run plagiarism checks" checkbox was active, even if cpc was disabled. While this had no direct effect, the UI is confusing and should be clear in showing what options are active when working with an exercise.

Steps for Testing

Prerequisites:

  • 1 Instructor
  1. Create a new Exercise -> Check that CPC and the checks after the due date are disabled by default.
  2. Activate CPC -> see that the checks also get enabled
  3. Deactivate CPC -> see that the checks automatically get disabled
  4. Check that creating the exercise works

Testserver States

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

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Client

Class/File Line Coverage Confirmation (assert/expect)
exercise-update-plagiarism.component.ts 100%

@github-actions github-actions bot added the client Pull requests that update TypeScript code. (Added Automatically!) label Nov 23, 2023
@github-actions github-actions bot added the tests label Nov 23, 2023
@Strohgelaender Strohgelaender marked this pull request as ready for review November 23, 2023 18:51
@Strohgelaender Strohgelaender requested a review from a team as a code owner November 23, 2023 18:51
@Strohgelaender Strohgelaender added this to the 6.7.1 milestone Nov 23, 2023
@sarpsahinalp sarpsahinalp temporarily deployed to artemis-test2.artemis.cit.tum.de November 23, 2023 19:25 — with GitHub Actions Inactive
@MaximilianJG
Copy link
Contributor

Tested on Artemis Test 2: Works as expected!

  • Import page renders
  • CPC Checks are disabled by default
  • When enabling CPC the checks also get activated / deactivated
  • Importing works

Copy link
Contributor

@sarpsahinalp sarpsahinalp 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 ts2. Worked as expected for exercises. In exam mode I wan't able to see the CPC part for the same exercise.
Exam exercise import
image
Exercise import
image

@Strohgelaender
Copy link
Contributor Author

@sarpsahinalp CPC is not available for exams, I updated the testing steps to reflect that properly. Thanks for testing!

milljoniaer
milljoniaer previously approved these changes Nov 24, 2023
Copy link
Contributor

@milljoniaer milljoniaer 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 👍

Copy link
Contributor

@jakubriegel jakubriegel left a comment

Choose a reason for hiding this comment

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

Please take a look at #7665

Copy link
Contributor

@tobias-lippert tobias-lippert 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

Copy link
Contributor

@jakubriegel jakubriegel 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. Thanks for this improvement!

Copy link

@FiasKr FiasKr 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, working as expected

Copy link

@JanaNF JanaNF left a comment

Choose a reason for hiding this comment

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

Manual tested on legacy_ts2. Worked as expected.

Copy link

@AntonGeTUM AntonGeTUM left a comment

Choose a reason for hiding this comment

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

Manually tested, worked as described

Copy link

@vinceclifford vinceclifford 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 TS2, works as expected

Copy link

@Predixx Predixx 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 ts2. I think it works. "Check the submissions for plagiarism one more time after the due date" was grayed out for me but checked.

@krusche krusche modified the milestones: 6.7.1, 6.7.2 Dec 4, 2023
@rabeatwork rabeatwork added the maintainer-approved The feature maintainer has approved the PR label Dec 8, 2023
@krusche krusche merged commit c227a8d into develop Dec 8, 2023
39 of 43 checks passed
@krusche krusche deleted the bugfix/plagiarism-detection-undefined branch December 8, 2023 14:23
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!) maintainer-approved The feature maintainer has approved the PR ready to merge tests
Projects
None yet