Lectures: Replace guided mode with status bar edit and create view

[florian-glombik]

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
• I chose a title conforming to the naming conventions for pull requests.

Client

• I strictly followed the client coding and design guidelines.
• I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
• I added multiple screenshots/screencasts of my UI changes.
• I translated all newly inserted strings into English and German.

Motivation and Context

We do currently have the guided mode for the creation of lectures, however, we consider the approach used for the creation of exercises (status bar that allows you to scroll to sections and displays the current state of these sections) as more user friendly than the current guided mode.

Description

• Introduce the status bar to the lecture create + edit view
• Remove the current guided mode
• Introduce signals and remove code smells
• Renamed previously named "wizard components" and replaced them in folder structure

Steps for Testing

Prerequisites:

• 1 Instructor

1. Create a new Lecture
2. Verify that no guided mode is offered anymore (no button in the upper right corner of the view)
3. Verify that the title and period section are offered
4. Fill out the sections and save the lecture
5. Verify that you are forwarded to the lecture edit page, but now attachments and units sections are available
6. Make changes to the attachments and units section
7. Verify that all changes have been saved
8. Verify that in edit mode the save button is enabled if there is a change made to title or period section (attachment / unit section changes are saved directly)

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.

Review Progress

Performance Review

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Test Coverage

Class/File	Line Coverage	Confirmation (assert/expect)
hasLectureUnsavedChanges.guard.ts	100%	✅
lecture-attachments.component.ts	97.2%	✅
lecture-unit-management.component.ts	89.47%	✅
lecture-update.component.ts	92.24%	✅
lecture.service.ts	97.08%	✅

Screenshots

new version (status bar instead of guided mode)

old version

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced a streamlined lecture update process, enhancing form handling and state management.
  • Added new methods for ingesting lectures and retrieving ingestion states within the lecture service.
  • Updated localization files to improve user interface clarity and functionality for editing lectures.

• Bug Fixes

  • Improved error handling and robustness in various components, including lecture unit management and attachment handling.

• Refactor

  • Simplified component structures and removed unnecessary wizard mode logic for a more straightforward user experience.
  • Enhanced the Attachment class by removing an empty constructor to simplify the class definition.

• Chores

  • Removed deprecated components and associated tests to clean up the codebase.
  • Updated import paths and component references to reflect new directory structures.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/main/webapp/app/lecture/lecture-units/lecture-units.component.html (1)

Line range hint 22-57: Consider refactoring repeated form patterns

While the Angular syntax is correct and follows guidelines (using @if), there's significant repetition in the form blocks. Consider extracting the common pattern into a reusable component or directive.

Example approach:

// lecture-unit-form.component.ts
interface UnitFormConfig {
  type: 'text' | 'video' | 'online' | 'attachment' | 'exercise';
  component: Type<any>;
}

const UNIT_FORM_COMPONENTS: Record<string, UnitFormConfig> = {
  text: { type: 'text', component: TextUnitFormComponent },
  video: { type: 'video', component: VideoUnitFormComponent },
  // ... other mappings
};

<!-- lecture-unit-form.component.html -->
<ng-container #formContainer></ng-container>

<!-- Updated template -->
<div class="form-group">
  <jhi-lecture-unit-form
    [type]="activeUnitType"
    [isEditMode]="isEditingLectureUnit"
    [formData]="getFormData()"
    (onCancel)="onCloseLectureUnitForms()"
    (formSubmitted)="handleFormSubmit($event)"
  />
</div>

This would:

• Reduce template complexity
• Make it easier to add new unit types
• Centralize form handling logic

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee94d3a and 0b2377b.

📒 Files selected for processing (8)

• src/main/webapp/app/lecture/close-edit-lecture-modal/close-edit-lecture-modal.component.html (1 hunks)
• src/main/webapp/app/lecture/lecture-period/lecture-period.component.html (1 hunks)
• src/main/webapp/app/lecture/lecture-units/lecture-units.component.html (2 hunks)
• src/main/webapp/app/lecture/lecture-update.component.html (1 hunks)
• src/main/webapp/app/lecture/lecture-update.component.ts (7 hunks)
• src/main/webapp/i18n/de/lecture.json (2 hunks)
• src/main/webapp/i18n/en/lecture.json (2 hunks)
• src/test/javascript/spec/component/lecture/lecture-update.component.spec.ts (5 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/main/webapp/app/lecture/lecture-update.component.ts

👮 Files not reviewed due to content moderation or server errors (3)

• src/main/webapp/i18n/de/lecture.json
• src/main/webapp/app/lecture/lecture-update.component.html
• src/test/javascript/spec/component/lecture/lecture-update.component.spec.ts

🧰 Additional context used

📓 Path-based instructions (6)

src/main/webapp/app/lecture/lecture-period/lecture-period.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/lecture/lecture-update.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/lecture/lecture-units/lecture-units.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/lecture/close-edit-lecture-modal/close-edit-lecture-modal.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/i18n/de/lecture.json (1)

Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".

src/test/javascript/spec/component/lecture/lecture-update.component.spec.ts (1)

Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}

🔇 Additional comments (5)

src/main/webapp/app/lecture/lecture-units/lecture-units.component.html (3)

2-3: LGTM: Translation keys updated to remove wizard mode references

The translation key changes align with the PR objective of replacing guided mode with a simpler interface.

---

18-18: LGTM: New lecture unit header translation updated

The translation key has been appropriately updated to remove the wizard mode context.

---

20-20: LGTM: Edit lecture unit header translation updated

The translation key has been appropriately updated to remove the wizard mode context.

src/main/webapp/app/lecture/close-edit-lecture-modal/close-edit-lecture-modal.component.html (1)

16-16: LGTM! Translation keys updated correctly

The changes align with the new sections structure and follow the Angular syntax guidelines using @if directives.

Also applies to: 21-21

src/main/webapp/i18n/en/lecture.json (1)

72-80: LGTM! Clear and well-structured section descriptions

The new sections structure provides clear descriptions for each part of the lecture creation/editing process.

Let's verify that all new translation keys are being used in the components:

✅ Verification successful

All translation keys are properly used in components

All new translation keys from lecture.json are being used appropriately in the corresponding components:

• Title section keys in lecture-update.component and close-edit-lecture-modal.component
• Period section keys in lecture-period.component
• Attachments section keys in lecture-update.component
• Units section keys in lecture-units.component

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check usage of new translation keys in components

# Search for each new translation key
echo "Checking usage of new translation keys..."
for key in "artemisApp.lecture.sections.title" \
           "artemisApp.lecture.sections.titleDescription" \
           "artemisApp.lecture.sections.period" \
           "artemisApp.lecture.sections.periodDescription" \
           "artemisApp.lecture.sections.attachments" \
           "artemisApp.lecture.sections.attachmentsDescription" \
           "artemisApp.lecture.sections.units" \
           "artemisApp.lecture.sections.unitsDescription"; do
    echo "\nChecking: $key"
    rg "$key" "src/main/webapp/app"
done

Length of output: 7373

[BBesrour]

code lgtm. also tested on TS1

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

src/main/webapp/app/lecture/lecture-update.component.html (3)

20-26: Simplify nested conditionals for better readability.

The nested conditionals for displaying the course title can be simplified.

-@if (lecture().course) {
-    <div class="form-group">
-        <label class="form-control-label" jhiTranslate="artemisApp.lecture.course" for="field_course"></label>
-        @if (lecture().course?.title) {
-            <div class="d-flex">
-                <input id="field_course" disabled type="text" class="form-control" name="course" [(ngModel)]="lecture().course!.title!" />
-            </div>
-        }
-    </div>
-}
+@if (lecture().course?.title) {
+    <div class="form-group">
+        <label class="form-control-label" jhiTranslate="artemisApp.lecture.course" for="field_course"></label>
+        <div class="d-flex">
+            <input id="field_course" disabled type="text" class="form-control" name="course" [(ngModel)]="lecture().course.title" />
+        </div>
+    </div>
+}

---

45-52: Simplify save button's disabled condition using a computed property.

The current disabled condition is complex and could be moved to a computed property for better maintainability.

Consider creating a computed property in the component:

public get isSaveDisabled(): boolean {
  return this.editForm.form.invalid || 
         this.isSaving || 
         this.processUnitMode || 
         !this.isChangeMadeToTitleOrPeriodSection || 
         !this.areSectionsValid();
}

Then update the template:

-[disabled]="editForm.form.invalid || isSaving || processUnitMode || !isChangeMadeToTitleOrPeriodSection || !areSectionsValid()"
+[disabled]="isSaveDisabled"

---

93-102: Improve file input validation.

The file input validation could be enhanced by adding proper aria-labels and error states.

 <input
     id="fileInput"
     type="file"
     class="custom-file-input file-input-margin"
     [accept]="acceptedFileExtensionsFileBrowser"
     (change)="onFileChange($event)"
     aria-describedby="fileHelp"
+    [attr.aria-invalid]="!fileName && fileInputTouched"
+    aria-required="true"
     [placeholder]="'artemisApp.attachmentUnit.createAttachmentUnit.chooseFile' | artemisTranslate"
     (click)="fileInputTouched = true"
 />

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae76d07 and 638d5d5.

📒 Files selected for processing (1)

• src/main/webapp/app/lecture/lecture-update.component.html (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/main/webapp/app/lecture/lecture-update.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

🔇 Additional comments (4)

src/main/webapp/app/lecture/lecture-update.component.html (4)

3-12: LGTM: Clean implementation of the new status bar.

The form header structure is well-organized with proper dynamic content handling and the new status bar component aligns with the PR objective of replacing the guided mode.

---

43-43: Replace   with margin utility class.

As previously suggested, use a margin utility class instead of  .

-<fa-icon [icon]="faBan" />&nbsp;<span jhiTranslate="entity.action.cancel"></span>
+<fa-icon [icon]="faBan" class="me-1" /><span jhiTranslate="entity.action.cancel"></span>

---

54-62: LGTM: Clean implementation of attachments section.

The section is well-structured with proper conditional rendering and translation keys.

---

31-36: Verify the implementation of domainActions for the markdown editor.

Please ensure that domainActionsDescription is properly initialized and contains the necessary actions for the markdown editor.

✅ Verification successful

The initialization of domainActionsDescription is properly implemented

The verification shows that domainActionsDescription is correctly initialized with a FormulaAction instance in the lecture-update component:

domainActionsDescription = [new FormulaAction()];

The FormulaAction is a well-implemented domain action that:

• Extends TextEditorDomainAction
• Provides functionality to toggle formula text in the editor
• Is consistently used across multiple components in the codebase for markdown editing
• Allows wrapping selected text with formula delimiters ($$)

This implementation aligns with the markdown editor's requirements and is consistent with how formula actions are handled throughout the application.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for domainActionsDescription initialization
ast-grep --pattern 'domainActionsDescription = $_'

Length of output: 176

---

Script:

#!/bin/bash
# Search for domainActionsDescription property/field declaration and initialization
ast-grep --pattern 'domainActionsDescription'

# Also search for any references to domainActionsDescription in the component file
rg "domainActionsDescription" -A 5 -B 5 "src/main/webapp/app/lecture/lecture-update.component.ts"

Length of output: 495

---

Script:

#!/bin/bash
# Let's check what FormulaAction is and its implementation
rg "FormulaAction" -A 5 -B 5

# Also check for any markdown editor related domain actions
ast-grep --pattern 'class $_Action {
  $$$
}'

Length of output: 31634