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

Assessment: Add internal assessment notes for tutors #8174

Merged
merged 210 commits into from
May 1, 2024

Conversation

chrisknedl
Copy link
Contributor

@chrisknedl chrisknedl commented Mar 11, 2024

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.

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.

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

Closes #1402.

Description

A new object AssessmentNote is added to results. This object has a OneToOne mapping with the creator, a createdDate and string to represent the note itself. The result has a OneToMany relationship with the AssessmentNote, which is in fact an artificial OneToOne relationship, but the other annotation is needed to make lazy fetching work. The assessment notes have no reference to their owning result.

Note: There are many changes on the client-side since the way the save/submit of the assessment was handled is/was implemented slightly differently per exercise type. This PR additionally makes this as uniform as possible in the scope of this PR. There may be a possible future refactoring opportunity to deduplicate some of the previous implementations. This is out-of-scope for this PR, though.

Steps for Testing

Prerequisites:

  • 2 Instructors
  • 1 Student
  1. Log in to Artemis
  2. Navigate to Course Administration
  3. Create a new programming exercise with
    • due date in the far future
    • manual assessment
    • online code editor enabled
  4. Make a submission as a student
  5. Change the due date to something like 3 minutes in the future
  6. After the due date has passed, go to the assessment dashboard of the course

Testing session steps start here!

  1. Start assessment for the exercise
  2. Enter some text in the text box at the very bottom of the page
  3. You should now be able to save, but not submit the assessment
    • Note: feedback, but empty assessment note can be submitted. I.e., there always has to be at least one feedback to be able to submit (previous behaviour as well), saving should be able after any changes.
  4. Leave and re-enter the assessment view, the text should still be there after saving
  5. Log in with another instructor, go to the ‘Course Management‘ → ‘Exercise’ → ‘Scores’ button on the exercise details page in order to view the assessment, and confirm that the note is still the same.

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

Screenshots

Bildschirmfoto 2023-08-24 um 11 48 14

@coderabbitai ignore

Christoph Knoedlseder and others added 30 commits June 27, 2023 12:06
…utor-assessment-container.component.ts

Co-authored-by: Lucas Welscher <ga53foy@mytum.de>
Strohgelaender
Strohgelaender previously approved these changes Apr 27, 2024
Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

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

lgtm now

b-fein
b-fein previously approved these changes Apr 27, 2024
@b-fein b-fein dismissed stale reviews from coderabbitai, Strohgelaender, and themself via 73bfd75 April 28, 2024 11:32
b-fein
b-fein previously approved these changes Apr 28, 2024
Copy link
Contributor

@b-fein b-fein left a comment

Choose a reason for hiding this comment

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

Another round of reapprovals needed: Again, only imports changed, though, due to #8492.

See 73bfd75 for changes.

BaumiCoder
BaumiCoder previously approved these changes Apr 28, 2024
Copy link
Member

@BaumiCoder BaumiCoder left a comment

Choose a reason for hiding this comment

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

Re-approach after import changes

@b-fein b-fein dismissed stale reviews from BaumiCoder and themself via 962ed42 April 28, 2024 19:22
@b-fein b-fein requested review from maximiliansoelch and removed request for maximiliansoelch April 29, 2024 06:15
@maximiliansoelch maximiliansoelch added the maintainer-approved The feature maintainer has approved the PR label Apr 29, 2024
@krusche krusche merged commit 8ea253f into develop May 1, 2024
37 checks passed
@krusche krusche deleted the feature/assessment/review-notes branch May 1, 2024 08:53
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. maintainer-approved The feature maintainer has approved the PR ready to merge server Pull requests that update Java code. (Added Automatically!) tests too-long-open !!! This is an antipattern, we should aim for small PRs that are only open for a short time !!!
Projects
Archived in project
Artemis Development
  
Review in progress
Development

Successfully merging this pull request may close these issues.

Notes functionality or "star" functionality for assessments