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

Tutorial groups: Improve free days management #7953

Conversation

Jan-Thurner
Copy link
Contributor

@Jan-Thurner Jan-Thurner commented Jan 25, 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.

Motivation and Context

This feature extension simplifies the creation of free Days over a longer period like holidays. In the old version, one would need to create many individual free days. Also, the extension allows for a partial free day. Organization internal free periods shorter than one can now be depicted with FreePeriodsWithinDay.
This Feature Extension saves Time for Exercise instructors as holidays can now be created with one action instead of needing one for every day.

Description

A button radio was added to the Free Period Form. Using it, users can choose what TimeFrame to declare as tutorial-free. Depending on the selected TimeFrame, different values need to be set within the form. Created Free Periods of any TimeFrame leads to cancelling all overlapping tutorial sessions. Editing or deleting Free Periods leads to reactivating sessions that were previously cancelled.

Steps for Testing

Prerequisites:

  • 1 Admin
  • 1 Instructor
  1. Log in to Artemis as an Administrator
  2. Click "Course Management" (Top Right)
  3. Click "Create new Course" (Top Right)
  4. Set start and end to realistic values and give the course a name.
  5. Set name of the Group of the Tutors to "artemis-test-tutors"
  6. Set a TimeZone (Bottom of the Form) and Create the Course.
  7. Click "Server Administration" > "User Management"
  8. Find "artemis_test_user6" and click "Edit"
  9. add them to the group "artemis-test-tutors" and click save.
  10. Log in to Artemis as Instructor
  11. Click "Course Management"
  12. Scroll until you find the new course
  13. click "Tutorials"
  14. click "Create Configuration"
  15. set a "Tutorial Group Period" with sufficient length
  16. click "create"
  17. click "Continue to Tutorial Group Management"
  18. Click "Create New Tutorial Group"
  19. Give it a name and a language and assign a Tutor
  20. Configure a recurring schedule and click "save"
  21. click "Holidays"
  22. click "Add New Free Period"
  23. Create at least one FreePeriod overlapping with at least one existing tutorial session.
  24. The Created Period should appear in the according table, sorted ascending by date
  25. Go back to the tutorialGroupFreePeriodManagement. (click "Tutorials")
  26. Click "sessions" of an existing tutor group with overlapping freePeriods. Check that the sessions overlapping are cancelled. check that not overlapping sessions are still active.
  27. Edit or delete some of the overlapping freePeriods by navigating back to the "Tutorial Free Periods Management". The sessions that were cancelled due to the now changed or deleted freePeriod should now be activated again.
  28. Create a FreePeriod overlapping with at least one already existing FreePeriod. This FreePeriod should not be created but a bad request should be returned with the error message "The given tutorial group free period overlaps with another tutorial group free period with ID {{ID}} in the same course.".
  29. Edit at least one Free Period of every type. Do this by hovering over the entry in the list of free days and clicking on edit. Check that the preselected values match the original values. Change the values and click "Edit". Check that the tutorial session is updated correctly. Also, check that sessions that should remain cancelled are still cancelled and check that those that should be active again are active again.
  30. Edit another freePeriod of any type to overlap with an already existing freePeriod of any type. It should again show the error: "The given tutorial group free period overlaps with another tutorial group free period in the same course".
  31. Delete a freePeriod. It should now be removed from the corresponding list.

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

Class/File Line Coverage Confirmation (assert/expect)
TutorialGroupFreePeriodRepository.java 100%
TutorialGroupFreePeriodService.java 83%
TutorialGroupScheduleService.java 100%
TutorialGroupFreePeriodResource.java 90%
TutorialGroupSessionResource.java 93%
TutorialGroupFreePeriodService.ts 96.15%
create-tutorial-group-free-period.component.ts 89.47%
edit-tutorial-group-free-period.component.ts 80%
tutorial-group-free-period-form.component.ts 80%
tutorial-group-free-periods-management.component.ts 91.66%
tutorial-group-free-perios-table.component.ts 100%

Screenshots

image image image image

Summary by CodeRabbit

  • New Features
    • Enhanced the tutorial group scheduling feature with the ability to select specific start and end dates and times, improving flexibility and precision in scheduling free periods.
  • Refactor
    • Improved form submission and validation logic for scheduling free periods, ensuring a smoother user experience.

FreePeriodWithinDay still needs to be validated and added to submit possible.
FreePeriodWithinDay still needs to be validated.
submit possible for FreePeriodWithinDay faulty.
…eparate Lists. The Time if start and endDate still get set to midnight somewhere.
@artemis-bot artemis-bot added this to In progress in Artemis Development Jan 25, 2024
@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) client Pull requests that update TypeScript code. (Added Automatically!) labels Jan 25, 2024
…-and-parts-of-a-day-as-free-as-options-into-the-Tutorial-Free-Days-Management.-#7372' into Tutorial-Add-period-of-free-days-and-parts-of-a-day-as-free-as-options-into-the-Tutorial-Free-Days-Management.-#7372
@Jan-Thurner Jan-Thurner dismissed coderabbitai’s stale review March 18, 2024 07:10

Same reason as before

Copy link
Contributor

@MarkusPaulsen MarkusPaulsen 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 TS1

@MarkusPaulsen MarkusPaulsen added the maintainer-approved The feature maintainer has approved the PR label Mar 18, 2024
Copy link
Contributor

@Santia-go Santia-go 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 TS1.
Everything work as expected.
One suggestion for a follow-up PR.

  1. The labels are almost not visible in the calendar of the Tutor period:
image
  1. Would be good to have the slection option in the calendar instead of select start and end date.
image

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.

demonstrated in testing session, works!

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.

Demonstrated in testing session, works as expected

Copy link
Contributor

@rstief rstief left a comment

Choose a reason for hiding this comment

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

Demonstrated in testing session works like a charm 🏖️

@krusche krusche changed the title Tutorial Free Periods: add period of free days and parts of a day as free as options into the tutorial free days management. Tutorial groups: Improve free days management Mar 19, 2024
@krusche krusche added this to the 6.9.3 milestone Mar 19, 2024
@krusche krusche merged commit 0c2efda into develop Mar 19, 2024
77 of 82 checks passed
@krusche krusche deleted the Tutorial-Add-period-of-free-days-and-parts-of-a-day-as-free-as-options-into-the-Tutorial-Free-Days-Management.-#7372 branch March 19, 2024 08:37
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 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.

None yet