Skip to content

Conversation

jonas-de
Copy link
Contributor

@jonas-de jonas-de commented Aug 27, 2025

Checklist

General

Client

  • I strictly followed the client 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

PR for #11050. Changed the course update accordingly, trying to incorporate as much as possible from @florian-glombik's feedback.

Description

This PR introduces sections for the course update form (also used for course creation), improves the wording and capitalization consistency, and adds indentation for nested fields.

Steps for Testing

  • 1 Instructor
  • 1 Admin

Instructor & Admin:

  1. Navigate to course settings.
  2. Change the settings, especially the bold checkboxes that toggle additional fields.
  3. Verify that everything looks good and works as expected.
  4. Navigate to the course details.
  5. Verify that the displayed details are correct and look good.

Admin only:

  1. Open the form to create a new course.
  2. Verify that everything looks good and works as expected.

Check for German & English.

Testserver States

You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.

Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Screenshots

grafik grafik

Summary by CodeRabbit

  • New Features

    • Per-field help text/tooltips for Max Points and Accuracy of Scores; dedicated Accuracy title/info.
    • Replaced inline time controls with a focused time zone input and change warning.
  • UI/UX

    • Course form reorganized into sections (General, Exercises & Grading, Additional Course Features, Enrollment) with heading/layout and accessibility refinements.
    • Added nested form spacing styles and improved enrollment/editor grouping.
  • Admin

    • Organization management controls gated to admins with per-organization remove buttons.
  • Localization

    • EN/DE translation updates for brevity/clarity; removed two obsolete keys.

@github-project-automation github-project-automation bot moved this to Work In Progress in Artemis Development Aug 27, 2025
@github-actions github-actions bot added client Pull requests that update TypeScript code. (Added Automatically!) core Pull requests that affect the corresponding module labels Aug 27, 2025
@helios-aet helios-aet bot temporarily deployed to artemis-test1.artemis.cit.tum.de August 27, 2025 00:39 Inactive
@helios-aet helios-aet bot temporarily deployed to artemis-test6.artemis.cit.tum.de August 27, 2025 00:46 Inactive
@jonas-de jonas-de marked this pull request as ready for review August 27, 2025 00:59
@jonas-de jonas-de requested a review from krusche as a code owner August 27, 2025 00:59
Copy link
Contributor

coderabbitai bot commented Aug 27, 2025

Walkthrough

Split course maxPoints and accuracyOfScores into separate detail lines with per-field help text; removed beta help text from timeZone. Reorganized course update form into new sections, added timeZone typeahead with change warning, reworked admin organization UI, updated test selector, added nested-form-group styles, and adjusted EN/DE i18n keys and tutorialGroups labels.

Changes

Cohort / File(s) Summary
Course detail view
src/main/webapp/app/core/course/manage/detail/course-detail.component.ts
Replace single maxPoints detail with two entries: maxPoints (now includes titleHelpText) and a new accuracyOfScores entry (with title and titleHelpText); remove the titleHelpText from the timeZone detail.
Course update template
src/main/webapp/app/core/course/manage/update/course-update.component.html
Reorganize form into sections (General, Exercises & Grading, Additional Course Features, Enrollment); add timeZone typeahead input with placeholder, change-warning block, accessibility labels, two-column rows for maxPoints/accuracyOfScores, nested form-group nesting, admin-gated organization UI, and moved/renamed feature blocks.
Course update styles
src/main/webapp/app/core/course/manage/update/course-update.component.scss
Add nested-form-group styling, nested-form-group-head, spacing for hr, and layout adjustments for nested rows (additive CSS only).
Translations — course (EN & DE)
src/main/webapp/i18n/en/course.json, src/main/webapp/i18n/de/course.json
Shorten/adjust multiple labels; convert accuracyOfScores from string to object with title and info; add course.form.sections (general/exercises/additionalFeatures/enrollment); remove noOrganizations and organizationAddingHint; other wording/capitalization tweaks.
Translations — tutorial groups (EN & DE)
src/main/webapp/i18n/en/tutorialGroups.json, src/main/webapp/i18n/de/tutorialGroups.json
Rename time zone label to reference tutorial groups and adjust EN changeWarning wording/grammar.
Component spec (test) update
src/main/webapp/app/core/course/manage/update/course-update.component.spec.ts
Refactor organization test to use a shared organization variable in describe scope; update remove-button selector to use dynamic id #removeOrganizationButton-{organization.id} (test-only change).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CU as CourseUpdateComponent
  participant TZ as TimeZone Input (typeahead)
  participant UI as WarningBanner

  User->>CU: Open Edit Course
  CU-->>User: Render sections (General, Exercises & Grading, Additional Features, Enrollment)
  User->>TZ: Select/change time zone
  TZ-->>CU: timeZoneChanged event
  CU->>UI: Display change warning / confirmation
  Note right of UI: Warns sessions/free days will be deleted and scheduled sessions recreated
Loading
sequenceDiagram
  autonumber
  actor User
  participant CD as CourseDetailComponent
  participant I18N as Translations

  User->>CD: Open Course Details
  CD->>I18N: Request titles/info for `maxPoints` and `accuracyOfScores`
  I18N-->>CD: Return `title` and `info` keys
  CD-->>User: Show separate lines for `maxPoints` and `accuracyOfScores` with help icons
  Note right of CD: `timeZone` shown without beta help text
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/general/improve-layout-course-update-component

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot]

This comment was marked as resolved.

@github-project-automation github-project-automation bot moved this from Work In Progress to Ready For Review in Artemis Development Aug 27, 2025

This comment was marked as outdated.

This comment was marked as outdated.

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 27, 2025
coderabbitai[bot]

This comment was marked as outdated.

Copy link

End-to-End (E2E) Test Results Summary

TestsPassed ☑️Skipped ⚠️Failed ❌️Time ⏱
End-to-End (E2E) Test Report205 ran200 passed3 skipped2 failed1h 11m 1s 458ms
TestResultTime ⏱
End-to-End (E2E) Test Report
e2e/exercise/quiz-exercise/QuizExerciseManagement.spec.ts
ts.Quiz Exercise Management › Quiz Exercise Creation › Creates a Quiz with Drag and Drop❌ failure2m 3s 888ms
e2e/exercise/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts
ts.Static code analysis tests › Configures SCA grading and makes a successful submission with SCA errors❌ failure2m 17s 815ms

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 27, 2025
Copy link
Contributor

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/main/webapp/app/core/course/manage/update/course-update.component.spec.ts (2)

821-821: Use template literal for readability (minor).

String interpolation is clearer than concatenation in selectors.

-const removeButton = fixture.debugElement.query(By.css('#removeOrganizationButton-' + organization.id));
+const removeButton = fixture.debugElement.query(By.css(`#removeOrganizationButton-${organization.id}`));

826-835: Align non-admin selector with new dynamic ID.

Keep the selector consistent with the per-organization ID pattern to future-proof the test.

-const removeButton = fixture.debugElement.query(By.css('#removeOrganizationButton'));
+const removeButton = fixture.debugElement.query(By.css(`#removeOrganizationButton-${organization.id}`));
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b187864 and 9ad99a6.

📒 Files selected for processing (2)
  • src/main/webapp/app/core/course/manage/update/course-update.component.html (12 hunks)
  • src/main/webapp/app/core/course/manage/update/course-update.component.spec.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/webapp/app/core/course/manage/update/course-update.component.html
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts

⚙️ CodeRabbit configuration file

angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

Files:

  • src/main/webapp/app/core/course/manage/update/course-update.component.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
  • GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
  • GitHub Check: Build .war artifact
  • GitHub Check: client-tests-selected
  • GitHub Check: server-tests
  • GitHub Check: client-tests
  • GitHub Check: client-style
  • GitHub Check: server-style
  • GitHub Check: Analyse
🔇 Additional comments (1)
src/main/webapp/app/core/course/manage/update/course-update.component.spec.ts (1)

808-813: Good move to shared test fixture for organization.

Scoping organization at describe-level and stubbing getOrganizationsByCourse in beforeEach keeps both tests consistent with the dynamic button IDs. LGTM.

Copy link

End-to-End (E2E) Test Results Summary

TestsPassed ☑️Skipped ⚠️Failed ❌️Time ⏱
End-to-End (E2E) Test Report205 ran197 passed3 skipped5 failed1h 18m 15s 497ms
TestResultTime ⏱
End-to-End (E2E) Test Report
e2e/exam/ExamChecklists.spec.ts
ts.Exam Checklists › Exercise group checks › Instructor adds exercise groups and total points possible check is correctly reacting to changes❌ failure35s 150ms
e2e/exam/ExamDateVerification.spec.ts
ts.Exam date verification › Exam timing › Student can start after start Date❌ failure2m 3s 462ms
e2e/exercise/quiz-exercise/QuizExerciseManagement.spec.ts
ts.Quiz Exercise Management › Quiz Exercise Creation › Creates a Quiz with Drag and Drop❌ failure2m 2s 964ms
e2e/exercise/quiz-exercise/QuizExerciseParticipation.spec.ts
ts.Quiz Exercise Participation › DnD Quiz participation › Student can participate in DnD Quiz❌ failure2m 3s 543ms
e2e/exercise/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts
ts.Static code analysis tests › Configures SCA grading and makes a successful submission with SCA errors❌ failure2m 22s 415ms

Copy link
Contributor

@ShuaiweiYu ShuaiweiYu left a comment

Choose a reason for hiding this comment

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

lgtm

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

@helios-aet helios-aet bot temporarily deployed to artemis-test1.artemis.cit.tum.de September 3, 2025 10:03 Inactive
Copy link

@HawKhiem HawKhiem 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 as instructor. Lgtm

Copy link
Member

@tobikli tobikli left a comment

Choose a reason for hiding this comment

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

code

Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@jonas-de jonas-de linked an issue Sep 19, 2025 that may be closed by this pull request
Copy link

End-to-End (E2E) Test Results Summary

TestsPassed ☑️Skipped ⚠️Failed ❌️Time ⏱
End-to-End (E2E) Test Report205 ran201 passed3 skipped1 failed1h 9m 26s 167ms
TestResultTime ⏱
End-to-End (E2E) Test Report
e2e/exercise/quiz-exercise/QuizExerciseManagement.spec.ts
ts.Quiz Exercise Management › Quiz Exercise Creation › Creates a Quiz with Drag and Drop❌ failure2m 4s 621ms

@krusche krusche changed the title General: Reformat and restructure the course update component General: Reformat and restructure the course update page Sep 19, 2025
@krusche krusche added this to the 8.4.1 milestone Sep 19, 2025
@krusche krusche changed the title General: Reformat and restructure the course update page General: Restructure course update page Sep 19, 2025
Copy link
Member

@krusche krusche left a comment

Choose a reason for hiding this comment

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

The new design looks better 👍

@krusche krusche merged commit af0cf0d into develop Sep 19, 2025
24 of 29 checks passed
@krusche krusche deleted the feature/general/improve-layout-course-update-component branch September 19, 2025 20:11
@github-project-automation github-project-automation bot moved this from Ready For Review to Merged in Artemis Development Sep 19, 2025
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!) core Pull requests that affect the corresponding module ready to merge
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

Improve course management settings formatting and structure
7 participants