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

Exam mode: Allow instructors to view unsubmitted exams and adjust summary layout #7588

Merged
merged 71 commits into from
Jan 7, 2024

Conversation

florian-glombik
Copy link
Contributor

@florian-glombik florian-glombik commented Nov 13, 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.
  • I added multiple screenshots/screencasts of my UI changes.
  • I translated all newly inserted strings into English and German.

Motivation and Context

I ran into this error while working on #7561 and think that it is not good for the UX to navigate to a page that is not loaded properly.
The underlying issue is that there is no access granted to the grade.

Description

  • Allowing instructors to access the grade
  • Improving existing check for test runs
  • Adding clear indicator when an exam is viewed that was not submitted
  • Adjusted the layout of the exam summary
  • Fixing teamscale test that fails on develop
  • Adjusted test for grade access control

Disabling the button that forwards to the student exam summary instead of forwarding to a broken page as the server endpoint denies access to the grade of an unsubmitted exam.

This issue could also be resolved by allowing access to the grade for instructors - since there is the explicit error message You are not allowed to access the grade summary of a student exam which was NOT submitted! thrown I assumed that there are some explicit thoughts behind denying this view for instructor.
As I do not know a scenario where the instructor would like to view unsubmitted exams, I think that just disabling the button that forwards to the student exam is a better solution than letting the instructor run into error messages.

Steps for Testing

  • 1 Student
  • 1 Exam that starts in the future (with prepared exams where the student is registered)
  • 1 Instructor
  • 1 Exam with an unsubmitted student exam
  1. student: verify the preview page of an exam does still have the general information centered
  2. instructor: assess the exam
  3. instructor: release the grades (= Release Dates of Results in the past)
  4. instructor: try to view the unsubmitted Student Exam
  5. instructor: see that no error message is displayed, but instead a banner that the currently viewed exam was not submitted (verify translation for English and German)

Testserver States







Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Client

Class/File Line Coverage Confirmation (assert/expect)
exam-result-summary.component.ts 91.09%
exam-result-overview.component.ts 85.71%

Server

Class/File Line Coverage Confirmation (assert/expect)
ExamService.java 93%
StudentExamResource.java 86%

Screenshots

New

new Layout
image

banner instead of a notification when viewing an resubmitted exam
image

Old

errorMessageWhenViewingUnsubmittedExamAfterReleasingGrades.mov

Summary by CodeRabbit

  • New Features

    • Introduced collapsible card components for exam content.
    • Enhanced exam result summary pages with new collapsible functionality and improved layout.
    • Added new methods for fetching and displaying Iris course and exercise settings.
  • Enhancements

    • Updated exam score calculation methods to accommodate bonus points.
    • Improved student exam grades summary with additional access control logic.
    • Redesigned exam general information and result overview pages for better readability and user experience.
    • Expanded Cypress test support with increased timeout for starting complaints.
  • Bug Fixes

    • Fixed UI inconsistencies in exercise card headers and grading key tables.
    • Corrected margin and alignment issues in general information components.
  • Refactor

    • Streamlined access checks and renamed methods for clarity in exam services.
    • Updated unit tests to reflect changes in collapsible card components and exam result calculations.
  • Style

    • Adjusted styles for tables and collapsible components to enhance visual presentation.
  • Tests

    • Added new unit tests for collapsible card functionality.
    • Removed outdated test cases related to removed functionalities.
    • Refactored existing tests to align with updated component structures.

@florian-glombik florian-glombik added client Pull requests that update TypeScript code. (Added Automatically!) component:Exam Mode small bugfix labels Nov 13, 2023
@florian-glombik florian-glombik self-assigned this Nov 13, 2023
@florian-glombik florian-glombik temporarily deployed to artemis-test2.artemis.cit.tum.de November 19, 2023 21:55 — with GitHub Actions Inactive
@florian-glombik florian-glombik changed the title Exam mode: Prevent user from viewing unsubmitted exams Exam mode: Allow instructors to view unsubmitted exams Nov 25, 2023
@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) labels Nov 25, 2023
@florian-glombik florian-glombik changed the title Exam mode: Allow instructors to view unsubmitted exams Exam mode: Allow instructors to view unsubmitted exams and adjust summary layout Nov 26, 2023
Copy link
Member

@konrad2002 konrad2002 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 on legacy ts11, works as expected

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 08328ab and 2eaac3f.
Files selected for processing (1)
  • src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java (2 hunks)
Additional comments: 1
src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java (1)
  • 452-464: The method getStudentExamGradesForSummary has been updated to include a new parameter accessingUserIsAtLeastInstructor. This parameter is used to determine if access to the exam summary is always allowed, which is the case for test runs or if the accessing user is at least an instructor. The logic for access control is sound and uses clear conditionals to throw an AccessForbiddenException when necessary.

However, the method name and the new parameter suggest that it could be used for different roles, not just students. It would be beneficial to add a method comment explaining the use case for the new parameter to clarify its purpose for future maintainers.

Copy link
Contributor

@laurenzfb laurenzfb 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 described

Copy link

github-actions bot commented Jan 7, 2024

⚠️ Unable to deploy to test servers ⚠️

The docker build needs to run through before deploying.

@github-actions github-actions bot added the deployment-error Added by deployment workflows if an error occured label Jan 7, 2024
@florian-glombik florian-glombik added deploy:artemis-test3 and removed deployment-error Added by deployment workflows if an error occured labels Jan 7, 2024
@florian-glombik florian-glombik temporarily deployed to artemis-test3.artemis.cit.tum.de January 7, 2024 09:39 — with GitHub Actions Inactive
@krusche krusche merged commit 0d2758e into develop Jan 7, 2024
60 of 62 checks passed
@krusche krusche deleted the bugfix/exam/prevent-displaying-unsubmitted-exams branch January 7, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix client Pull requests that update TypeScript code. (Added Automatically!) component:Exam Mode cypress Pull requests that update cypress tests. (Added Automatically!) ready to merge server Pull requests that update Java code. (Added Automatically!) small tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.